This is an archive of the discontinued LLVM Phabricator instance.

Qualify all types used in AST matcher macros.
ClosedPublic

Authored by alexfh on Jun 17 2015, 5:22 AM.

Details

Summary

Qualify all types used in AST matcher macros. This makes it possible to
put AST matchers in user code into a namespace other than clang::ast_matchers
and this way prevent ODR violations that could happen when a matcher with the
same name is defined in multiple translation units. Updated comments
accordingly.

Diff Detail

Event Timeline

alexfh updated this revision to Diff 27828.Jun 17 2015, 5:22 AM
alexfh retitled this revision from to Qualify all types used in AST matcher macros..
alexfh updated this object.
alexfh edited the test plan for this revision. (Show Details)
alexfh added reviewers: djasper, klimek.
alexfh added a subscriber: Unknown Object (MLST).
klimek accepted this revision.Jun 17 2015, 5:25 AM
klimek edited edge metadata.

lg

This revision is now accepted and ready to land.Jun 17 2015, 5:25 AM
djasper added inline comments.Jun 17 2015, 5:30 AM
include/clang/ASTMatchers/ASTMatchersMacros.h
23–36

I don't know whether we need to recommend using an unnamed namespace. I would think that certain projects also have a repository of their own reusable matchers.

Do the custom matchers even have to be inside ::clang::ast_matchers?

alexfh added inline comments.Jun 17 2015, 5:34 AM
include/clang/ASTMatchers/ASTMatchersMacros.h
23–36

No, they can be in any namespace now. I just wanted to emphasize that putting all user-defined matchers to clang::ast_matchers is not a good idea. I'll suggest to use user's own namespace for the matchers.

djasper accepted this revision.Jun 17 2015, 5:36 AM
djasper edited edge metadata.

LG

alexfh updated this revision to Diff 27829.Jun 17 2015, 5:42 AM
alexfh edited edge metadata.

Updated usage instructions.

alexfh updated this revision to Diff 27830.Jun 17 2015, 5:46 AM

clang-format with LLVM style.

alexfh closed this revision.Jun 17 2015, 5:53 AM