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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
