[clang] Print more information about failed static assertions

[tbaeder]

Summary

For

static constexpr m = 10;
static_assert(m == 11);

the output is before:

./test.cpp:2:1: error: static_assert failed due to requirement 'm == 11'
static_assert(m == 11);
^             ~~~~~~~
1 error generated.

and after:

./test.cpp:2:1: error: static assertion failed due to requirement 'm == 11'
static_assert(m == 11);
^             ~~~~~~~
./test.cpp:2:15: note: left-hand side of operator '==' evaluates to '10'
static_assert(m == 11);
              ^
1 error generated.

The patch adds a new function Sema::DiagnoseStaticAssertDetails(), which currently only handles binary operators with an integer literal on one side and a non-integer literal on the other side. This is intentionally kept simple, but can be improved incrementally over time of course.

[tbaeder]

clang/lib/Sema/SemaDeclCXX.cpp

16654

Oops, this needs to go.

[tbaeder]

Oh, I noticed that g++ already does this:

test.cpp:27:21: error: static assertion failed
   27 | static_assert(foo() < 4);
      |               ~~~~~~^~~
test.cpp:27:21: note: the comparison reduces to ‘(10 < 4)’

[aaron.ballman]

Wow, I *really* like this improvement, thank you for working on it! This is going to be especially helpful for things like static_assert(foo() == 12, "oh no, foo wasn't 12!");. Can you also add a release note for this improvement?

clang/include/clang/Basic/DiagnosticSemaKinds.td

1536

Should probably add at least one test that shows the "right-hand" version of the diagnostic.

clang/lib/Sema/SemaDeclCXX.cpp

16564–16571

I think we want a little bit more smarts here -- -2 is not an IntegerLiteral, it's a unary expression whose subexpression is an integer literal. I think we need to handle unary expressions because it'd be weird for foo() == 12 to get helpful information but foo() == -12 not to.

16579–16580

Similar issue here for unary expressions; we probably don't want 1 == -12 to get past this check.

16587–16590

This same logic seems correct for float, fixed point, and complex (though this one may need some extra work because of the real and imaginary parts). Any reason not to handle those as well?

[tbaeder]

I just took the plunge and made DiagnoseStaticAssertDetails() take into account all static assertions, regardless of integer literals. It now also prints integers, booleans, chars and floats correctly.

For example:

test.cpp:30:1: error: static assertion failed due to requirement 'inv(true) == inv(false)'
static_assert(inv(true) == inv(false));
^             ~~~~~~~~~~~~~~~~~~~~~~~
test.cpp:30:15: note: left-hand side of operator '==' evaluates to 'false'
static_assert(inv(true) == inv(false));
              ^~~~~~~~~
test.cpp:30:28: note: right-hand side of operator '==' evaluates to 'true'
static_assert(inv(true) == inv(false));
                           ^~~~~~~~~~

the output when both sides are printed is rather clumsy to me, and some things are not handled at all (e.g. failed static assertions when the toplevel expression is not a BinaryOpertator, e.g. static_assert(!k) - the user doesn't know what value k is, which is important in case k is of an integer type).

But again, this kind of begs people to suggest more and more improvements, but I think this is a good enough start :)

[tbaeder]

clang/lib/Sema/SemaDeclCXX.cpp

16574

I really hope there's a better way to do this that I just don't know about :)

[tbaeder]

Whatever, I had time and you're still asleep, so I updated the output:

test.cpp:30:1: error: static assertion failed due to requirement 'inv(true) == inv(false)'
static_assert(inv(true) == inv(false));
^             ~~~~~~~~~~~~~~~~~~~~~~~
test.cpp:30:25: note: expression evaluates to false == true
static_assert(inv(true) == inv(false));
              ~~~~~~~~~~^~~~~~~~~~~~~
1 error generated.

[tbaeder]

Also print nullptr expressions.

[tbaeder]

FWIW, complex numbers are already covered it seems:

test.cpp:35:1: error: static assertion failed due to requirement '__real c == __imag c'
static_assert(__real c == __imag c);
^             ~~~~~~~~~~~~~~~~~~~~
test.cpp:35:24: note: expression evaluates to 5 == 6
static_assert(__real c == __imag c);
              ~~~~~~~~~^~~~~~~~~~~
1 error generated.

[tbaeder]

Ignore what I said, they are supported now though.

[xbolva00]

In D130894#3696534, @tbaeder wrote:

FWIW, complex numbers are already covered it seems:

test.cpp:35:1: error: static assertion failed due to requirement '__real c == __imag c'
static_assert(__real c == __imag c);
^             ~~~~~~~~~~~~~~~~~~~~
test.cpp:35:24: note: expression evaluates to 5 == 6
static_assert(__real c == __imag c);
              ~~~~~~~~~^~~~~~~~~~~
1 error generated.

Use ‘5 ==6’ ? So add quotes ..

[aaron.ballman]

In D130894#3696590, @xbolva00 wrote:

Use ‘5 ==6’ ? So add quotes ..

+1 to the suggestion to use quotes for a bit of visual distinction between the diagnostic message and the code embedded within it.

clang/lib/Sema/SemaDeclCXX.cpp

16573–16584

Thankfully, there is a better way:

llvm::raw_svector_ostream OS(Str);
OS << (BoolValue ? "true" : "false");

which applies to the other cases in this function as well.

16647–16649

FixedPointLiteral? ImaginaryLiteral?

16658

16672

[tbaeder]

+1 to the suggestion to use quotes for a bit of visual distinction between the diagnostic message and the code embedded within it.

One problem is that both the integer value 0 and the character constant '0' are printed as '0' (same for all other single-digit numbers). gcc's output doesn't have that problem because it prints chars as integers, but I don't really like that.

[aaron.ballman]

In D130894#3698552, @tbaeder wrote:

+1 to the suggestion to use quotes for a bit of visual distinction between the diagnostic message and the code embedded within it.

One problem is that both the integer value 0 and the character constant '0' are printed as '0' (same for all other single-digit numbers). gcc's output doesn't have that problem because it prints chars as integers, but I don't really like that.

Agreed on not printing chars as integers, but would printing ''0'' in that case to make it more "clear" that it's a character constant? I think we'd want that for something like static_assert('a' < 0, "what?") so it's not printed as 'a < 0'.

[tbaeder]

IMO it's clear enough...

test.cpp:24:1: error: static assertion failed due to requirement 'c != c'
static_assert(c != c);
^             ~~~~~~
test.cpp:24:17: note: expression evaluates to 'a != a'
static_assert(c != c);
              ~~^~~~
test.cpp:25:1: error: static assertion failed due to requirement 'c < 'a''
static_assert(c < 'a');
^             ~~~~~~~
test.cpp:25:15: note: left-hand side of operator '<' evaluates to 'a'
static_assert(c < 'a');
              ^
2 errors generated.

Printing the first note as ''a' != 'a'' doesn't make it clearer. But for

test.cpp:24:1: error: static assertion failed due to requirement 'c != c'
static_assert(c != c);
^             ~~~~~~
test.cpp:24:17: note: expression evaluates to '0 != 0'
static_assert(c != c);
              ~~^~~~
test.cpp:25:1: error: static assertion failed due to requirement 'c > 'a''
static_assert(c > 'a');
^             ~~~~~~~
test.cpp:25:15: note: left-hand side of operator '>' evaluates to '0'
static_assert(c > 'a');
              ^

where c is a char variable, the output looks kind of weird. "0 != 0" looks like we're comparing two integers.

We might also go the g++ route and use evaluates to ('0' != '0') for the full expression, but that looks weird for just one side: evaluates to (6). But we could also always print the full expression so we only have one case.

[xbolva00]

personally I dont see huge value to print “ left-hand side of operator '==' evaluates to …”

I would like to see the full expression.

+1 to just copy gcc’s approach

[tbaeder]

For the record, the output now looks something like this:

test.cpp:24:1: error: static assertion failed due to requirement 'c != c'
static_assert(c != c);
^             ~~~~~~
test.cpp:24:17: note: expression evaluates to '('0' != '0')'
static_assert(c != c);
              ~~^~~~
test.cpp:25:1: error: static assertion failed due to requirement 'c > 'a''
static_assert(c > 'a');
^             ~~~~~~~
test.cpp:25:17: note: expression evaluates to '('0' > 'a')'
static_assert(c > 'a');
              ~~^~~~~
2 errors generated.

[xbolva00]

looks great

[aaron.ballman]

In D130894#3701749, @tbaeder wrote:

For the record, the output now looks something like this:

test.cpp:24:1: error: static assertion failed due to requirement 'c != c'
static_assert(c != c);
^             ~~~~~~
test.cpp:24:17: note: expression evaluates to '('0' != '0')'
static_assert(c != c);
              ~~^~~~

This looks hard to read to me...

test.cpp:25:1: error: static assertion failed due to requirement 'c > 'a''
static_assert(c > 'a');
^ ~~~~~~~
test.cpp:25:17: note: expression evaluates to '('0' > 'a')'

Same confusion here -- it took me a while to realize what's happening is that we're quoting the part outside of the parens and that's throwing me for a loop. We typically quote things that are syntax but in this case, the parens are not part of the syntax and so the surrounding quotes are catching me. Given that parens are being used as a visual delimiter and a single quote serves the same purpose, I would expect something more like:

test.cpp:25:17: note: expression evaluates to ''0' > 'a''
test.cpp:26:17: note: expression evaluates to '0 > 'a''
test.cpp:27:17: note: expression evaluates to '14 > 5'
test.cpp:28:17: note: expression evaluates to 'true == false'
test.cpp:29:17: note: expression evaluates to 'nullptr != nullptr'

Adding a few more folks as reviewers to see if there is a consensus position on how to proceed.

[xbolva00]

or

test.cpp:25:17: note: evaluated expression: '0' > 'a'

[tbaeder]

I don't know much about fixit hints, would they be appropriate to display it below the line itself, e.g.

test.cpp:24:17: note: expression evaluates to
static_assert(c != c);
            '0' != '0'
              ~~^~~~

Or would it be even better to just inline the evaluated expression into the static_assert ala

test.cpp:24:17: note: expression evaluates to
static_assert('0' != '0');
                ~~^~~~

[aaron.ballman]

In D130894#3702233, @tbaeder wrote:

I don't know much about fixit hints, would they be appropriate to display it below the line itself, e.g.

test.cpp:24:17: note: expression evaluates to
static_assert(c != c);
            '0' != '0'
              ~~^~~~

Nope, that would be a bad thing -- Clang has the ability to automatically apply fix-it hints to the source, so this would modify the static assertion to continue to be false which isn't much of a fix.

Or would it be even better to just inline the evaluated expression into the static_assert ala

test.cpp:24:17: note: expression evaluates to
static_assert('0' != '0');
                ~~^~~~

Making sure I'm following along, you mean that we'd display the error for the static assertion, and then the note would show static_assert(<actual values in the reconstituted expression>)?

I think this might work for very simple static asserts, but what about more complicated ones like static_assert(foo() && (bar() == 12 || baz()) && 'a' != quux(4000));?

[xbolva00]

Any tests with macros?

[cjdb]

Thank you so much for working on this! It's been on my todo list for a while and just haven't gotten around to it.

In D130894#3702166, @aaron.ballman wrote:

In D130894#3701749, @tbaeder wrote:

For the record, the output now looks something like this:

test.cpp:24:1: error: static assertion failed due to requirement 'c != c'
static_assert(c != c);
^             ~~~~~~
test.cpp:24:17: note: expression evaluates to '('0' != '0')'
static_assert(c != c);
              ~~^~~~

This looks hard to read to me...

test.cpp:25:1: error: static assertion failed due to requirement 'c > 'a''
static_assert(c > 'a');
^ ~~~~~~~
test.cpp:25:17: note: expression evaluates to '('0' > 'a')'

Same confusion here -- it took me a while to realize what's happening is that we're quoting the part outside of the parens and that's throwing me for a loop. We typically quote things that are syntax but in this case, the parens are not part of the syntax and so the surrounding quotes are catching me. Given that parens are being used as a visual delimiter and a single quote serves the same purpose, I would expect something more like:

test.cpp:25:17: note: expression evaluates to ''0' > 'a''
test.cpp:26:17: note: expression evaluates to '0 > 'a''
test.cpp:27:17: note: expression evaluates to '14 > 5'
test.cpp:28:17: note: expression evaluates to 'true == false'
test.cpp:29:17: note: expression evaluates to 'nullptr != nullptr'

Adding a few more folks as reviewers to see if there is a consensus position on how to proceed.

