clang-tidy check: Uninitialised primitive members in constructors

Clang Frontend
1

I’m looking to make use of a clang-tidy check that would find (and optionally value-initialise) primitive class members (pointers, int, doubles etc).

class C
{
int x_;
public:
C() {} // no x_()
};

Is this:

  1. Already possible? (I can’t find it)

  2. Already work in progress? (I’d like to participate)

  3. Possible at all?

  1. Is motivated by my very weak understanding of the matchers, I’m not sure if I can match things that are missing if the missing things depend on the node I’m looking at. It would be nice to match the compiler generated constructor too (and generate one) if that would fail to initialise primitives as required.

Thanks in advance,

Jon

2

In article <CAAbBDD_qERV4xufHtjBjgQT9m-1AsxretLTS_ABvQGGkeAOX+A@mail.gmail.com>,
    Jonathan Coe <jbcoe@me.com> writes:

I'm looking to make use of a clang-tidy check that would find (and
optionally value-initialise) primitive class members (pointers, int,
doubles etc).

class C
{
  int x_;
public:
  C() {} // no x_()
};

Is this:

1) Already possible? (I can't find it)

I believe so.

2) Already work in progress? (I'd like to participate)

I am interested in clang-tidy checkers/fixers that identify the same
issues as cppcheck and this is something that they identify currently.
You can use cppcheck as a reference checking mechanism.

3) Possible at all?

Yes.

3. Is motivated by my very weak understanding of the matchers, I'm not sure
if I can match things that are missing if the missing things depend on the
node I'm looking at. It would be nice to match the compiler generated
constructor too (and generate one) if that would fail to initialise
primitives as required.

I would play with clang-query and experiment with the matchers that
you need to identify:

- a class with POD members
- a constructor for a class containing POD members

Maybe you just match all class declarations and all constructors and
you walk through the other information in code in order to identify
"class with POD members".

Your check can maintain state, so you can note that you don't have to
do everything with your matcher. You can record that you saw a class
with POD members and that a c'tor was declared but not defined so
that when you see the definition after the declaration you know this
is a definition you should drill into more carefully.

Remember that POD members can also be initialized by assignment in the
c'tor, so you'll want to have that to avoid false positives.

Start with the simplest case that you can identify accurately and
correct. A tool that works correctly for a subset of cases is still
useful. You can incrementally enhance the check over time to extend
it to more elaborate cases.

Work out of trunk. A bunch of changes have been made to clang-tidy in
trunk compared to 3.5.

The best thing is probably to post questions to the list as you
encounter them.

3

Thanks, this is very helpful.

clang-query>match recordDecl(hasDescendant(fieldDecl(hasType(anyOf(builtinType(), pointerType()))))

will find a class with pointer or builtin types. I’m not sure how to record this to make use of it later.

I want to find any constructor (ignoring implicit ones for now) which triggered this first match but left one of its builtin or pointer fields uninitialised. I can’t yet see how to iterate over fields as the information I care about is which ones are not initialised and they’ll be missing from whatever constructor expression I’m matching.

I can live with false positives in the constructor body for now.

regards,

Jon

4

In article <CAAbBDD9bfbF7aX2_ZFQsMMDEX7cVtOP3wSDBjKN0Ogvx-jrY8A@mail.gmail.com>,
    Jonathan Coe <jbcoe@me.com> writes:

Thanks, this is very helpful.

clang-query>match
recordDecl(hasDescendant(fieldDecl(hasType(anyOf(builtinType(),
pointerType()))))

will find a class with pointer or builtin types. I'm not sure how to record
this to make use of it later.

In the context of clang-tidy, you'll register the above matcher and then
when nodes match it, your check will be called. You can use this to
stash away pointers to the AST or extract whatever information you
need at the time you see the class declaration.

You can add another matcher for the constructor definition and then
when that matcher invokes your check you can use the information
stashed away to do the more detailed analysis.

I don't have clang-query handy right now, but you should be able to
use a combination of constructorDecl, forEachConstructorInitializer,
hasAnyConstructorInitializer, ctorInitializer, isWritten, forField,
and withInitializer to get the necessary matcher for the initializers.

If writing a single matcher expression is too difficult, you can
always match on something simpler and then traverse the details
through the AST nodes themselves.

I find that using the matchers with bind("id") helps me differentiate
the different matchers that invoked my callback. You can see an
example of that style here on my "remove void args" tool:
<https://github.com/LegalizeAdulthood/remove-void-args/blob/master/6/RemoveVoidArgs.cpp&gt;

(I'm in the process of reworking that as a clang-tidy check.)

I can live with false positives in the constructor body for now.

Yeah, I find that its best to start with the simplest case and work
your way up to more complicated cases. Use a /lit/ test like the
other checks in clang-tidy use to verify your change is working
properly.

5

Excellent

I'm close to having this working for simple cases.

I'm struggling to see how I can find the location for the replacement text.
I want the start of the initialisers but I don't see how to get a location for that.

Jon

6

In article <2A81C75A-0E4C-448D-A98E-A538EA32DF27@gmail.com>,
    Jonathan Coe <jonathanbcoe@gmail.com> writes:

I'm struggling to see how I can find the location for the replacement text.
I want the start of the initialisers but I don't see how to get a location fo

r that.

From CXXConstructorDecl::init_begin() you can get the first

initializer.

From FunctionDecl::parameters() you can get the parameters.

So you should be able to look at the source location of the last
parameter and the source location of the first initializer. There
doesn't seem to be any function for explicitly locating the ':' that
begins the initializer list, but it will be between the above two
source ranges.

7

I have this working well now. Thanks for your input.

I recall seeing a list of checks in development but I can’t find it any more.

How would I go about submitting my new check(s) for inclusion in clang-tools-extra?

Thanks again,

Jon

8

In article <CAAbBDD8PYQtiWCbDXED0-38wbZw0PgnRcEhgZfqw7JdEYytpUA@mail.gmail.com>,
    Jonathan Coe <jbcoe@me.com> writes:

How would I go about submitting my new check(s) for inclusion in
clang-tools-extra?

I'm following the process described here:
<http://llvm.org/docs/Phabricator.html&gt;

Works great!