ymandel added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:60
+ return RegexAndScope{
+ .Regex = llvm::Regex(Name),
+ .MatchQualifiedName = Name.contains("::"),
----------------
I don't think that the clang style-guide accepts designated initializers yet.
================
Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:79
+ llvm::Regex Regex;
+ bool MatchQualifiedName = false;
+ };
----------------
Worth a comment i think.
================
Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:86
+// Returns a matcher that matches NamedDecls against a list of provided regular
+// expressions. If a regular expression contains '::' the NamedDecls qualified
+// name will be used for matching, otherwise its name will be used.
----------------
nit: s/NamedDecls/`NamedDecl`'s/
================
Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:55
return llvm::any_of(NameList, [&Node](const std::string &Name) {
- return llvm::Regex(Name).match(Node.getName());
- });
+ if (Name.find("::") != std::string::npos) {
+ return llvm::Regex(Name).match(Node.getQualifiedNameAsString());
----------------
flx wrote:
> hokein wrote:
> > hokein wrote:
> > > If we pass the name as a `llvm::StringRef`, we can use `if
> > > (Name.contains("::"))` which seems a bit clearer to me.
> > there is a tricky case where the `::` is a prefix of the Name, e.g. a
> > global symbol x, the Name is `::x`, and `getQualifiedNameAsString` of
> > symbol x is `x` (without the `::` prefix), then the matcher will not find a
> > match, but we expect this is a match right?
> >
> I agree that's a tricky case.
>
> We could prepend "::" to getQualifiedNameAsString() and always match against
> it. Would this work?
>
> Or we could do this only for regex names that have a leading "::". This
> would avoid extra string operations, and users could still match using
> "^foo::Bar$" to match ::foo::Bar.
>Or we could do this only for regex names that have a leading "::". This would
>avoid extra string operations, and users could still match using "^foo::Bar$"
>to match ::foo::Bar.
This should work most of the time. There could still be mistakes, like if a
user spells the "::" in some odd way, like "[:][:]", but that seems quite
unlikely in practice.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98738/new/
https://reviews.llvm.org/D98738
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