Agreed on confusion, but ''0' > 'a'' is painful to read. Sadly, short of changing quotes to backticks (something that'd be good for non-terminal-based consumers of SARIF), I'm not sure I can suggest anything better in one line. If we can span multiple lines, which I think would improve readability:

test.cpp:25:17: note: expression evaluates with:
  LHS: '0'
  RHS: 'a'
test.cpp:26:17: note: expression evaluates with:
  LHS: 0
  RHS: 'a'
test.cpp:27:17: note: expression evaluates with:
  LHS: 14
  RHS: 5
test.cpp:28:17: note: expression evaluates with:
  LHS: true
  RHS: false
test.cpp:29:17: note: expression evaluates with:
  LHS: nullptr
  RHS: nullptr

My assertion library follows what Catch2 and simple-test do, and uses Actual and Expected, but I'm not sure hardcoding those terms into Clang is a good idea.

[tbaeder]

I don't really want to bike-shed about the presentation for too long... I'm fine with just removing the parens, since we present it like that in the error message as well anyway:

./assert.cpp:6:1: error: static assertion failed due to requirement ''c' == 'a''
static_assert('c' == 'a')

Any tests with macros?

I can add some, but they should be handled transparently, with the usual "expanded from macro" note.

[aaron.ballman]

In D130894#3709027, @tbaeder wrote:

I don't really want to bike-shed about the presentation for too long...

I understand the desire, but at the same time, the whole goal of this patch is to improve the presentation of the diagnostic. You can't invite us all to a bikeshed painting party and then ask us not to use our brushes, that's just cruel! :-D

I'm fine with just removing the parens, since we present it like that in the error message as well anyway:

./assert.cpp:6:1: error: static assertion failed due to requirement ''c' == 'a''
static_assert('c' == 'a')

That's still my preference as well. Splitting across multiple lines with hard line breaks has some issues for some IDEs that want to display diagnostic information in a listbox (IIRC), so I'd rather we avoid ad hoc use of line breaks in diagnostics for the moment (it'd be easier/more appropriate if we were making a diagnostic policy for using multiple lines, but that'd involve an RFC and is more effort than I think is needed for this patch).

Any tests with macros?

I can add some, but they should be handled transparently, with the usual "expanded from macro" note.

I'm fine either way.

[cjdb]

In D130894#3709431, @aaron.ballman wrote:

In D130894#3709027, @tbaeder wrote:

I don't really want to bike-shed about the presentation for too long...

I understand the desire, but at the same time, the whole goal of this patch is to improve the presentation of the diagnostic. You can't invite us all to a bikeshed painting party and then ask us not to use our brushes, that's just cruel! :-D

Agreed. Getting the presentation of diagnostics right is critical.

I'm fine with just removing the parens, since we present it like that in the error message as well anyway:

./assert.cpp:6:1: error: static assertion failed due to requirement ''c' == 'a''
static_assert('c' == 'a')

That's still my preference as well. Splitting across multiple lines with hard line breaks has some issues for some IDEs that want to display diagnostic information in a listbox (IIRC), so I'd rather we avoid ad hoc use of line breaks in diagnostics for the moment (it'd be easier/more appropriate if we were making a diagnostic policy for using multiple lines, but that'd involve an RFC and is more effort than I think is needed for this patch).

Fair enough, no further objections on my part.

Any tests with macros?

I can add some, but they should be handled transparently, with the usual "expanded from macro" note.

I'm fine either way.

[aaron.ballman]

LGTM, thank you for working on this, it's a great usability enhancement!

[mstorsjo]

This broke building with GCC (also noted by buildbot mails):

../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
../tools/clang/lib/Sema/SemaDeclCXX.cpp:16666:19: error: declaration of ‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)::<unnamed struct>::Expr’ changes meaning of ‘Expr’ [-fpermissive]
16666 |       const Expr *Expr;
      |                   ^~~~
In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
                 from ../tools/clang/include/clang/AST/ASTLambda.h:18,
                 from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here as ‘class clang::Expr’
  109 | class Expr : public ValueStmt {
      |       ^~~~

[xbolva00]

In D130894#3715124, @mstorsjo wrote:

This broke building with GCC (also noted by buildbot mails):

../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
../tools/clang/lib/Sema/SemaDeclCXX.cpp:16666:19: error: declaration of ‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)::<unnamed struct>::Expr’ changes meaning of ‘Expr’ [-fpermissive]
16666 |       const Expr *Expr;
      |                   ^~~~
In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
                 from ../tools/clang/include/clang/AST/ASTLambda.h:18,
                 from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here as ‘class clang::Expr’
  109 | class Expr : public ValueStmt {
      |       ^~~~

It is kinda bad that GCC throws an error and Clang does not even print a warning.

How is it even possible? Clang does not implement strict(er) rules ? @aaron.ballman

[aaron.ballman]

In D130894#3715143, @xbolva00 wrote:

In D130894#3715124, @mstorsjo wrote:

This broke building with GCC (also noted by buildbot mails):

../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
../tools/clang/lib/Sema/SemaDeclCXX.cpp:16666:19: error: declaration of ‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)::<unnamed struct>::Expr’ changes meaning of ‘Expr’ [-fpermissive]
16666 |       const Expr *Expr;
      |                   ^~~~
In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
                 from ../tools/clang/include/clang/AST/ASTLambda.h:18,
                 from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here as ‘class clang::Expr’
  109 | class Expr : public ValueStmt {
      |       ^~~~

It is kinda bad that GCC throws an error and Clang does not even print a warning.

How is it even possible? Clang does not implement strict(er) rules ? @aaron.ballman

I pushed up a change to "fix" this, but I believe GCC is wrong to give an error here. At least, nobody else gives one and GCC is inconsistent about what it diagnoses: https://godbolt.org/z/hTqM8MPch

[mstorsjo]

In D130894#3715709, @aaron.ballman wrote:

In D130894#3715143, @xbolva00 wrote:

In D130894#3715124, @mstorsjo wrote:

This broke building with GCC (also noted by buildbot mails):

../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
../tools/clang/lib/Sema/SemaDeclCXX.cpp:16666:19: error: declaration of ‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)::<unnamed struct>::Expr’ changes meaning of ‘Expr’ [-fpermissive]
16666 |       const Expr *Expr;
      |                   ^~~~
