[AST Matchers] has matcher bug?

I am have a problem with has matcher. It doesn’t work for cases like
implicitCastExpr(has(implicitCastExpr()))
or

cxxMemberCallExpr(has(materializeTemporaryExpr())))
or

Hello Piotr,

has() matcher ignores implicit casts and parens. That's not a bug (however, it will be good to point it in doxygen).

13.05.2016 15:27, Piotr Padlewski via cfe-dev пишет:

Thanks for response Alexey.
Is there any reason why it do so? This is very unintuitive and it also makes it harder and uglier to use matchers - instead of saying something(has(something2())) I have to say something2(hasParent(something())).

Piotr

The problem is that in C++ you often have implicit conversions that are completely irrelevant.
has() ignoring implicit conversions is more closely resembling what’s written in the code.

Both options:

  1. adding a new function hasDirect
  2. changing has() to not go through implicit conversions, and refactoring all uses of has() to has(ignoringParenImpCasts())
    … seem fine to me, with different trade-offs.

Thoughts?
/Manuel

The question is: in how many places the ignoring implicit casts is needed for has? I am not sure about statistics, but if it would turned out that in more than half places, the developer that wrote check didn’t need that, then I would probably go with option 2.

For option 2 I would suggest to add another matcher that would be equivalent to “has” that we have right now, but I am not sure how to call it. “hasIndirect” would suggest that it only accepts indirect childs.

Anyway, I like option 2 more, because it seems more resonable.

Piotr

The question is: in how many places the ignoring implicit casts is needed for has? I am not sure about statistics, but if it would turned out that in more than half places, the developer that wrote check didn’t need that, then I would probably go with option 2.

For option 2 I would suggest to add another matcher that would be equivalent to “has” that we have right now, but I am not sure how to call it. “hasIndirect” would suggest that it only accepts indirect childs.

Why would we need a new matcher? We already have has(ignoringParenImpCasts(x)) which would be equivalent to today’s has(x), don’t we?

Sure, I assumed that the feature to “has” be equvalent to has(ignoringParenImpCasts(x)) was important enough, to have new matcher, so you would not have to write has(ignoringParenImpCasts(x)).

So, I’m all for (2) unless we find people who object, and making people write has(ignoringParenImpCasts())
That’s already what we force people to do for various has* versions.

I don’t see any objection, so I am taking care of this refactor right now.

http://reviews.llvm.org/D20801 please see review.

Piotr

Thanks! See comments on code review.