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

2021-02-22 Thread Nathan James via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5616c5b8664b: [clang] Tweaked fixit for static assert with 
no message (authored by njames93).

Changed prior to commit:
  https://reviews.llvm.org/D89065?vs=325452=325474#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/FixIt/fixit-static-assert.cpp


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ 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}:", \"\""
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+isa(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 @@
 
   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);


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ 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}:", \"\""
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = 

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

2021-02-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/test/FixIt/fixit-static-assert.cpp:2-3
+// 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 -Werror -fsyntax-only
+// RUN: %clang_cc1 -std=c++14 %s -fixit-recompile -fixit-to-temporary -Werror

njames93 wrote:
> aaron.ballman wrote:
> > Any reason not to pass `-verify=cxx17` and `// cxx17-no-diagnostics`?
> I don't know my way around clang testing framework that well.
> For the recompile test I think Werror has to be used as verify resulted in an 
> assertion.
Ah, okay, thanks for the explanation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2021-02-22 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 325452.
njames93 marked an inline comment as done.
njames93 added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/FixIt/fixit-static-assert.cpp


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-static-assert.cpp
@@ -0,0 +1,29 @@
+// 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}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+isa(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 @@
 
   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);


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-static-assert.cpp
@@ -0,0 +1,29 @@
+// 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}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+isa(BO->getRHS()->IgnoreImpCasts()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return 

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

2021-02-22 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done.
njames93 added inline comments.



Comment at: clang/test/FixIt/fixit-static-assert.cpp:2-3
+// 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 -Werror -fsyntax-only
+// RUN: %clang_cc1 -std=c++14 %s -fixit-recompile -fixit-to-temporary -Werror

aaron.ballman wrote:
> Any reason not to pass `-verify=cxx17` and `// cxx17-no-diagnostics`?
I don't know my way around clang testing framework that well.
For the recompile test I think Werror has to be used as verify resulted in an 
assertion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2021-02-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some nits.




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:861
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&





Comment at: clang/lib/Parse/ParseDeclCXX.cpp:863
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(BO->getRHS()->IgnoreImpCasts()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");





Comment at: clang/test/FixIt/fixit-static-assert.cpp:2-3
+// 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 -Werror -fsyntax-only
+// RUN: %clang_cc1 -std=c++14 %s -fixit-recompile -fixit-to-temporary -Werror

Any reason not to pass `-verify=cxx17` and `// cxx17-no-diagnostics`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2021-02-21 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 325287.
njames93 added a comment.

Add recompile test to ensure fix-its are correct


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/FixIt/fixit-static-assert.cpp


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ 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 -Werror -fsyntax-only
+// RUN: %clang_cc1 -std=c++14 %s -fixit-recompile -fixit-to-temporary -Werror
+
+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}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(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 @@
 
   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);


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ 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 -Werror -fsyntax-only
+// RUN: %clang_cc1 -std=c++14 %s -fixit-recompile -fixit-to-temporary -Werror
+
+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}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(BO->getRHS()->IgnoreImpCasts()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return 

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

2021-02-21 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 325284.
njames93 added a comment.

Add tests for fixits.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/FixIt/fixit-static-assert.cpp


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-static-assert.cpp
@@ -0,0 +1,26 @@
+// 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 -Werror -fsyntax-only
+
+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}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(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 @@
 
   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);


Index: clang/test/FixIt/fixit-static-assert.cpp
===
--- /dev/null
+++ clang/test/FixIt/fixit-static-assert.cpp
@@ -0,0 +1,26 @@
+// 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 -Werror -fsyntax-only
+
+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}:", \"\""
+
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -856,6 +856,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(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 @@
 
   

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

2021-02-21 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:874
 /// [C++0x] static_assert-declaration:
 ///   static_assert ( constant-expression  ,  string-literal  ) ;
 ///

aaron.ballman wrote:
> xbolva00 wrote:
> > Do we warn for:
> > 
> > static_assert(“my msg”)? 
> > 
> > I remember some long time ago bug in llvm itself -  assert(“my msg”) - 
> > condition expr was missing.
> We don't, but that would be a reasonable warning to add (not necessarily as 
> part of this patch).
There is a clang-tidy check, misc-static-assert, that will flag asserts like 
that, although it won't flag static asserts of that kind.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2021-02-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:874
 /// [C++0x] static_assert-declaration:
 ///   static_assert ( constant-expression  ,  string-literal  ) ;
 ///

