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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
