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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to