In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
                 from ../tools/clang/include/clang/AST/ASTLambda.h:18,
                 from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here as ‘class clang::Expr’
  109 | class Expr : public ValueStmt {
      |       ^~~~

It is kinda bad that GCC throws an error and Clang does not even print a warning.

How is it even possible? Clang does not implement strict(er) rules ? @aaron.ballman

I pushed up a change to "fix" this

FWIW, the same issue actually already was fixed by ebe0674acb8bb3404d0e2a6b689d5e3cd02bb0b6 - although your commit made it even clearer.

[aaron.ballman]

In D130894#3715725, @mstorsjo wrote:

In D130894#3715709, @aaron.ballman wrote:

In D130894#3715143, @xbolva00 wrote:

In D130894#3715124, @mstorsjo wrote:

This broke building with GCC (also noted by buildbot mails):

../tools/clang/lib/Sema/SemaDeclCXX.cpp: In member function ‘void clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)’:
../tools/clang/lib/Sema/SemaDeclCXX.cpp:16666:19: error: declaration of ‘const clang::Expr* clang::Sema::DiagnoseStaticAssertDetails(const clang::Expr*)::<unnamed struct>::Expr’ changes meaning of ‘Expr’ [-fpermissive]
16666 |       const Expr *Expr;
      |                   ^~~~
In file included from ../tools/clang/include/clang/AST/DeclCXX.h:22,
                 from ../tools/clang/include/clang/AST/ASTLambda.h:18,
                 from ../tools/clang/lib/Sema/SemaDeclCXX.cpp:15:
