hubert.reinterpretcast added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16970
+ OS << '\'';
+ WriteCharValueForDiagnostic(CodeUnit, BTy, TyWidth, OS);
+ OS << "' (0x"
----------------
To have the diagnostic printer handle separating any potential grapheme (if it
is capable of doing so--potentially in the future), we need to isolate the
result of `WriteCharValueForDiagnostic` in a separate message substitution.
================
Comment at: clang/test/SemaCXX/static-assert.cpp:287
+ static_assert((char16_t)L'ゆ' == L"C̵̭̯̠̎͌ͅť̺"[1], ""); // expected-error
{{failed}} \
+ // expected-note {{evaluates
to ''ゆ' (0x3086) == '̵' (0x335)'}}
+ static_assert(L"＼／"[1] == u'\xFFFD', ""); // expected-error {{failed}} \
----------------
tahonermann wrote:
> cor3ntin wrote:
> > hubert.reinterpretcast wrote:
> > > hubert.reinterpretcast wrote:
> > > > cor3ntin wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > The C++23 escaped string formatting facility would not generate a
> > > > > > trailing combining character like this. I recommend following suit.
> > > > > >
> > > > > > Info on U+0335:
> > > > > > https://util.unicode.org/UnicodeJsps/character.jsp?a=0335
> > > > > >
> > > > > This is way outside the scope of the patch. The diagnostic output
> > > > > facility has no understanding of combining characters or graphemes
> > > > > and do not attempt to match std::print. It probably would be an
> > > > > improvement but this patch is not trying to modify how all
> > > > > diagnostics are printed. (all of that logic is in Diagnostic.cpp)
> > > > This patch is pushing the envelope of what appears in diagnostics. One
> > > > can also argue that someone writing
> > > > ```
> > > > static_assert(false, "\u0301");
> > > > ```
> > > > gets what they deserve, but that case does not have a big problem
> > > > anyway (because the provided message text appears after `: `).
> > > >
> > > > This patch increases the exposure of the diagnostic output facility to
> > > > input that it does not handle well. I disagree that it is outside the
> > > > scope of this patch to insist that it does not generate such inputs to
> > > > the diagnostic output facility (even if a possible solution is to
> > > > modify the diagnostic output facility first).
> > > @cor3ntin, do you have status quo examples for how grapheme-extending
> > > characters that are not already "problematic" in their original context
> > > are emitted in diagnostics in contexts where they are?
> > Are you looking for that sort of examples? https://godbolt.org/z/c79xWr7Me
> > That shows that clang has no understanding of graphemes
> gcc and MSVC get that case "right" (probably by accident).
> https://godbolt.org/z/Tjd6xnEon
I was more looking for cases where the output grapheme includes elements that
were part of the fixed message text (gobbling quotes, etc.). Also, this patch
is in the wrong shape for handling this concern in the diagnostic printer
because the delimiting of the replacement text happens in this patch.
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
