[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-10 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment. LGTM too. I described a more ad-hoc implementation previously: https://steveire.wordpress.com/2018/11/20/composing-ast-matchers-in-clang-tidy/ but I prefer this implementation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72233/new/ https://reviews.llvm.org/

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the patch! I've commit on your behalf in 2823e91d55891e33a7a8b9a4016db4ec9e2765ae CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72233/n

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-08 Thread Rihan Yang via Phabricator via cfe-commits
air20 marked an inline comment as done. air20 added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:282 REGISTER_MATCHER(hasInitStatement); + REGISTER_MATCHER(hasInitializer); REGISTER_MATCHER(hasKeywordSelector); aaron.ballman wrot

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-08 Thread Rihan Yang via Phabricator via cfe-commits
air20 updated this revision to Diff 236866. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72233/new/ https://reviews.llvm.org/D72233 Files: clang/docs/LibASTMatchersReference.html clang/include/clang/ASTMatchers/ASTMatchers.h clang/include/clang/ASTMatchers/ASTMatchersInternal.h

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:282 REGISTER_MATCHER(hasInitStatement); + REGISTER_MATCHER(hasInitializer); REGISTER_MATCHER(hasKeywordSelector); air20 wrote: > aaron.ballman wrote: > > air20 wrot

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-07 Thread Rihan Yang via Phabricator via cfe-commits
air20 marked an inline comment as done. air20 added a comment. In D72233#1808890 , @aaron.ballman wrote: > In D72233#1808125 , @air20 wrote: > > > In D72233#1807698 , @aaron

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D72233#1808125 , @air20 wrote: > In D72233#1807698 , @aaron.ballman > wrote: > > > LGTM aside from a minor nit. Do you need someone to commit on your behalf? > > > How do I submit

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-07 Thread Rihan Yang via Phabricator via cfe-commits
air20 marked an inline comment as done. air20 added a comment. In D72233#1806483 , @aaron.ballman wrote: > Just to make sure I understand the purpose -- the goal here is to optionally > match one or more inner matchers without failing even if none of the

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor nit. Do you need someone to commit on your behalf? Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:282 REGISTER_MATCHER(hasIni

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-06 Thread Rihan Yang via Phabricator via cfe-commits
air20 updated this revision to Diff 236514. air20 marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72233/new/ https://reviews.llvm.org/D72233 Files: clang/docs/LibASTMatchersReference.html clang/include/clang/ASTMatchers/ASTMatchers.h clang/include/cl

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-06 Thread Rihan Yang via Phabricator via cfe-commits
air20 updated this revision to Diff 236513. air20 added a comment. Included base commits that was missing in the previous diff. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72233/new/ https://reviews.llvm.org/D72233 Files: clang/docs/LibASTMatchersReference.html clang/include/clan

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-06 Thread Rihan Yang via Phabricator via cfe-commits
air20 updated this revision to Diff 236510. air20 added a comment. Fixed Registry.cpp and documentation as per comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72233/new/ https://reviews.llvm.org/D72233 Files: clang/docs/LibASTMatchersReference.html clang/include/clang/ASTMa

[PATCH] D72233: Add a new AST matcher 'optionally'.

2020-01-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Just to make sure I understand the purpose -- the goal here is to optionally match one or more inner matchers without failing even if none of the inner matchers match anything, and this is a different use case than `anyOf()` because that would fail when none of th