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 {{evaluates to ''?' == '#''}}
+// expected-error@13 {{failed}} expected-warning@13 {{trigraph ignored}} 
expected-note@13 {{evaluates to '63 == 35'}}
 // expected-error@16 {{}}
----------------
cor3ntin wrote:
> tahonermann wrote:
> > aaron.ballman wrote:
> > > I think the original diagnostic was actually more understandable as it 
> > > relates more closely to what's written in the static assertion. I could 
> > > imagine something like `evaluates to '?' (63) == '#' (35)` would also be 
> > > reasonable.
> > I agree. I would also be ok with printing the integer value as primary with 
> > the character as secondary:
> >   evaluates to 63 ('?') == 35 ('#')
> > 
> > There are two kinds of non-printable characters:
> >   # Control characters (including new-line)
> >   # character values that don't correspond to a character (e.g., lone 
> > trailing characters or invalid code unit values).
> > For the first case, I would support printing them as either C escapes or 
> > universal-character-names. e.g.,
> >   evaluates to 0 ('\0') == 1 (\u0001)
> > For the second case, I would support printing them as C hex escapes. e.g, 
> >   evaluates to -128 ('\x80') == -123 ('\x85')
> > 
> > 
> > For the first case, I would support printing them as either C escapes or 
> > universal-character-names. e.g.,
> 
> As mentioned before, we should be consistent with what we do for diagnostics 
> messages in general - (`ie `pushEscapedString`).
> I check and we already do that. https://godbolt.org/z/doah9YGMT
> 
> Question is, why do we sometimes don't?
> Note that in general i don't have an opinion about displaying the value of 
> characters literal _in addition_ of the character itself, it seems like a 
> good thing)
The character escape in the current error message is handled in 
`CharacterLiteral::print` 
(https://github.com/llvm/llvm-project/blob/fcb6a9c07cf7a2bc63d364e3b7f60aaadadd57cc/clang/lib/AST/Expr.cpp#L1064-L1083),
 and the reason for `'\0'` being escaped to `\x00` and `'\u{9}'` escaped to 
`\t` is that `escapeCStyle` does not escape null character 
(https://github.com/llvm/llvm-project/blob/c1c86f9eae73786bcdacddaab248817c4f176935/clang/include/clang/Basic/CharInfo.h#L174-L200).
`pushEscapedString` does not escape whitespace characters, so we should use 
`escapeCStyle` if we are to use c-style escape for them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155610

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to