[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

cor3ntin added a comment. This is starting to look pretty good! I'm happy with the general direction, my only concern is that printing a prefix does not seem useful - we are trying to display the value, not how it was produced. Comment at:

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

hazohelet updated this revision to Diff 546782. hazohelet added a comment. Address comments from Corentin - Use default `pushEscapedString` escaping (``) instead of UCN representation `\u0001` - Convert multi-byte characters (`wchar_t`, `char16_t`, `char32_t`) to UTF-8 and prints them. - Added

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

hazohelet added a comment. In D155610#4550346 , @cor3ntin wrote: > I've been thinking about it and I think I have a cleaner design for the > printing of characters: > > We need a `CharToString(unsigned, Qualtype) -> SmallString` method that takes > a

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

cor3ntin added a comment. I've been thinking about it and I think I have a cleaner design for the printing of characters: We need a `CharToString(unsigned, Qualtype) -> SmallString` method that takes a value and the type. for `char` and `char8_t` we can just return the value. For `wchar_t`,

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

cor3ntin added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:838-858 + if (UseUCN) +OutStream << "\\u" + << llvm::format_hex_no_prefix(CodepointValue, /*Width=*/4, +/*Upper=*/false); +

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

hazohelet added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:838-858 + if (UseUCN) +OutStream << "\\u" + << llvm::format_hex_no_prefix(CodepointValue, /*Width=*/4, +/*Upper=*/false); +

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

cor3ntin added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:838-858 + if (UseUCN) +OutStream << "\\u" + << llvm::format_hex_no_prefix(CodepointValue, /*Width=*/4, +/*Upper=*/false); +

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

hazohelet updated this revision to Diff 545582. hazohelet added a comment. Address review comments - Print the character representation only when the type of the expressions is `char` or `char8_t` - Use `pushEscapedString` in the printing so that we can reuse its escaping logic - Use

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

hazohelet added a comment. Thanks everyone for the comments! Comment at: clang/test/Lexer/cxx1z-trigraphs.cpp:24 // expected-error@11 {{}} expected-warning@11 {{trigraph ignored}} -// expected-error@13 {{failed}} expected-warning@13 {{trigraph ignored}} expected-note@13

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

cor3ntin added inline comments. Comment at: clang/test/Lexer/cxx1z-trigraphs.cpp:24 // expected-error@11 {{}} expected-warning@11 {{trigraph ignored}} -// expected-error@13 {{failed}} expected-warning@13 {{trigraph ignored}} expected-note@13 {{evaluates to ''?' == '#''}} +//

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

cjdb added a comment. I think Tom's suggestion about using escapes and UCNs is great. I have no real opinion on whether the numeric values is the one that's parenthesised. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155610/new/

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

tahonermann added inline comments. Comment at: clang/test/Lexer/cxx1z-trigraphs.cpp:24 // expected-error@11 {{}} expected-warning@11 {{trigraph ignored}} -// expected-error@13 {{failed}} expected-warning@13 {{trigraph ignored}} expected-note@13 {{evaluates to ''?' == '#''}}

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

hazohelet added a comment. In D155610#4511547 , @tbaeder wrote: > What if you switch from `IgnoreParenImpCasts` at the top of > `DiagnoseStaticAssertDetails()` to just `IgnoreParens()`? That improve cases > where we simply compare a character literal

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

tbaeder added a comment. I think the discussion is getting derailed a bit. The original reason I was talking to Takuya about this is this: https://godbolt.org/z/GjsYrexT3 static_assert('a' == 100); For code like this, we print the it as `''a' == 100'`, but the AST contains a cast from the

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

cor3ntin added a comment. I agree with Aaron that in the current state, the common case diagnostics are made worse. But there is room for improvement! I think what we want to do here is modify `ConvertAPValueToString` so that it applies the same escape logic to `char` as we do in

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

tbaeder added a comment. What if you switch from `IgnoreParenImpCasts` at the top of `DiagnoseStaticAssertDetails()` to just `IgnoreParens()`? That improve cases where we simply compare a character literal to an integer, since the literal should be implicitly cast to an integer. Repository:

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

aaron.ballman added reviewers: cor3ntin, tahonermann. aaron.ballman added a comment. I wonder if we want to see whether the character is printable before deciding whether to show the numeric value for it or show a character value? For whitespace characters, printing the character leads to odd

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

hazohelet created this revision. hazohelet added reviewers: aaron.ballman, tbaeder, cjdb. Herald added a project: All. hazohelet requested review of this revision. Herald added a project: clang. BEFORE this patch, the values printed in `static_assert` were printed after ignoring the implicit