njames93 updated this revision to Diff 235278. njames93 added a comment. I added test cases for the matchers and the check, wasn't able to add the CXXRangeFor to the matcher as that class uses a different dialect for the initializer statement handling. I also couldn't generate the new documentation as the script always fails on my machine, even without the changes i have made, resulting in the ASTMatcher html being empty
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 Files: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp clang/include/clang/ASTMatchers/ASTMatchers.h clang/lib/ASTMatchers/Dynamic/Registry.cpp clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp =================================================================== --- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp +++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp @@ -1071,6 +1071,20 @@ LanguageMode::Cxx17OrLater)); } +TEST(hasInitStorage, MatchesSelectionInitializers) { + EXPECT_TRUE(matches("void baz() { if (int i = 1; i > 0) {} }", + ifStmt(hasInitStorage()), LanguageMode::Cxx17OrLater)); + EXPECT_TRUE( + notMatches("void baz() { if (int i = 1) {} }", ifStmt(hasInitStorage()))); + EXPECT_TRUE( + notMatches("void baz() { if (1 > 0) {} }", ifStmt(hasInitStorage()))); + EXPECT_TRUE( + matches("void baz(int i) { switch (int j = i; j) { default: break; } }", + switchStmt(hasInitStorage()), LanguageMode::Cxx17OrLater)); + EXPECT_TRUE(notMatches("void baz(int i) { switch (i) { default: break; } }", + switchStmt(hasInitStorage()))); +} + TEST(TemplateArgumentCountIs, Matches) { EXPECT_TRUE( matches("template<typename T> struct C {}; C<int> c;", Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp =================================================================== --- clang/lib/ASTMatchers/Dynamic/Registry.cpp +++ clang/lib/ASTMatchers/Dynamic/Registry.cpp @@ -279,6 +279,7 @@ REGISTER_MATCHER(hasIndex); REGISTER_MATCHER(hasInit); REGISTER_MATCHER(hasInitializer); + REGISTER_MATCHER(hasInitStorage); REGISTER_MATCHER(hasKeywordSelector); REGISTER_MATCHER(hasLHS); REGISTER_MATCHER(hasLocalQualifiers); Index: clang/include/clang/ASTMatchers/ASTMatchers.h =================================================================== --- clang/include/clang/ASTMatchers/ASTMatchers.h +++ clang/include/clang/ASTMatchers/ASTMatchers.h @@ -4297,6 +4297,22 @@ return Node.isConstexpr(); } +/// Matches selection statements with initializer. +/// +/// Given: +/// \code +/// if (int foo = bar(); bar > 0) {} +/// switch (int baz = bar(); baz) {} +/// \endcode +/// ifStmt(hasInitStorage()) +/// matches the declaration of foo. +/// switchStmt(hasInitStorage()) +/// matches the declaration of baz. +AST_POLYMORPHIC_MATCHER(hasInitStorage, + AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt)) { + return Node.hasInitStorage(); +} + /// Matches the condition expression of an if statement, for loop, /// switch statement or conditional operator. /// Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp +++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -fexceptions +// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -fexceptions -std=c++17 namespace std { struct string { @@ -117,3 +117,12 @@ return x; } } + +int *init_and_condition() { + if (int *x = g(); x != nullptr) { + return x; + } else { + h(&x); + return x; + } +} Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp +++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp @@ -26,12 +26,14 @@ compoundStmt(forEach( ifStmt(unless(isConstexpr()), // FIXME: Explore alternatives for the - // `if (T x = ...) {... return; } else { <use x> }` - // pattern: + // `if (T x = ...) {... return; } else { <use x> }` and + // 'if (T x = ...; cond) {... return; } else { use <x> }' + // patterns: // * warn, but don't fix; // * fix by pulling out the variable declaration out of // the condition. unless(hasConditionVariableStatement(anything())), + unless(hasInitStorage()), hasThen(stmt(anyOf(InterruptsControlFlow, compoundStmt(has(InterruptsControlFlow))))), hasElse(stmt().bind("else")))
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits