[PATCH] D46834: [Sema][Cxx17] Error message for C++17 static_assert(pred) without string literal

[Jan Korous via Phabricator via cfe-commits]

jkorous created this revision.
jkorous added reviewers: arphaman, dexonsmith, rsmith, doug.gregor.
jkorous added a project: clang.
Herald added a subscriber: cfe-commits.

C++17 adds form of static_assert without string literal.

  static_assert ( bool_constexpr )

Currently clang produces diagnostics like this:

  assert.cpp:1:1: error: static_assert failed
  static_assert(false);
  ^             ~~~~~
  1 error generated.

This patch adds assert expression to error message in case string literal is missing:

  /Users/jkorous/assert.cpp:1:1: error: static_assert failed "false"
  static_assert(false);
  ^             ~~~~~
  1 error generated.

Reasons why printing assert expression in absence of string literal is useful are:

1. Serialized diagnostics (--serialize-diagnostics) don't contain code snippet and thus error message lack context.
2. No all tools scraping clang diagnostics use caret snippet provided by -f-caret-diagnostics.

This patch doesn't address message length limit as currently clang doesn't truncate messages anyway but it might be subject of another patch.

rdar://problem/38931241


Repository:
  rC Clang

https://reviews.llvm.org/D46834

Files:
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCXX/static-assert-cxx17.cpp
  test/SemaCXX/static-assert.cpp


Index: test/SemaCXX/static-assert.cpp
===================================================================
--- test/SemaCXX/static-assert.cpp
+++ test/SemaCXX/static-assert.cpp
@@ -50,7 +50,7 @@
 StaticAssertProtected<X> sap2; // expected-note {{instantiation}}
 
 static_assert(true); // expected-warning {{C++17 extension}}
-static_assert(false); // expected-error-re {{failed{{$}}}} expected-warning {{extension}}
+static_assert(false); // expected-error {{static_assert failed "false"}} expected-warning {{C++17 extension}}
 
 
 // Diagnostics for static_assert with multiple conditions
Index: test/SemaCXX/static-assert-cxx17.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/static-assert-cxx17.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+
+static_assert(false, "foo"); // expected-error {{static_assert failed "foo"}}
+static_assert(false == true); // expected-error {{static_assert failed "false == true"}}
+static_assert(false, "foo \"bar\" baz"); // expected-error {{static_assert failed "foo \"bar\" baz"}}
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -13736,9 +13736,16 @@
 
     if (!Failed && !Cond) {
       SmallString<256> MsgBuffer;
-      llvm::raw_svector_ostream Msg(MsgBuffer);
-      if (AssertMessage)
-        AssertMessage->printPretty(Msg, nullptr, getPrintingPolicy());
+      {
+        llvm::raw_svector_ostream Msg(MsgBuffer);
+        if (AssertMessage)
+          AssertMessage->printPretty(Msg, nullptr, getPrintingPolicy());
+        else {
+          AssertExpr->printPretty(Msg, nullptr, getPrintingPolicy());
+          MsgBuffer.insert(MsgBuffer.begin(), '\"');
+          MsgBuffer.append(1, '\"');
+        }
+      }
 
       Expr *InnerCond = nullptr;
       std::string InnerCondDescription;
@@ -13747,11 +13754,12 @@
                                    /*AllowTopLevelCond=*/false);
       if (InnerCond) {
         Diag(StaticAssertLoc, diag::err_static_assert_requirement_failed)
-          << InnerCondDescription << !AssertMessage
-          << Msg.str() << InnerCond->getSourceRange();
+          << InnerCondDescription << MsgBuffer.empty() << MsgBuffer.str()
+          << InnerCond->getSourceRange();
       } else {
         Diag(StaticAssertLoc, diag::err_static_assert_failed)
-          << !AssertMessage << Msg.str() << AssertExpr->getSourceRange();
+          << MsgBuffer.empty() << MsgBuffer.str()
+          << AssertExpr->getSourceRange();
       }
       Failed = true;
     }
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -819,7 +819,10 @@
 
 /// ParseStaticAssertDeclaration - Parse C++0x or C11 static_assert-declaration.
 ///
-/// [C++0x] static_assert-declaration:
+/// [C++17] static_assert-declaration:
+///           static_assert ( constant-expression  ) ;
+///
+/// [C++11] static_assert-declaration:
 ///           static_assert ( constant-expression  ,  string-literal  ) ;
 ///
 /// [C11]   static_assert-declaration:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46834.146615.patch
Type: text/x-patch
Size: 3217 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180514/ebcbdea9/attachment.bin>