[cfe-dev] clang-tidy or static analyzer or ...

[Stephen Kelly via cfe-dev]

On 11/09/2019 01:13, Artem Dergachev via cfe-dev wrote:
> 
> 
> On 9/10/19 3:54 PM, Stephen Kelly via cfe-dev wrote:
>> On 10/09/2019 23:46, Artem Dergachev via cfe-dev wrote:
>>> Yup, the most principled way of doing this with ASTMatchers is to 
>>> start with the function decl and then recurse inside it:
>>>
>>>      functionDecl(matchesName(...), forEachDescendant(returnStmt(...)))
>>>
>>> You can always do this in an inside out, but it most likely has 
>>> performance implications (i never really understood how ASTMatcher 
>>> performance works as i've never had any real performance problems 
>>> with them):
>>>
>>>      returnStmt(..., hasAncestor(functionDecl(matchesName(...))))
>>>
>> These will give unexpected results in the presence of lambdas for 
>> example. The `forFunction` expression should be used instead.
>>
> 
> Yay nice, i never noticed this one, thanks!
> 
> Would it make sense to make a "direct" variant of this matcher as well, 
> i.e. a variant of forEachDescendant for functionDecls that only scans 
> statements within that function and doesn't descend into nested 
> declarations?


I've found that in practice, forEachDescendant is far less useful than 
it seems. For example, you might think that it helps you match 
descendants which are ints in a function called takeValue:

     takeValue(someInt);

Imagine there is also a string overload of takeValue too.

However, if you use forEachDescendant to match the ints, you'll match 
the wrong stuff:

     takeValue(toString(someInt));

Always use a semantically specific matcher instead of 
has/hasDescendant/forEachDescendant etc. And if a semantically specific 
matcher doesn't exist, create it (either in your own code or upstream).

Thanks,

Stephen.