This is an archive of the discontinued LLVM Phabricator instance.

[ASTMatchers] Introduce a matcher for matching any given Objective-C selector
ClosedPublic

Authored by george.karpenkov on Mar 23 2018, 6:08 PM.

Diff Detail

Event Timeline

alexfh requested changes to this revision.Mar 26 2018, 9:06 AM
alexfh added inline comments.
include/clang/ASTMatchers/ASTMatchers.h
2737

I'd suggest to do the same as with hasAnyName: leave just the std::vector<llvm::StringRef> overload. WDYT?

This revision now requires changes to proceed.Mar 26 2018, 9:06 AM
include/clang/ASTMatchers/ASTMatchers.h
2737

OK.
I think it much more sense to accepts a vector of std::string in the first place, since we need an owning container for the matcher anyway.

george.karpenkov edited the summary of this revision. (Show Details)

@alexfh OK to commit?

Not yet. A few more comments.

lib/ASTMatchers/ASTMatchersInternal.cpp
327

What's the point of this variable? Maybe just use its initializer directly?

334

const std::string & / const auto & ?

335

S == SelString?

342

What's the point of this variable? Maybe just use its initializer directly?

416

What was wrong with the partially qualified name here?

alexfh requested changes to this revision.Mar 28 2018, 11:34 AM
This revision now requires changes to proceed.Mar 28 2018, 11:34 AM
george.karpenkov marked 3 inline comments as done.
george.karpenkov added inline comments.
lib/ASTMatchers/ASTMatchersInternal.cpp
327

The principle of not having too many things magically happening in one line would be IMO violated if we write internal::Matcher<NamedDecl>(new internal::HasNameMatcher(vectorFromRefs(NameRefs)))

416

It does not work.
Using an AST_MATCHER_P macro in ASTMatchersInternal constructs a matcher inside the internal namespace, breaking the previous version of this function.

alexfh accepted this revision.Mar 28 2018, 3:21 PM

LG

lib/ASTMatchers/ASTMatchersInternal.cpp
327

I wouldn't consider any of this stuff magical, but I also won't object to retaining this variable.

This revision is now accepted and ready to land.Mar 28 2018, 3:21 PM
This revision was automatically updated to reflect the committed changes.