[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Jun 6 16:00:49 PDT 2020


njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, lebedev.ri, gribozavr2.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

Ignore template instantiations in the matchers, Addresses readability-simplify-boolean-expr false-positive for bool from template. <https://bugs.llvm.org/show_bug.cgi?id=46226>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81336

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
@@ -948,3 +948,18 @@
 }
 // CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return
 // CHECK-FIXES: S == (A)S;{{$}}
+
+template <bool B>
+void ignoreInstantiations() {
+  if (B) {
+    return;
+  } else {
+    return;
+  }
+}
+
+void instantiate() {
+  // Just make sure the check isn't fooled by template instantiations.
+  ignoreInstantiations<true>();
+  ignoreInstantiations<false>();
+}
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -422,7 +422,7 @@
                                                   bool Value,
                                                   StringRef BooleanId) {
   Finder->addMatcher(
-      ifStmt(isExpansionInMainFile(),
+      ifStmt(unless(isInTemplateInstantiation()),
              hasCondition(cxxBoolLiteral(equals(Value)).bind(BooleanId)))
           .bind(IfStmtId),
       this);
@@ -432,7 +432,7 @@
                                                   bool Value,
                                                   StringRef TernaryId) {
   Finder->addMatcher(
-      conditionalOperator(isExpansionInMainFile(),
+      conditionalOperator(unless(isInTemplateInstantiation()),
                           hasTrueExpression(cxxBoolLiteral(equals(Value))),
                           hasFalseExpression(cxxBoolLiteral(equals(!Value))))
           .bind(TernaryId),
@@ -442,13 +442,13 @@
 void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder,
                                                   bool Value, StringRef Id) {
   if (ChainedConditionalReturn)
-    Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+    Finder->addMatcher(ifStmt(unless(isInTemplateInstantiation()),
                               hasThen(returnsBool(Value, ThenLiteralId)),
                               hasElse(returnsBool(!Value)))
                            .bind(Id),
                        this);
   else
-    Finder->addMatcher(ifStmt(isExpansionInMainFile(),
+    Finder->addMatcher(ifStmt(unless(isInTemplateInstantiation()),
                               unless(hasParent(ifStmt())),
                               hasThen(returnsBool(Value, ThenLiteralId)),
                               hasElse(returnsBool(!Value)))
@@ -474,12 +474,16 @@
   auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
                                              hasAnySubstatement(SimpleElse)));
   if (ChainedConditionalAssignment)
-    Finder->addMatcher(ifStmt(hasThen(Then), hasElse(Else)).bind(Id), this);
+    Finder->addMatcher(ifStmt(unless(isInTemplateInstantiation()),
+                              hasThen(Then), hasElse(Else))
+                           .bind(Id),
+                       this);
   else
-    Finder->addMatcher(
-        ifStmt(unless(hasParent(ifStmt())), hasThen(Then), hasElse(Else))
-            .bind(Id),
-        this);
+    Finder->addMatcher(ifStmt(unless(isInTemplateInstantiation()),
+                              unless(hasParent(ifStmt())), hasThen(Then),
+                              hasElse(Else))
+                           .bind(Id),
+                       this);
 }
 
 void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
@@ -487,6 +491,7 @@
                                                           StringRef Id) {
   Finder->addMatcher(
       compoundStmt(
+          unless(isInTemplateInstantiation()),
           hasAnySubstatement(
               ifStmt(hasThen(returnsBool(Value)), unless(hasElse(stmt())))),
           hasAnySubstatement(returnStmt(has(ignoringParenImpCasts(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81336.269033.patch
Type: text/x-patch
Size: 4089 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200606/4cfd89d3/attachment.bin>


More information about the cfe-commits mailing list