Page MenuHomePhabricator

[ASTMatchers] Add matcher hasAnyName.
ClosedPublic

Authored by sbenza on Feb 11 2016, 12:37 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

sbenza updated this revision to Diff 47704.Feb 11 2016, 12:37 PM
sbenza retitled this revision from to [ASTMatchers] Add matcher hasAnyName..
sbenza updated this object.
sbenza added a reviewer: alexfh.
sbenza added a subscriber: cfe-commits.
klimek added inline comments.Feb 12 2016, 3:40 AM
lib/ASTMatchers/ASTMatchersInternal.cpp
408 ↗(On Diff #47704)

That empty line confuses me for some reason.

443 ↗(On Diff #47704)

Here and elsewhere: please annotate all bool parameters with comments about their names, or use enums /*AllowFullyQualified=*/

alexfh edited reviewers, added: klimek; removed: alexfh.Feb 12 2016, 4:56 AM
alexfh removed a subscriber: klimek.
alexfh added a subscriber: alexfh.Feb 12 2016, 5:00 AM
alexfh added inline comments.
include/clang/ASTMatchers/ASTMatchersInternal.h
644 ↗(On Diff #47704)

Why not ArrayRef<StringRef>?

bkramer added inline comments.
include/clang/ASTMatchers/ASTMatchersInternal.h
644 ↗(On Diff #47704)

That's an artifact of how llvm::VariadicFunction works. It gives you an array ref of pointers to the arguments.

alexfh added inline comments.Feb 12 2016, 5:28 AM
include/clang/ASTMatchers/ASTMatchersInternal.h
644 ↗(On Diff #47704)

Thanks! Good to know. Maybe a add a comment or is it just me who doesn't know this?

sbenza updated this revision to Diff 47802.Feb 12 2016, 7:41 AM
sbenza marked 2 inline comments as done.

Minor fixes:

  • Add argument comments
  • Move vector contructor to callers to simply API of HasNameMatcher
include/clang/ASTMatchers/ASTMatchersInternal.h
644 ↗(On Diff #47704)

No reason to leak the VariadicFunction design into here.
I moved the transformation into std::vector<> to the caller.

jbcoe added a subscriber: jbcoe.Feb 12 2016, 2:02 PM
sbenza edited reviewers, added: alexfh; removed: klimek.Feb 19 2016, 7:55 AM
sbenza edited subscribers, added: klimek; removed: alexfh.
alexfh accepted this revision.Feb 22 2016, 8:03 AM
alexfh edited edge metadata.

Looks good. Thanks!

(I suppose, Manuel has no substantial comments on this, since he has already seen the patch).

This revision is now accepted and ready to land.Feb 22 2016, 8:03 AM
This revision was automatically updated to reflect the committed changes.