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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

(Somehow I missed Richard's comment when I replied last night, even though it preceded mine by 5 hours...)

In https://reviews.llvm.org/D46834#1098727, @rsmith wrote:

> This would violate our guidelines for diagnostic messages; see https://clang.llvm.org/docs/InternalsManual.html#the-format-string
>
> In particular: "Take advantage of location information. The user will be able to see the line and location of the caret, so you don’t need to tell them that the problem is with the 4th argument to the function: just point to it."
>
> > 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. Not all tools scraping clang diagnostics use caret snippet provided by -fcaret-diagnostics.
>
> This seems like an excellent reason to fix those tools -- per our documentation, Clang's diagnostics are not designed to be used without the caret and snippet.
>
> If you want to change our diagnostic policy from "the diagnostic text plus the line and location of the caret should be enough to identify the problem; do not repeat information in the diagnostic that the caret portion shows" to "the diagnostic text alone should be sufficient", that's certainly something we can discuss, but you'll need to make the case for why that's a good change of policy.


I wasn't aware of the policy.  I can't convince myself that `static_assert` should be an exception to it.

It's ironic, though.  What I'm concerned about are interfaces like a Vim location list, a condensed view of diagnostics that won't show the source until you select a particular diagnostic.  The status quo may lead some users to leverage a macro like this to hack the location list view:

  #define MY_STATIC_ASSERT(X) static_assert(X, #X)

getting rid of which was the motivation for the single-argument `static_assert` in the first place.


Repository:
  rC Clang

https://reviews.llvm.org/D46834