-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang] Inject tokens containing #embed back into token stream #97274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4d5008f
d2406e0
f34179d
1603695
e9c0253
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1018,6 +1018,7 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, | |
|
||
// primary-expression | ||
case tok::numeric_constant: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you adapt the couple other places where we parse a numeric constant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please elaborate which other places you meant? I briefly examined all mentions of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ping @cor3ntin There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These cases [[clang::availability(
#embed "smth.txt"
)]] void f();
struct S {
virtual void f() =
#embed "zero.bin"
;
}; However, it's a bit ambiguous whether these should be supported at all in the proposed C++ wording for embed. I sent a mail to the C++ committee It's probably fine to ignore that for now, the patch is a great improvement as it is |
||
case tok::binary_data: | ||
// constant: integer-constant | ||
// constant: floating-constant | ||
|
||
|
@@ -1067,18 +1068,9 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, | |
} | ||
|
||
case tok::annot_embed: { | ||
// We've met #embed in a context where a single value is expected. Take last | ||
// element from #embed data as if it were a comma expression. | ||
EmbedAnnotationData *Data = | ||
reinterpret_cast<EmbedAnnotationData *>(Tok.getAnnotationValue()); | ||
SourceLocation StartLoc = ConsumeAnnotationToken(); | ||
ASTContext &Context = Actions.getASTContext(); | ||
Res = IntegerLiteral::Create(Context, | ||
llvm::APInt(CHAR_BIT, Data->BinaryData.back()), | ||
Context.UnsignedCharTy, StartLoc); | ||
if (Data->BinaryData.size() > 1) | ||
Diag(StartLoc, diag::warn_unused_comma_left_operand); | ||
break; | ||
injectEmbedTokens(); | ||
return ParseCastExpression(ParseKind, isAddressOfOperand, isTypeCast, | ||
isVectorLiteral, NotPrimaryExpression); | ||
} | ||
|
||
case tok::kw___super: | ||
|
@@ -3578,15 +3570,29 @@ ExprResult Parser::ParseFoldExpression(ExprResult LHS, | |
T.getCloseLocation()); | ||
} | ||
|
||
void Parser::ExpandEmbedDirective(SmallVectorImpl<Expr *> &Exprs) { | ||
void Parser::injectEmbedTokens() { | ||
EmbedAnnotationData *Data = | ||
reinterpret_cast<EmbedAnnotationData *>(Tok.getAnnotationValue()); | ||
SourceLocation StartLoc = ConsumeAnnotationToken(); | ||
ASTContext &Context = Actions.getASTContext(); | ||
for (auto Byte : Data->BinaryData) { | ||
Exprs.push_back(IntegerLiteral::Create(Context, llvm::APInt(CHAR_BIT, Byte), | ||
Context.UnsignedCharTy, StartLoc)); | ||
MutableArrayRef<Token> Toks(PP.getPreprocessorAllocator().Allocate<Token>( | ||
Data->BinaryData.size() * 2 - 1), | ||
Data->BinaryData.size() * 2 - 1); | ||
unsigned I = 0; | ||
for (auto &Byte : Data->BinaryData) { | ||
Toks[I].startToken(); | ||
Toks[I].setKind(tok::binary_data); | ||
Toks[I].setLocation(Tok.getLocation()); | ||
Toks[I].setLength(1); | ||
Toks[I].setLiteralData(&Byte); | ||
if (I != ((Data->BinaryData.size() - 1) * 2)) { | ||
Toks[I + 1].startToken(); | ||
Toks[I + 1].setKind(tok::comma); | ||
Toks[I + 1].setLocation(Tok.getLocation()); | ||
} | ||
I += 2; | ||
} | ||
PP.EnterTokenStream(std::move(Toks), /*DisableMacroExpansion=*/true, | ||
/*IsReinject=*/false); | ||
ConsumeAnyToken(/*ConsumeCodeCompletionTok=*/true); | ||
} | ||
|
||
/// ParseExpressionList - Used for C/C++ (argument-)expression-list. | ||
|
@@ -3624,17 +3630,8 @@ bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs, | |
if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace)) { | ||
Diag(Tok, diag::warn_cxx98_compat_generalized_initializer_lists); | ||
Expr = ParseBraceInitializer(); | ||
} else if (Tok.is(tok::annot_embed)) { | ||
ExpandEmbedDirective(Exprs); | ||
if (Tok.isNot(tok::comma)) | ||
break; | ||
Token Comma = Tok; | ||
ConsumeToken(); | ||
checkPotentialAngleBracketDelimiter(Comma); | ||
continue; | ||
} else { | ||
} else | ||
Expr = ParseAssignmentExpression(); | ||
} | ||
|
||
if (EarlyTypoCorrection) | ||
Expr = Actions.CorrectDelayedTyposInExpr(Expr); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both the documentation and name of this function is incorrect now that the underlying functionality has changed - would you mind updating it to something more appropriate @Fznamznon? It used to return the character itself:
But now returns the value of the character.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the comment and the interface in 295e4f4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @AaronBallman
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AaronBallman! Would it be worth also changing
Spelling
toValue
or something like that?