[PATCH] D116518: [ast-matchers] Add hasSubstatementSequence matcher

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5435-5442
+/// Matches two consecutive statements within a compound statement.
+///
+/// Given
+/// \code
+///   { if (x > 0) return true; return false; }
+/// \endcode
+/// compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt()))
----------------
LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > How do we extend this to support testing arbitrary sequences of statements? (If this supports two statements, someone will find a need for three, etc).
> Yeah, I was wondering that too.  I didn't see (but didn't look extensively) any support for variadic matchers taking a parameter pack.
> 
> I stopped at 2 because this suits my immediate needs with `readability-simplify-boolean-expr` where I have to manually loop over `CompoundStmt` matches in order to verify that the `if (x) return true; return false;` constructs consist of two adjacent statements.
I don't think we have any variadic matchers yet to model on, but I think if this is only needed for one check, we can add it in the current form as a local matcher for that check. Whenever we figure out how to give the better interface through the static and dynamic matchers, then we can figure out how to lift this to the general matcher interface.

WDYT?


================
Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:2636
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceComplex) {
+  const char *Text = R"code(
----------------
LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > Some more complex-ish tests: can we match within function try or catch blocks and GNU statement expressions?
> Adding something for GNU statement expressions is a good idea.
> 
> I'm open for other suggestions that might improve coverage.
> 
> At the moment, I believe I have 100% coverage of the new matcher, with the exception of GNU statement expressions (honestly, I just copy/pasted that from another matcher on `CompoundStmt`, I wasn't even aware of this GNU extension).
I just double-checked and we do model function try blocks as a body, and we model function body as being a `CompoundStmt`, so I think we're good enough there. I was more worried we did something odd with those (they're pretty easy to forget about given how uncommon they are).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116518/new/

https://reviews.llvm.org/D116518