[PATCH] D54404: Exclude matchers which can have multiple results

[Samuel Benzaquen via Phabricator via cfe-commits]

sbenza added a comment.

In https://reviews.llvm.org/D54404#1296224, @aaron.ballman wrote:

> In https://reviews.llvm.org/D54404#1295426, @steveire wrote:
>
> > I acknowledge and share the future-proofing concern.
> >
> > We could possibly use something trait-based instead and put the trait beside the matcher definition in ASTMatchers.h, but that doesn't really solve the problem. It only moves the problem.
>
>
> If we could somehow incorporate it into the matcher definition itself, though, it means we don't have two lists of things to keep updated. You're right that it doesn't eliminate the problem -- we still have to know to ask the question when reviewing new matchers (so perhaps something that requires you to explicitly opt-in/out would be better).
>
> Adding @sbenza in case he has ideas.


We could put something in `MatcherDescriptor` to indicate this. However, I am still not sure what property are we looking for here.



================
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:606
+  // be used with particular arguments.
+  static std::vector<StringRef> ExcludedMatchers{
+      "allOf",
----------------
I don't think we are allowed to have non-trivial static storage duration objects like this.
You have to use llvm::ManagedStatic. Or just make it a raw array.


================
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:624
+      "hasAnyDeclaration",
+      "hasAnyName",
+      "hasAnyParameter",
----------------
I'm not sure what goes in this list.
`hasAnyName` is here but not `hasName`.
What is ambiguous about `hasAnyName`?


Repository:
  rC Clang

https://reviews.llvm.org/D54404