steveire added a comment.

I don't get email notifications when I'm pinged on Phab for some reason. I 
didn't know about this until the email from Aaron.

  // Fails
  EXPECT_TRUE(
      matches(Input, declRefExpr(traverse(TK_IgnoreImplicitCastsAndParentheses,
                                          hasParent(returnStmt())))));

Why do you use `TK_IgnoreImplicitCastsAndParentheses` instead of 
`TK_IgnoreUnlessSpelledInSource`? All you seem to be doing is showing that the 
former is broken. I've always thought we should remove the former. `AsIs` and 
`IgnoreUnlessSpelledInSource` should be enough for anyone.

> @steveire, would you consider this [when using 
> `TK_IgnoreImplicitCastsAndParentheses`] to be a bug in the traversal behavior?

It's probably a bug in how `TK_IgnoreImplicitCastsAndParentheses` is processed?

> I have strong reservations about modal matching, especially given that it had 
> severe issues

I think "severe" is an overstatement.

> I'm only worried we're making AST matching more confusing by having two 
> different ways of inconsistently accomplishing the same-ish thing.

The `traverse` matcher and `IgnoreUnlessSpelledInSource` were introduced so 
that matchers like `hasParentIgnoringImplicit` (and all the other 
`hasParentIgnoring*`) would never be needed (and so that the already-existing 
`ignore*` matchers would be needed only rarely). I agree with Aaron that it's 
not a good design to continue.

I think the existence of this new `hasParentIgnoringImplicit` matcher is 
further motivation that `IgnoreUnlessSpelledInSource` should be used more, 
especially when writing new matcher code. It is simpler. I get the impression 
people haven't tried it and prefer to write the complicated stuff instead.

I still don't see a problem with `traverse` being modal, but that fact seems to 
make you not use it, @ymandel ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88275/new/

https://reviews.llvm.org/D88275

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to