Page MenuHomePhabricator

Add new matchers for dependent names in templates
ClosedPublic

Authored by steveire on Wed, Nov 4, 8:11 AM.

Diff Detail

Event Timeline

steveire created this revision.Wed, Nov 4, 8:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Nov 4, 8:11 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
steveire requested review of this revision.Wed, Nov 4, 8:11 AM
aaron.ballman added inline comments.Thu, Nov 5, 11:48 AM
clang/include/clang/ASTMatchers/ASTMatchers.h
2838

Missing full stop at the end of the comment.

2859

This will allow users to match on members that don't have identifiers -- is that intentional? If not, my recommendation is to use something like:

if (const IdentifierInfo *II = Node.getMember().getAsIdentifierInfo())
  return II->isStr(N);
return false;

Either way, we should document and test what the expected behavior is for things like constructors/destructors, overloaded operators, and the likes. (But we don't have to test every kind of odd declaration name.)

2898

Similar concerns here about matching members without identifiers.

2902

I assume this is a const auto & and not a value type? If so, please update the type.

2903

const auto *

2904

!isa<FieldDecl, CXXMethodDecl, VarDecl>(ND)

2906–2908

return ND->getName() != MemberName;

steveire updated this revision to Diff 303271.Thu, Nov 5, 3:13 PM
steveire marked an inline comment as done.

Update

clang/include/clang/ASTMatchers/ASTMatchers.h
2859

I was not able to demonstrate the problem with a test: https://godbolt.org/z/3Grd1b

Can you be more specific?

aaron.ballman added inline comments.Fri, Nov 6, 4:36 AM
clang/include/clang/ASTMatchers/ASTMatchers.h
2859

I was thinking of something along these lines: https://godbolt.org/z/K8serj

steveire added inline comments.Fri, Nov 6, 8:24 AM
clang/include/clang/ASTMatchers/ASTMatchers.h
2859

Added a test for that.

aaron.ballman accepted this revision.Fri, Nov 6, 10:20 AM

LGTM!

clang/include/clang/ASTMatchers/ASTMatchers.h
2859

Thanks!

This revision is now accepted and ready to land.Fri, Nov 6, 10:20 AM
This revision was landed with ongoing or failed builds.Fri, Nov 6, 1:03 PM
This revision was automatically updated to reflect the committed changes.