[PATCH] D105759: [WIP] Implement P2361 Unevaluated string literals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/Expr.h:1845
   StringRef getString() const {
-    assert(getCharByteWidth() == 1 &&
+    assert((isUnevaluated() || getCharByteWidth() == 1) &&
            "This function is used in places that assume strings use char");
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Do we also want to assert that if it is unevaluated, it's char byte width *is* one byte? (No such thing as a multibyte unevaluated string literal.)
> This test is there because unevaluated strings don't have bytes at all! (trying to call `getCharByteWidth()` on them would assert)
Ah, good point!


================
Comment at: clang/lib/Lex/Pragma.cpp:807
   if (Tok.is(tok::string_literal) && !Tok.hasUDSuffix()) {
     StringLiteralParser Literal(Tok, PP);
     if (Literal.hadError)
----------------
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.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:374-376
+  ParsedAttr::Kind AttrKind =
+      ParsedAttr::getParsedKind(AttrName, ScopeName, Syntax);
+
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > I don't think this needed to move?
> We use attrKind in the else close after
Derp, my eyes when crossed, I thought the scope was still fine. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759