[clang] 5616c5b - [clang] Tweaked fixit for static assert with no message

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-02-22T17:43:53Z
New Revision: 5616c5b8664b54671e699e5c45178f11cbb680b3

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

LOG: [clang] Tweaked fixit for static assert with no message

If a static assert has a message as the right side of an and condition, suggest a fix it of replacing the '&&' to ','.

`static_assert(cond && "Failed Cond")` -> `static_assert(cond, "Failed cond")`

This use case comes up when lazily replacing asserts with static asserts.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D89065

Added: 
    clang/test/FixIt/fixit-static-assert.cpp

Modified: 
    clang/lib/Parse/ParseDeclCXX.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 811a999418a0..44c072b8987f 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@ Decl *Parser::ParseAliasDeclarationAfterDeclarator(
                                        DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+                                               SourceLocation EndExprLoc) {
+  if (const auto *BO = dyn_cast_or_null<BinaryOperator>(AssertExpr)) {
+    if (BO->getOpcode() == BO_LAnd &&
+        isa<StringLiteral>(BO->getRHS()->IgnoreImpCasts()))
+      return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return FixItHint::CreateInsertion(EndExprLoc, ", \"\"");
+}
+
 /// ParseStaticAssertDeclaration - Parse C++0x or C11 static_assert-declaration.
 ///
 /// [C++0x] static_assert-declaration:
@@ -892,12 +902,11 @@ Decl *Parser::ParseStaticAssertDeclaration(SourceLocation &DeclEnd){
 
   ExprResult AssertMessage;
   if (Tok.is(tok::r_paren)) {
-    Diag(Tok, getLangOpts().CPlusPlus17
-                  ? diag::warn_cxx14_compat_static_assert_no_message
-                  : diag::ext_static_assert_no_message)
-      << (getLangOpts().CPlusPlus17
-              ? FixItHint()
-              : FixItHint::CreateInsertion(Tok.getLocation(), ", \"\""));
+    if (getLangOpts().CPlusPlus17)
+      Diag(Tok, diag::warn_cxx14_compat_static_assert_no_message);
+    else
+      Diag(Tok, diag::ext_static_assert_no_message)
+          << getStaticAssertNoMessageFixIt(AssertExpr.get(), Tok.getLocation());
   } else {
     if (ExpectAndConsume(tok::comma)) {
       SkipUntil(tok::semi);

diff  --git a/clang/test/FixIt/fixit-static-assert.cpp b/clang/test/FixIt/fixit-static-assert.cpp
new file mode 100644
index 000000000000..ac07df4d1a58
--- /dev/null
+++ b/clang/test/FixIt/fixit-static-assert.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++14 %s -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// Ensure no warnings are emitted in c++17.
+// RUN: %clang_cc1 -std=c++17 %s -verify=cxx17
+// RUN: %clang_cc1 -std=c++14 %s -fixit-recompile -fixit-to-temporary -Werror
+
+// cxx17-no-diagnostics
+
+static_assert(true && "String");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+// String literal prefixes are good.
+static_assert(true && R"(RawString)");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+static_assert(true && L"RawString");
+// CHECK-DAG: {[[@LINE-1]]:20-[[@LINE-1]]:22}:","
+
+static_assert(true);
+// CHECK-DAG: {[[@LINE-1]]:19-[[@LINE-1]]:19}:", \"\""
+
+// While its technically possible to transform this to
+// static_assert(true, "String") we don't attempt this fix.
+static_assert("String" && true);
+// CHECK-DAG: {[[@LINE-1]]:31-[[@LINE-1]]:31}:", \"\""
+
+// Don't be smart and look in parentheses.
+static_assert((true && "String"));
+// CHECK-DAG: {[[@LINE-1]]:33-[[@LINE-1]]:33}:", \"\""