xbolva00 wrote:
> Do we warn for:
> 
> static_assert(“my msg”)? 
> 
> I remember some long time ago bug in llvm itself -  assert(“my msg”) - 
> condition expr was missing.
We don't, but that would be a reasonable warning to add (not necessarily as 
part of this patch).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2021-02-20 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:874
 /// [C++0x] static_assert-declaration:
 ///   static_assert ( constant-expression  ,  string-literal  ) ;
 ///

Do we warn for:

static_assert(“my msg”)? 

I remember some long time ago bug in llvm itself -  assert(“my msg”) - 
condition expr was missing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2021-02-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D89065#2320031 , @njames93 wrote:

> I don't see any support for verifying fix-its in the test cases so unsure how 
> i could go about testing this change.

We usually test fix-its with `-fdiagnostics-parseable-fixits` as in the tests 
here: https://github.com/llvm/llvm-project/tree/main/clang/test/FixIt (not 
saying the test has to go into the `FixIt` directory, it should probably live 
in `test/Parser` given the changes.)

Btw, this is going to cause merge conflicts with 
https://reviews.llvm.org/D95396 and I sort of wonder if we should combine the 
two patches into one given that they're fixing the same code in different ways.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2021-02-19 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2020-11-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2020-10-19 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2020-10-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 297434.
njames93 added a comment.

FixIt now properly generated


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

Files:
  clang/lib/Parse/ParseDeclCXX.cpp


Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -858,6 +858,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(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:
@@ -894,12 +904,11 @@
 
   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);


Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -858,6 +858,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa(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:
@@ -894,12 +904,11 @@
 
   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);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-10-10 Thread Nathan James via Phabricator via cfe-commits
njames93 planned changes to this revision.
njames93 added a comment.

Just noticed this doesn't quite work as expected, will update


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2020-10-08 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 297072.
njames93 added a comment.

Remove unnecessary changes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

Files:
  clang/lib/Parse/ParseDeclCXX.cpp


Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -858,6 +858,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa_and_nonnull(BO->getRHS()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return FixItHint::CreateInsertion(EndExprLoc, ", \"\"");
+}
+
 /// ParseStaticAssertDeclaration - Parse C++0x or C11 
static_assert-declaration.
 ///
 /// [C++0x] static_assert-declaration:
@@ -894,12 +904,11 @@
 
   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);


Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -858,6 +858,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa_and_nonnull(BO->getRHS()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return FixItHint::CreateInsertion(EndExprLoc, ", \"\"");
+}
+
 /// ParseStaticAssertDeclaration - Parse C++0x or C11 static_assert-declaration.
 ///
 /// [C++0x] static_assert-declaration:
@@ -894,12 +904,11 @@
 
   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);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2020-10-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

I don't see any support for verifying fix-its in the test cases so unsure how i 
could go about testing this change.




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:18
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OperatorKinds.h"

I'll undo this, clangd include inserter and clang format making undesired 
changes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89065/new/

https://reviews.llvm.org/D89065

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


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

2020-10-08 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision.
njames93 added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
njames93 requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89065

Files:
  clang/lib/Parse/ParseDeclCXX.cpp


Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -10,15 +10,16 @@
 //
 
//===--===//
 
-#include "clang/Parse/Parser.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Parse/ParseDiagnostic.h"
+#include "clang/Parse/Parser.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/ParsedTemplate.h"
@@ -858,6 +859,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa_and_nonnull(BO->getRHS()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return FixItHint::CreateInsertion(EndExprLoc, ", \"\"");
+}
+
 /// ParseStaticAssertDeclaration - Parse C++0x or C11 
static_assert-declaration.
 ///
 /// [C++0x] static_assert-declaration:
@@ -894,12 +905,11 @@
 
   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);


Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -10,15 +10,16 @@
 //
 //===--===//
 
-#include "clang/Parse/Parser.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Parse/ParseDiagnostic.h"
+#include "clang/Parse/Parser.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/ParsedTemplate.h"
@@ -858,6 +859,16 @@
DeclFromDeclSpec);
 }
 
+static FixItHint getStaticAssertNoMessageFixIt(const Expr *AssertExpr,
+   SourceLocation EndExprLoc) {
+  if (const auto *BO = llvm::dyn_cast_or_null(AssertExpr)) {
+if (BO->getOpcode() == BO_LAnd &&
+llvm::isa_and_nonnull(BO->getRHS()))
+  return FixItHint::CreateReplacement(BO->getOperatorLoc(), ",");
+  }
+  return FixItHint::CreateInsertion(EndExprLoc, ", \"\"");
+}
+
 /// ParseStaticAssertDeclaration - Parse C++0x or C11 static_assert-declaration.
 ///
 /// [C++0x] static_assert-declaration:
@@ -894,12 +905,11 @@
 
   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);