Page MenuHomePhabricator

[clang-tidy] Lift common matchers to utils namespace
ClosedPublic

Authored by etienneb on May 2 2016, 4:51 PM.

Diff Detail

Event Timeline

etienneb updated this revision to Diff 55918.May 2 2016, 4:51 PM
etienneb retitled this revision from to [clang-tidy] Lift common matchers to utils namespace.
etienneb updated this object.
etienneb added a reviewer: alexfh.
etienneb added a subscriber: cfe-commits.
alexfh requested changes to this revision.May 2 2016, 6:49 PM
alexfh edited edge metadata.
alexfh added inline comments.
clang-tidy/utils/Matchers.h
20

All these should go to ASTMatchers.h (with tests and documentation).

This revision now requires changes to proceed.May 2 2016, 6:49 PM

Who is the owner of ASTMatcher?

If he is willing to receive these matchers in ASTMatcher, I'll lift them.
Otherwise, we should at least lift them within clang-tidy.

alexfh added a comment.May 3 2016, 1:03 AM

Who is the owner of ASTMatcher?

If he is willing to receive these matchers in ASTMatcher, I'll lift them.
Otherwise, we should at least lift them within clang-tidy.

Send a patch to sbenza or klimek. I think, these matchers are generic enough to be useful in the core matchers library.

etienneb updated this revision to Diff 57314.May 15 2016, 8:49 PM
etienneb edited edge metadata.
etienneb marked an inline comment as done.

rebase over trunk. matchers are lifted to ASTMatchers.

etienneb updated this revision to Diff 57316.May 15 2016, 8:52 PM
etienneb edited edge metadata.

nits: indent

aaron.ballman accepted this revision.May 16 2016, 8:22 AM
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a subscriber: aaron.ballman.

LGTM!

alexfh accepted this revision.May 17 2016, 12:00 PM
alexfh edited edge metadata.

LG. Thank you!

clang-tidy/utils/Matchers.h
20

Consider moving this to ASTMatchers.h in a follow-up.

This revision is now accepted and ready to land.May 17 2016, 12:00 PM
aaron.ballman added inline comments.May 17 2016, 12:02 PM
clang-tidy/utils/Matchers.h
20

Honestly, this one is a lot less interesting as a public AST matcher. I would avoid that unless there's a really solid case for it.

etienneb closed this revision.May 17 2016, 12:42 PM
etienneb marked 2 inline comments as done.