CODEWITHSUNDEEP
c++clangstatic-analysisabstract-syntax-tree
Pavel Zaichenkov
Pavel Zaichenkov

Reputation: 835

Matching private class members using clang AST Matcher

I'm writing a Clang tool to statically analyse a source file, and to match and rename all private members of a class.

Consider an example:

class AClass { // problem: my matcher modifies AST node here too
private:
    int a; // <- I know how to rename this 'a' using other matcher
public:
    AClass() {
        AClass cl;
        this->a = 1; // <- rename this 'a'
        cl.a = 2; // <- rename this 'a'
    }
};
void bar(AClass);
void foo() {
    //bar(AClass());
}

I use the following matcher to get access to AST nodes that I'd like to modify. It works as I expect.

clang-query> match memberExpr(hasDeclaration(namedDecl(isPrivate())))

Match #1:

sum.cpp:7:9: note: "root" binds here
    this->a = 1;
    ^~~~~~~

Match #2:

sum.cpp:8:9: note: "root" binds here
    cl.a = 2;
    ^~~~
2 matches.

If in the example I uncomment a line with bar(AClass());, a problem arises. There is an extra match, precisely

Match #3:

sum.cpp:1:7: note: "root" binds here
class AClass {
^~~~~~
3 matches.

which results in rewriting class AClass declaration in a weird way. I want to get rid of this match.

The matcher returns a pointer to a MemberExpr object. I tried to filter the third match by checking isArrow() predicate and it helped, but then I am unable to match expressions with dots, like cl.a.

I am looking for other AST matcher expression or some code that operates on MemberExpr objects, and accesses all private variables in the source file and nothing more.

Upvotes: 4

Views: 2564

Answers (1)

Benjamin Bannier
Benjamin Bannier

Reputation: 58594

Like you have observed the problem is due to the implicitly defined copy constructor. Here is the dump of the AST,

% clang-check -ast-dump -ast-dump-filter=AClass s.cpp --
Dumping AClass:
CXXRecordDecl 0x2cb3700 </tmp/s.cpp:1:1, line:10:1> line:1:7 referenced class AClass definition
|-CXXRecordDecl 0x2cb3810 <col:1, col:7> col:7 implicit referenced class AClass
|-AccessSpecDecl 0x2cb38a0 <line:2:1, col:8> col:1 private
|-FieldDecl 0x2cb38e0 <line:3:3, col:7> col:7 referenced a 'int'
|-AccessSpecDecl 0x2cb3930 <line:4:1, col:7> col:1 public
|-CXXConstructorDecl 0x2cfaee0 <line:5:3, line:9:3> line:5:3 used AClass 'void (void)'
| `-CompoundStmt 0x2cfb440 <col:12, line:9:3>
|   |-DeclStmt 0x2cfb240 <line:6:5, col:14>
|   | `-VarDecl 0x2cfafe0 <col:5, col:12> col:12 used cl 'class AClass' callinit
|   |   `-CXXConstructExpr 0x2cfb210 <col:12> 'class AClass' 'void (void)'
|   |-BinaryOperator 0x2cfb2c0 <line:7:5, col:15> 'int' lvalue '='
|   | |-MemberExpr 0x2cfb270 <col:5, col:11> 'int' lvalue ->a 0x2cb38e0
|   | | `-CXXThisExpr 0x2cfb258 <col:5> 'class AClass *' this
|   | `-IntegerLiteral 0x2cfb2a0 <col:15> 'int' 1
|   `-BinaryOperator 0x2cfb360 <line:8:5, col:12> 'int' lvalue '='
|     |-MemberExpr 0x2cfb310 <col:5, col:8> 'int' lvalue .a 0x2cb38e0
|     | `-DeclRefExpr 0x2cfb2e8 <col:5> 'class AClass' lvalue Var 0x2cfafe0 'cl' 'class AClass'
|     `-IntegerLiteral 0x2cfb340 <col:12> 'int' 2
|-CXXConstructorDecl 0x2cfb070 <line:1:7> col:7 implicit used AClass 'void (const class AClass &) throw()' inline
| |-ParmVarDecl 0x2cfb1b0 <col:7> col:7 used 'const class AClass &'
| |-CXXCtorInitializer Field 0x2cb38e0 'a' 'int'
| | `-ImplicitCastExpr 0x2cfb9e0 <col:7> 'int' <LValueToRValue>
| |   `-MemberExpr 0x2cfb998 <col:7> 'const int' lvalue .a 0x2cb38e0
| |     `-DeclRefExpr 0x2cfb970 <col:7> 'const class AClass' lvalue ParmVar 0x2cfb1b0 '' 'const class AClass &'
| `-CompoundStmt 0x2cfba28 <col:7>
`-CXXDestructorDecl 0x2cfb770 <col:7> col:7 implicit used ~AClass 'void (void) throw()' inline
  `-CompoundStmt 0x2cfb890 <col:7>

where the copy constructor assigned a definition at <line:1:7>. Here the MemberExpr you are accidentially matching is inside the implicit copy constructor. Like Richard Cohen wrote in his comment it is not the MemberExpr which is implicit, but its ancestor, the copy constructor. You can remove these unwanted matches by filtering on the containing like suggested,

match memberExpr(hasDeclaration(namedDecl(isPrivate())),
                 unless(hasAncestor(isImplicit())))

which leaves you with the matches you want,

Match #1:

/tmp/s.cpp:7:5: note: "root" binds here
    this->a = 1; // <- rename this 'a'
    ^~~~~~~

Match #2:

/tmp/s.cpp:8:5: note: "root" binds here
    cl.a = 2;    // <- rename this 'a'
    ^~~~
2 matches.

Upvotes: 7

Related Questions