erichkeane added a comment.
A couple of small things, otherwise I'm happy; but Aaron has some bigger opens
above, plus clang-format, plus the modules from Richard.
================
Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:257
+
+def err_unevaluated_string_prefix : Error<
+ "an unevaluated string literal cannot have an encoding prefix">;
----------------
aaron.ballman wrote:
> erichkeane wrote:
> > Is there value to combining these two diagnostics with a %select?
> I waffled when doing this review, so it's funny you mention it. :-D
>
> We could do: `an unevaluated string literal cannot %select{have an encoding
> prefix|be a user-defined literal}0` but there was just enough text in the
> `select` that I felt it wasn't critical to combine. But I don't feel strongly
> either way.
I was waffly on this too, so your waffling + my waffling I think is sufficient
reason to not deal with this now.
================
Comment at: clang/lib/AST/Expr.cpp:1083
+
+ if (Kind != StringKind::Unevaluated) {
+ assert(Ctx.getAsConstantArrayType(Ty) &&
----------------
minor preference (perhaps 'nit' level) to move this whole CharByteWidth +
IsPascal calculation into its own function. This constructor is absurdly long
as it is.
================
Comment at: clang/lib/Lex/LiteralSupport.cpp:98
+ case 'r':
+ return true;
+ }
----------------
For future clarification, the ones from the 'simple' list here:
https://en.cppreference.com/w/cpp/language/escape
that we are missing are: `\a` `\b` `\f` and `\v`.
I personally think I'm ok with that until someone else says they care.
================
Comment at: clang/lib/Lex/LiteralSupport.cpp:1541
+ Kind = tok::string_literal;
+ for (const auto &Tok : StringToks) {
+ // Unevaluated string literals can never have a prefix
----------------
aaron.ballman wrote:
> This means we're looping over (almost) all the string tokens three times --
> once here, once below on line 1562, and again on 1605.
Hrm.... this is unfortunate. Is there no way to combine the loops? I guess
(hope?) that hte list of tokens is at least going to be short...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105759/new/
https://reviews.llvm.org/D105759
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
