[PATCH] D46027: [clang-tidy] Fix PR35824

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.
xazax.hun added a reviewer: alexfh.
Herald added subscribers: dkrupp, rnkovacs, baloghadamsoftware, whisperity.

This approach will also introduce false negatives.
A better approach would be to check if the null statement is the result of 
folding an `if constexpr`.
The current AST API does not expose this information.
A better fix might require AST patches on the clang side.

An alternative version for the current approach is to overload `isConstexpr` 
matcher to `IfStmt`s.

Which solution do you prefer?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46027

Files:
  clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
  test/clang-tidy/bugprone-suspicious-semicolon-constexpr.cpp


Index: test/clang-tidy/bugprone-suspicious-semicolon-constexpr.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-suspicious-semicolon-constexpr.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-semicolon %t -- -- -std=c++17
+
+void fail()
+{
+  int x = 0;
+  if(x > 5); (void)x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: potentially unintended 
semicolon [bugprone-suspicious-semicolon]
+  // CHECK-FIXES: if(x > 5) (void)x;
+}
+
+template <int X>
+int foo(int a) {
+    if constexpr(X > 0) {
+        return a;
+    }
+    return a + 1;
+}
+
+int main(void) {
+    return foo<0>(1);
+}
Index: clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
===================================================================
--- clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
+++ clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
@@ -45,12 +45,15 @@
   unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
 
   const auto *Statement = Result.Nodes.getNodeAs<Stmt>("stmt");
-  const bool IsIfStmt = isa<IfStmt>(Statement);
+  const auto *IfNode = dyn_cast<IfStmt>(Statement);
 
-  if (!IsIfStmt &&
+  if (!IfNode &&
       SM.getSpellingLineNumber(Token.getLocation()) != SemicolonLine)
     return;
 
+  if (IfNode && IfNode->isConstexpr())
+    return;
+
   SourceLocation LocEnd = Semicolon->getLocEnd();
   FileID FID = SM.getFileID(LocEnd);
   llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, LocEnd);
@@ -64,7 +67,7 @@
   unsigned NewTokenIndent = SM.getSpellingColumnNumber(Token.getLocation());
   unsigned NewTokenLine = SM.getSpellingLineNumber(Token.getLocation());
 
-  if (!IsIfStmt && NewTokenIndent <= BaseIndent &&
+  if (!IfNode && NewTokenIndent <= BaseIndent &&
       Token.getKind() != tok::l_brace && NewTokenLine != SemicolonLine)
     return;
 



Index: test/clang-tidy/bugprone-suspicious-semicolon-constexpr.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-suspicious-semicolon-constexpr.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-semicolon %t -- -- -std=c++17
+
+void fail()
+{
+  int x = 0;
+  if(x > 5); (void)x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: potentially unintended semicolon [bugprone-suspicious-semicolon]
+  // CHECK-FIXES: if(x > 5) (void)x;
+}
+
+template <int X>
+int foo(int a) {
+    if constexpr(X > 0) {
+        return a;
+    }
+    return a + 1;
+}
+
+int main(void) {
+    return foo<0>(1);
+}
Index: clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
===================================================================
--- clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
+++ clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
@@ -45,12 +45,15 @@
   unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
 
   const auto *Statement = Result.Nodes.getNodeAs<Stmt>("stmt");
-  const bool IsIfStmt = isa<IfStmt>(Statement);
+  const auto *IfNode = dyn_cast<IfStmt>(Statement);
 
-  if (!IsIfStmt &&
+  if (!IfNode &&
       SM.getSpellingLineNumber(Token.getLocation()) != SemicolonLine)
     return;
 
+  if (IfNode && IfNode->isConstexpr())
+    return;
+
   SourceLocation LocEnd = Semicolon->getLocEnd();
   FileID FID = SM.getFileID(LocEnd);
   llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, LocEnd);
@@ -64,7 +67,7 @@
   unsigned NewTokenIndent = SM.getSpellingColumnNumber(Token.getLocation());
   unsigned NewTokenLine = SM.getSpellingLineNumber(Token.getLocation());
 
-  if (!IsIfStmt && NewTokenIndent <= BaseIndent &&
+  if (!IfNode && NewTokenIndent <= BaseIndent &&
       Token.getKind() != tok::l_brace && NewTokenLine != SemicolonLine)
     return;
 

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