Page MenuHomePhabricator

[ASTMatchers] New matcher forFunction
ClosedPublic

Authored by baloghadamsoftware on Apr 21 2016, 2:47 AM.

Details

Summary

Matcher proposed in the review of checker misc-assign-operator (name pending). Its goal is to find the direct enclosing function declaration of a statement and run the inner matcher on it. Two version is attached in this patch (thus it will not compile), to be decided which approach to take. The second one always chooses one single parent while the first one does a depth-first search upwards (thus a height-first search) and returns the first positive match of the inner matcher (thus it always returns zero or one matches, not more). Further questions: is it enough to implement it in-place, or ASTMatchersInternals or maybe ASTMatchFinder should be involved?

Diff Detail

Event Timeline

baloghadamsoftware retitled this revision from to [ASTMatchers] New matcher forFunction.
baloghadamsoftware updated this object.
baloghadamsoftware added a reviewer: sbenza.

Please be sure to run clang\docs\tools\dump_ast_matchers.py to regenerate the documentation as well.

I will run it, once we are approaching the final version. This one is more of a question than a real patch.

sbenza edited edge metadata.Apr 21 2016, 7:55 AM

Thanks for doing this!

I think we want the version that iterates all parents.
Otherwise it will have problems with templates.
That is, you won't know which FunctionDecl you will get: the template or the instantiation. And you might need one or the other specifically.

include/clang/ASTMatchers/ASTMatchers.h
5124

I think this matcher works for Decls too, but we can leave it like this until we need the polymorphism.

5127

You should not assert() this.
Parents can be empty if the statement is not inside a function. eg a global variable initializer.

5132

If you are taking from the back, use a vector<> instead.
Or SmallVector<> to avoid allocation in most cases.

5134

Just call CurNode.get<FunctionDecl>. It'll do the dyn_cast for you.

5139

We should check for LambdaExpr here too, and try to match against LambdaExpr::getCallOperator()

5141

You don't need to get a Stmt. getParents() works with DynTypedNodes.
There might be some non-stmt nodes in the middle, like variable declarations.

5142

We should also not traverse FunctionDecls.
Eg:

void F() {
  struct S {
    void F2() {
    }
  };
}

So, on each node: if is a FunctionDecl or a LambdaExpr run the matcher, otherwise traverse.

unittests/ASTMatchers/ASTMatchersTest.cpp
5581

All of this is kind of unnecessary for the test.
The function could just be:

PosVec& operator=(const PosVec&) {
  auto x = [] { return 1; };
  return *this;
}
5596

EXPECT_TRUE(notMatches(...

5601

Please add test cases for the changes suggested to the matcher.

baloghadamsoftware edited edge metadata.

Updated according to the comments.

baloghadamsoftware marked 9 inline comments as done.Apr 22 2016, 6:48 AM
sbenza accepted this revision.Apr 29 2016, 10:42 AM
sbenza edited edge metadata.
This revision is now accepted and ready to land.Apr 29 2016, 10:42 AM
xazax.hun closed this revision.May 4 2016, 5:05 AM