erichkeane added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:257
+
+def err_unevaluated_string_prefix : Error<
+ "an unevaluated string literal cannot have an encoding prefix">;
----------------
Is there value to combining these two diagnostics with a %select?
================
Comment at: clang/lib/Lex/LiteralSupport.cpp:108
+ unsigned CharWidth, DiagnosticsEngine *Diags,
+ const LangOptions &Features, bool Unevaluated) {
const char *EscapeBegin = ThisTokBuf;
----------------
This is like the 3rd time we're using 'Unevaluated' as a bool parameter. I
have a pretty strong preference for making it a scoped-enum in 'Basic'
somewhere.
================
Comment at: clang/lib/Lex/LiteralSupport.cpp:1813
+ Diags->Report(TokLoc, UnevaluatedStringHasUDL
+ ? diag::err_unevaluated_string_udl
+ : diag::err_string_concat_mixed_suffix)
----------------
Is this OK? It looks like we're passing a ton of parameters to a diag type
that doesn't have any wildcards?
================
Comment at: clang/lib/Lex/LiteralSupport.cpp:95-96
+ case '?':
+ case 'n':
+ case 't':
+ return true;
----------------
aaron.ballman wrote:
> aaron.ballman wrote:
> > cor3ntin wrote:
> > > aaron.ballman wrote:
> > > > Do you intend to miss a bunch of escapes like `\'` and `\r` (etc)?
> > > \' is there. I am less sure about '\r' and '\a'. for example. This is
> > > something I realized after writing P2361.
> > > what does '\a` in static assert mean? even '\r' is not so obvious
> > Looking at the list again, I think only `\a` is really of interest here. I
> > know some folks like @jfb have mentioned that `\a` could be used to
> > generate an alert sound on a terminal, which is a somewhat useful feature
> > for a failed static assertion if you squint at it hard enough.
> >
> > But the rest of the missing ones do seem more questionable to support.
> @jfb and @cor3ntin -- any opinions on whether `\a` should be supported? My
> opinion is that it should be supported because it has some utility for anyone
> running the compiler from a command line, but it's a pretty weak opinion.
I might consider rejecting ANY character escape in the less-than-32 part of the
table.
For consistency at least, I don't see value in allowing \a if we're rejecting
layout things like \t.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105759/new/
https://reviews.llvm.org/D105759
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