../tools/clang/include/clang/AST/Expr.h:109:7: note: ‘Expr’ declared here as ‘class clang::Expr’
  109 | class Expr : public ValueStmt {
      |       ^~~~

It is kinda bad that GCC throws an error and Clang does not even print a warning.

How is it even possible? Clang does not implement strict(er) rules ? @aaron.ballman

I pushed up a change to "fix" this

FWIW, the same issue actually already was fixed by ebe0674acb8bb3404d0e2a6b689d5e3cd02bb0b6 - although your commit made it even clearer.

Oh neat, thanks for pointing that out! I reflexively rename declarations when they share the name of a type because I have run into similar issues to this before, but it's usually things like IDE "go to definition" behavior that winds up breaking, not the build. (I run into this all the time with people doing const Attr *Attr)

[xbolva00]

Btw,

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1697r0.html

[aaron.ballman]

In D130894#3715884, @xbolva00 wrote:

Btw,

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1697r0.html

Yup, that paper is currently closed in the WG21 paper tracker FWIW.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 // C++ declarations
 def err_static_assert_expression_is_not_constant : Error<
   "static assertion expression is not an integral constant expression">;
 def err_constexpr_if_condition_expression_is_not_constant : Error<
   "constexpr if condition is not a constant expression">;
 def err_static_assert_failed : Error<"static assertion failed%select{: %1|}0">;
 def err_static_assert_requirement_failed : Error<
   "static assertion failed due to requirement '%0'%select{: %2|}1">;
+def note_bin_op_evaluates : Note<
+  "%select{left-hand|right-hand}0 side of operator '%1' evaluates to '%2'">;
 
 def warn_consteval_if_always_true : Warning<
   "consteval if is always true in an %select{unevaluated|immediate}0 context">,
   InGroup<DiagGroup<"redundant-consteval-if">>;
 
 def ext_inline_variable : ExtWarn<
   "inline variables are a C++17 extension">, InGroup<CXX17>;
 def warn_cxx14_compat_inline_variable : Warning<

clang/include/clang/Sema/Sema.h

Decl *ActOnStaticAssertDeclaration(SourceLocation StaticAssertLoc,
                                      Expr *AssertExpr,
                                      Expr *AssertMessageExpr,
                                      SourceLocation RParenLoc);
   Decl *BuildStaticAssertDeclaration(SourceLocation StaticAssertLoc,
                                      Expr *AssertExpr,
                                      StringLiteral *AssertMessageExpr,
                                      SourceLocation RParenLoc,
                                      bool Failed);
+  void DiagnoseStaticAssertDetails(const Expr *E);
 
   FriendDecl *CheckFriendTypeDecl(SourceLocation LocStart,
                                   SourceLocation FriendLoc,
                                   TypeSourceInfo *TSInfo);
   Decl *ActOnFriendTypeDecl(Scope *S, const DeclSpec &DS,
                             MultiTemplateParamsArg TemplateParams);
   NamedDecl *ActOnFriendFunctionDecl(Scope *S, Declarator &D,
                                      MultiTemplateParamsArg TemplateParams);

clang/lib/Sema/SemaDeclCXX.cpp

Decl *Sema::ActOnStaticAssertDeclaration(SourceLocation StaticAssertLoc,
 
   if (DiagnoseUnexpandedParameterPack(AssertExpr, UPPC_StaticAssertExpression))
     return nullptr;
 
   return BuildStaticAssertDeclaration(StaticAssertLoc, AssertExpr,
                                       AssertMessage, RParenLoc, false);
 }
 
+/// Try to print more useful information about a failed static_assert
+/// with expression \E
+void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
+  if (const BinaryOperator *Op = dyn_cast_or_null<BinaryOperator>(E)) {
+    unsigned ExprIsRHS = true;
+    const Expr *ExprSide;
+
+    if (isa<IntegerLiteral>(Op->getRHS()->IgnoreParenImpCasts())) {
+      ExprIsRHS = false;
+      ExprSide = Op->getLHS();
+    } else if (isa<IntegerLiteral>(Op->getLHS()->IgnoreParenImpCasts())) {
+      ExprIsRHS = true;
+      ExprSide = Op->getRHS();
+    } else
+      return;
+
+    // We already print enough info about these earlier,
+    // e.g. "requirement '2 == 1' failed" where 2 is
+    // a substituted template parameter.
+    if (isa<SubstNonTypeTemplateParmExpr>(ExprSide))
+      return;
+
+    if (isa<IntegerLiteral>(ExprSide))
+      return;
+
+    // Evaluate the expression again
+    Expr::EvalResult EvalResult;
+    SmallVector<PartialDiagnosticAt, 0> Notes;
+    EvalResult.Diag = &Notes;
+    ExprSide->EvaluateAsRValue(EvalResult, Context, true);
+
+    SmallString<12> ValueString;
+    if (EvalResult.Val.isInt())
+      EvalResult.Val.getInt().toString(ValueString);
+    else
+      return;
+
+    Diag(ExprSide->getExprLoc(), diag::note_bin_op_evaluates)
+        << (unsigned)ExprIsRHS << Op->getOpcodeStr() << ValueString;
+  }
+}
+
 Decl *Sema::BuildStaticAssertDeclaration(SourceLocation StaticAssertLoc,
                                          Expr *AssertExpr,
                                          StringLiteral *AssertMessage,
                                          SourceLocation RParenLoc,
                                          bool Failed) {
   assert(AssertExpr != nullptr && "Expected non-null condition");
   if (!AssertExpr->isTypeDependent() && !AssertExpr->isValueDependent() &&
       !Failed) {
if (!Failed && !Cond) {
 
       Expr *InnerCond = nullptr;
       std::string InnerCondDescription;
       std::tie(InnerCond, InnerCondDescription) =
         findFailedBooleanCondition(Converted.get());
       if (InnerCond && isa<ConceptSpecializationExpr>(InnerCond)) {
         // Drill down into concept specialization expressions to see why they
         // weren't satisfied.
         Diag(StaticAssertLoc, diag::err_static_assert_failed)
           << !AssertMessage << Msg.str() << AssertExpr->getSourceRange();
         ConstraintSatisfaction Satisfaction;
         if (!CheckConstraintSatisfaction(InnerCond, Satisfaction))
           DiagnoseUnsatisfiedConstraint(Satisfaction);
       } else if (InnerCond && !isa<CXXBoolLiteralExpr>(InnerCond)
                            && !isa<IntegerLiteral>(InnerCond)) {
         Diag(StaticAssertLoc, diag::err_static_assert_requirement_failed)
           << InnerCondDescription << !AssertMessage
           << Msg.str() << InnerCond->getSourceRange();
+        DiagnoseStaticAssertDetails(InnerCond);
       } else {
         Diag(StaticAssertLoc, diag::err_static_assert_failed)
           << !AssertMessage << Msg.str() << AssertExpr->getSourceRange();
       }
       Failed = true;
     }
   } else {
     ExprResult FullAssertExpr = ActOnFinishFullExpr(AssertExpr, StaticAssertLoc,
                                                     /*DiscardedValue*/false,
                                                     /*IsConstexpr*/true);
     if (FullAssertExpr.isInvalid())
       Failed = true;
     else
       AssertExpr = FullAssertExpr.get();
   }
 
   Decl *Decl = StaticAssertDecl::Create(Context, CurContext, StaticAssertLoc,
                                         AssertExpr, AssertMessage, RParenLoc,
                                         Failed);
 
   CurContext->addDecl(Decl);
   return Decl;
 }

clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp

