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

[Corentin Jabot via Phabricator via cfe-commits]

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: clang/lib/Sema/SemaDeclCXX.cpp:16839
+
+static void PrintCharLiteralPrefix(BuiltinType::Kind BTK,
+                                   llvm::raw_ostream &OS) {
----------------
We have similar switches in
* `StringLiteral::outputString` 
* `TryPrintAsStringLiteral` (APValue.cpp)
* `CharacterLiteral::print`

Sadly they all look at different things so I don't know if we could refactor all of that.
But looking further done, I don;t think we should print a prefix here, so we could remove that bit entirely. 


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16899
+          uint32_t CodePoint = static_cast<uint32_t>(V.getInt().getZExtValue());
+          PrintCharLiteralPrefix(BTy->getKind(), OS);
+          OS << '\'';
----------------
Looking at the diagnostics, I don't think it makes sense to print a prefix here. You could just leave that part out.


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

https://reviews.llvm.org/D155610