aaron.ballman added inline comments.

================
Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:290
+  "encoding prefix '%0' on an unevaluated string literal has no effect"
+  "%select{| and is incompatible with c++2c}1">,
+  InGroup<DiagGroup<"invalid-unevaluated-string">>;
----------------
cor3ntin wrote:
> hubert.reinterpretcast wrote:
> > aaron.ballman wrote:
> > > hubert.reinterpretcast wrote:
> > > > I am not seeing any tests covering C mode.
> > > > 
> > > > @aaron.ballman, can you confirm that you are okay with the warning when 
> > > > processing C code (and, moreover, the newly-added errors for numeric 
> > > > escape sequences)?
> > > > 
> > > Good call on C test coverage, thank you for bringing that up!
> > > 
> > > I think the behavior in C is reasonable, even if it's not mandated by the 
> > > standard. We're going from issuing an error in C to issuing a warning and 
> > > the warning text helpfully omits the C++2c part of the diagnostic, so it 
> > > seems like we're more relaxed in C than we previously were.
> > > 
> > > Do you have concerns about the behavior in C? (Or are you saying you'd 
> > > prefer this to be an error in C as it was before?)
> > My question was with respect to the change from the Clang 16 status quo for 
> > numeric escapes introduced by the prior change and carried forward with 
> > this one: https://godbolt.org/z/aneGr1Yfb
> > 
> > ```
> > _Static_assert(1, "\x30"); // error in Clang 17
> > ```
> > 
> Yes, this is intended, numeric escape sequences in unevaluated string 
> literals are not allowed in any language modes - the motivation for C is the 
> same as C++. 
Barring evidence that we're breaking working code without workarounds, I'm 
happy with the behavior in C. I don't think numeric escape sequences make a lot 
of sense in the contexts where these strings appear in C, same as for C++. 
However, if we had evidence that this breaks working code in problematic ways, 
we could consider changing it to warning-defaults-to-error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156596

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

Reply via email to