[PATCH] D105759: Implement P2361 Unevaluated string literals

Mon Jun 26 19:42:28 PDT 2023

ChuanqiXu added inline comments.

================
Comment at: clang/lib/Lex/Pragma.cpp:807
   if (Tok.is(tok::string_literal) && !Tok.hasUDSuffix()) {
     StringLiteralParser Literal(Tok, PP);
     if (Literal.hadError)
----------------
aaron.ballman wrote:
> aaron.ballman wrote:
> > cor3ntin wrote:
> > > aaron.ballman wrote:
> > > > Should this also be modified?
> > > Probably but because I'm not super familiar with module map things I preferred being conservative
> > Paging @rsmith for opinions.
> > 
> > Lacking those opinions, I think being conservative here is fine.
> Pinging @ChuanqiXu for opinions.
I think the both options (to modify it or not) are acceptable. Because the input here  should be the output of the clang itself. See https://github.com/llvm/llvm-project/blob/ebd0b8a0472b865b7eb6e1a32af97ae31d829033/clang/lib/Basic/Module.cpp#L229-L231 and https://github.com/llvm/llvm-project/blob/ebd0b8a0472b865b7eb6e1a32af97ae31d829033/clang/lib/Frontend/Rewrite/FrontendActions.cpp#L238-L240.

We can see there is no deprecated prefix. So while it is acceptable to modify this since its pattern matches the paper, it doesn't matter really since we can control the input completely.

Personally, I prefer to not touch it. Since I feel like this use case doesn't have been used a lot. So the effort here may not be worthy.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759