template<typename T> void f() {
     extern int arr1[];
     extern T arr1;
     extern T arr2;
     extern int arr2[];
     static_assert(sizeof(arr1) == 12, "");
     static_assert(sizeof(arr2) == 12, "");
 
     // Use a failing test to ensure the type isn't considered dependent.
-    static_assert(sizeof(arr2) == 13, ""); // expected-error {{failed}}
+    static_assert(sizeof(arr2) == 13, ""); // expected-error {{failed}} \
+                                           // expected-note {{evaluates to '12'}}
   }
 
   void g() { f<int[3]>(); } // expected-note {{in instantiation of}}
 
   template<typename T> void h1() {
     extern T arr3;
     {
       int arr3;

clang/test/CXX/drs/dr7xx.cpp

#endif
     static_assert(B<1>().u<0> == 2, "");
     static_assert(B<0>().u<1> == 3, "");
 
     static_assert(B<1>().v<1> == 1, "");
     static_assert(B<1>().v<0> == 2, "");
     static_assert(B<0>().v<1> == 3, "");
     static_assert(B<0>().v<0> == 4, "");
 #if __cplusplus < 201702L
-    // expected-error@-2 {{failed}}
+    // expected-error@-2 {{failed}} \
+    // expected-note@-2 {{left-hand side of operator '==' evaluates to '2'}}
 #endif
 
     static_assert(B<1>().w<1> == 1, "");
     static_assert(B<1>().w<0> == 2, "");
     static_assert(B<0>().w<1> == 3, "");
     static_assert(B<0>().w<0> == 4, "");
 #endif
   }

clang/test/CXX/temp/temp.decls/temp.variadic/init-capture.cpp

template<typename ...T> void f(T ...t) {
   weird((void)[&... x = t] {
     return T() + &x; // expected-error {{unexpanded parameter pack 'x'}}
   }...               // expected-error {{does not contain any unexpanded}}
   );
 }
 
 template<int ...a> constexpr auto x = [...z = a] (auto F) { return F(z...); };
 static_assert(x<1,2,3>([](int a, int b, int c) { return 100 * a + 10 * b + c; }) == 123);
-static_assert(x<1,2,3>([](int a, int b, int c) { return 100 * a + 10 * b + c; }) == 124); // expected-error {{failed}}
+static_assert(x<1,2,3>([](int a, int b, int c) { return 100 * a + 10 * b + c; }) == 124); // expected-error {{failed}} \
+                                                                                          // expected-note {{evaluates to '123'}}
 
 template<int ...a> constexpr auto y = [z = a...] (auto F) { return F(z...); }; // expected-error {{must appear before the name of the capture}}
 static_assert(y<1,2,3>([](int a, int b, int c) { return 100 * a + 10 * b + c; }) == 123);
-static_assert(y<1,2,3>([](int a, int b, int c) { return 100 * a + 10 * b + c; }) == 124); // expected-error {{failed}}
+static_assert(y<1,2,3>([](int a, int b, int c) { return 100 * a + 10 * b + c; }) == 124); // expected-error {{failed}} \
+                                                                                          // expected-note {{evaluates to '123'}}

clang/test/PCH/cxx-templates.cpp

namespace ClassScopeExplicitSpecializations {
   static_assert(A<4>().f<1>() == 1, "");
 }
 
 namespace DependentMemberExpr {
 #ifndef NO_ERRORS
   // This used to mark 'f' invalid without producing any diagnostic. That's a
   // little hard to detect, but we can make sure that constexpr evaluation
   // fails when it should.
-  static_assert(A<int>().f() == 1); // expected-error {{static assertion failed}}
+  static_assert(A<int>().f() == 1); // expected-error {{static assertion failed}} \
+                                    // expected-note {{left-hand side of operator '==' evaluates to '0'}}
 #endif
 }
 
 namespace DependentTemplateName {
   struct HasMember {
     template <class T> struct Member;
   };
 

clang/test/Parser/objc-static-assert.mm

@interface A {
   static_assert(1, "");
   _Static_assert(1, "");
 
   static_assert(0, ""); // expected-error {{static assertion failed}}
   _Static_assert(0, ""); // expected-error {{static assertion failed}}
 
   static_assert(a, ""); // expected-error {{static assertion expression is not an integral constant expression}}
   static_assert(sizeof(a) == 4, "");
-  static_assert(sizeof(a) == 3, ""); // expected-error {{static assertion failed}}
+  static_assert(sizeof(a) == 3, ""); // expected-error {{static assertion failed}} \
+                                     // expected-note {{evaluates to '4'}}
 }
 
 static_assert(1, "");
 _Static_assert(1, "");
 
 - (void)f;
 @end
 
 @implementation A {
   int b;
   static_assert(1, "");
   _Static_assert(1, "");
   static_assert(sizeof(b) == 4, "");
-  static_assert(sizeof(b) == 3, ""); // expected-error {{static assertion failed}}
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static assertion failed}} \
+                                     // expected-note {{evaluates to '4'}}
 }
 
 static_assert(1, "");
 
 - (void)f {
   static_assert(1, "");
 }
 @end
 
 @interface B
 @end
 
 @interface B () {
   int b;
   static_assert(sizeof(b) == 4, "");
-  static_assert(sizeof(b) == 3, ""); // expected-error {{static assertion failed}}
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static assertion failed}} \
+                                     // expected-note {{evaluates to '4'}}
 }
 @end
 
 #else
 
 #if __has_feature(objc_cxx_static_assert)
 #error failed
 #endif

