1601103 - Enable clang-tidy's cppcoreguidelines-pro-type-member-init

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Kelsey Gilbert [:jgilbert]]

This would have caught a bug for me.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1601103 - Enable clang-tidy's cppcoreguidelines-pro-type-member-init.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Many thanks for doing that!
I am curious, which bug it would have caught?

FYI, we have 1180 occurrences of this issue in our code base.

Comment 3

[Kelsey Gilbert [:jgilbert]]

More or less:

class WebGLContext {
  bool mOptionsFrozen;
};

WebGLContext::WebGLContext() {
  [...] // many lines
  mOptionsFrozen = false; // This line removed by refactor
}

bool WebGLContext::EnsureDefaultFB() {
  if (!Resize(mRequestedSize)) {
    if (mOptionsFrozen) { // ASAN generates cryptic "unusual bool value" crash here
      LoseContext();
    }
    return false;
  }
  return true;
}

I did debate myself on what reliability to give this, and settled on high, but I can be talked down to medium.

This is a common thing we do in our C++03-era code, but that we should really stop doing. It's not a huge foot-gun when first writing code, but it's a definite tripwire for refactors.

After landing this I intend to yak-shave all the instances from my modules out.

Comment 4

[Kelsey Gilbert [:jgilbert]]

ASAN log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=279239241&repo=try&lineNumber=2587
The fix: https://github.com/jdashg/gecko-cinn/commit/4f0172e4e34353b6c16ebdd1604594486fcc1880

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/584510e57b7b
Enable clang-tidy's cppcoreguidelines-pro-type-member-init. r=andi

Comment 6

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/584510e57b7b