[clang-tools-extra] be6b9e8 - Revert "[clang-tidy] Simplify static assert check"

Roman Lebedev via cfe-commits cfe-commits at lists.llvm.org
Sun May 30 06:47:21 PDT 2021


Author: Roman Lebedev
Date: 2021-05-30T16:44:31+03:00
New Revision: be6b9e8ae71768d2e09ec14619ca4ecfdef553fa

URL: https://github.com/llvm/llvm-project/commit/be6b9e8ae71768d2e09ec14619ca4ecfdef553fa
DIFF: https://github.com/llvm/llvm-project/commit/be6b9e8ae71768d2e09ec14619ca4ecfdef553fa.diff

LOG: Revert "[clang-tidy] Simplify static assert check"

This patch starts to produce a very obvious false-positives,
despite the fact the preexisting tests already cover the pattern.
they clearly don't actually cover it.

https://godbolt.org/z/3zdqvbfxj

This reverts commit 1709bb8c7395418236ec94fe3b9d91fed746452b.

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
    clang-tools-extra/clang-tidy/misc/StaticAssertCheck.h

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp b/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
index e9ea69aaeb323..224936887e033 100644
--- a/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
@@ -27,37 +27,48 @@ StaticAssertCheck::StaticAssertCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context) {}
 
 void StaticAssertCheck::registerMatchers(MatchFinder *Finder) {
-  auto NegatedString =
-      unaryOperator(hasOperatorName("!"), hasUnaryOperand(stringLiteral()));
+  auto NegatedString = unaryOperator(
+      hasOperatorName("!"), hasUnaryOperand(ignoringImpCasts(stringLiteral())));
   auto IsAlwaysFalse =
       expr(anyOf(cxxBoolLiteral(equals(false)), integerLiteral(equals(0)),
                  cxxNullPtrLiteralExpr(), gnuNullExpr(), NegatedString))
           .bind("isAlwaysFalse");
-  auto IsAlwaysFalseWithCast =
-      anyOf(IsAlwaysFalse, cStyleCastExpr(has(IsAlwaysFalse)).bind("castExpr"));
-  auto AssertExprRoot =
-      anyOf(binaryOperator(
-                hasAnyOperatorName("&&", "=="),
-                hasEitherOperand(stringLiteral().bind("assertMSG")),
-                anyOf(binaryOperator(hasEitherOperand(IsAlwaysFalseWithCast)),
-                      anything()))
-                .bind("assertExprRoot"),
-            IsAlwaysFalse);
+  auto IsAlwaysFalseWithCast = ignoringParenImpCasts(anyOf(
+      IsAlwaysFalse, cStyleCastExpr(has(ignoringParenImpCasts(IsAlwaysFalse)))
+                         .bind("castExpr")));
+  auto AssertExprRoot = anyOf(
+      binaryOperator(
+          hasAnyOperatorName("&&", "=="),
+          hasEitherOperand(ignoringImpCasts(stringLiteral().bind("assertMSG"))),
+          anyOf(binaryOperator(hasEitherOperand(IsAlwaysFalseWithCast)),
+                anything()))
+          .bind("assertExprRoot"),
+      IsAlwaysFalse);
   auto NonConstexprFunctionCall =
       callExpr(hasDeclaration(functionDecl(unless(isConstexpr()))));
   auto AssertCondition =
-      expr(optionally(expr(anyOf(AssertExprRoot,
-                            unaryOperator(hasUnaryOperand(AssertExprRoot))))),
-           unless(findAll(NonConstexprFunctionCall)))
+      expr(
+          anyOf(expr(ignoringParenCasts(anyOf(
+                    AssertExprRoot, unaryOperator(hasUnaryOperand(
+                                        ignoringParenCasts(AssertExprRoot)))))),
+                anything()),
+          unless(findAll(NonConstexprFunctionCall)))
           .bind("condition");
   auto Condition =
-      anyOf(callExpr(traverse(TK_AsIs, callExpr(hasDeclaration(functionDecl(
-                                           hasName("__builtin_expect"))))),
-                     hasArgument(0, AssertCondition)),
+      anyOf(ignoringParenImpCasts(callExpr(
+                hasDeclaration(functionDecl(hasName("__builtin_expect"))),
+                hasArgument(0, AssertCondition))),
             AssertCondition);
 
+  Finder->addMatcher(conditionalOperator(hasCondition(Condition),
+                                         unless(isInTemplateInstantiation()))
+                         .bind("condStmt"),
+                     this);
+
   Finder->addMatcher(
-      mapAnyOf(ifStmt, conditionalOperator).with(hasCondition(Condition)).bind("condStmt"), this);
+      ifStmt(hasCondition(Condition), unless(isInTemplateInstantiation()))
+          .bind("condStmt"),
+      this);
 }
 
 void StaticAssertCheck::check(const MatchFinder::MatchResult &Result) {

diff  --git a/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.h b/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.h
index 796fc4827db42..0168d1fcd107f 100644
--- a/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.h
@@ -30,9 +30,6 @@ class StaticAssertCheck : public ClangTidyCheck {
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-  llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
-    return TK_IgnoreUnlessSpelledInSource;
-  }
 
 private:
   SourceLocation getLastParenLoc(const ASTContext *ASTCtx,


        


More information about the cfe-commits mailing list