clang/test/SemaCXX/static-assert-cxx17.cpp

void foo6() {
   // expected-error@-1{{static assertion failed due to requirement 'X<int>{1, 2}'}}
   static_assert(X<typename T::T>({1, 2}));
   // expected-error@-1{{static assertion failed due to requirement 'X<int>({1, 2})'}}
   static_assert(typename T::T{0});
   // expected-error@-1{{static assertion failed due to requirement 'int{0}'}}
   static_assert(typename T::T(0));
   // expected-error@-1{{static assertion failed due to requirement 'int(0)'}}
   static_assert(sizeof(X<typename T::T>) == 0);
-  // expected-error@-1{{static assertion failed due to requirement 'sizeof(X<int>) == 0'}}
+  // expected-error@-1{{static assertion failed due to requirement 'sizeof(X<int>) == 0'}} \
+  // expected-note@-1 {{evaluates to '8'}}
   static_assert((const X<typename T::T> *)nullptr);
   // expected-error@-1{{static assertion failed due to requirement '(const X<int> *)nullptr'}}
   static_assert(static_cast<const X<typename T::T> *>(nullptr));
   // expected-error@-1{{static assertion failed due to requirement 'static_cast<const X<int> *>(nullptr)'}}
   static_assert((const X<typename T::T>[]){} == nullptr);
   // expected-error@-1{{static assertion failed due to requirement '(const X<int>[0]){} == nullptr'}}
   static_assert(sizeof(X<decltype(X<typename T::T>().X<typename T::T>::~X())>) == 0);
-  // expected-error@-1{{static assertion failed due to requirement 'sizeof(X<void>) == 0'}}
+  // expected-error@-1{{static assertion failed due to requirement 'sizeof(X<void>) == 0'}} \
+  // expected-note@-1 {{evaluates to '8'}}
   static_assert(constexpr_return_false<typename T::T, typename T::U>());
   // expected-error@-1{{static assertion failed due to requirement 'constexpr_return_false<int, float>()'}}
 }
 template void foo6<ExampleTypes>();
 // expected-note@-1{{in instantiation of function template specialization 'foo6<ExampleTypes>' requested here}}

clang/test/SemaTemplate/instantiate-var-template.cpp

namespace InstantiationDependent {
 
   template<int> constexpr int a = 1;
   template<typename T> constexpr T b = a<sizeof(sizeof(f(T())))>; // expected-error {{invalid application of 'sizeof' to an incomplete type 'void'}}
 
   static_assert(b<int> == 1, "");
   static_assert(b<char> == 1, ""); // expected-note {{in instantiation of}} expected-error {{not an integral constant}}
 
   template<typename T> void f() {
-    static_assert(a<sizeof(sizeof(f(T())))> == 0, ""); // expected-error {{static assertion failed due to requirement 'a<sizeof (sizeof (f(type-parameter-0-0())))> == 0'}}
+    static_assert(a<sizeof(sizeof(f(T())))> == 0, ""); // expected-error {{static assertion failed due to requirement 'a<sizeof (sizeof (f(type-parameter-0-0())))> == 0'}} \
+                                                       // expected-note {{evaluates to '1'}}
   }
 }
 
 namespace PR24483 {
   template<typename> struct A;
   template<typename... T> A<T...> models;
   template<> struct B models<>; // expected-error {{incomplete type 'struct B'}} expected-note {{forward declaration}}
 }
 
 namespace InvalidInsertPos {
   template<typename T, int N> T v;
   template<int N> decltype(v<int, N-1>) v<int, N>;
   template<> int v<int, 0>;
   int k = v<int, 500>;
 }

clang/test/SemaTemplate/instantiation-dependence.cpp

namespace PR46791 { // also PR45782
   };
 
   struct A {};
   struct B { typedef int value_type; };
   struct C { typedef int element_type; };
   struct D : B, C {};
 
   static_assert(trait<A>::specialization == 0);
-  static_assert(trait<B>::specialization == 1); // FIXME expected-error {{failed}}
-  static_assert(trait<C>::specialization == 2); // FIXME expected-error {{failed}}
+  static_assert(trait<B>::specialization == 1); // FIXME expected-error {{failed}} \
+                                                // expected-note {{evaluates to '0'}}
+  static_assert(trait<C>::specialization == 2); // FIXME expected-error {{failed}} \
+                                                // expected-note {{evaluates to '0'}}
   static_assert(trait<D>::specialization == 0); // FIXME-error {{ambiguous partial specialization}}
 }
 
 namespace TypeQualifier {
   // Ensure that we substitute into an instantiation-dependent but
   // non-dependent qualifier.
   template<int> struct A { using type = int; };
   template<typename T> A<sizeof(sizeof(T::error))>::type f() {} // expected-note {{'int' cannot be used prior to '::'}}