Revert r351209 (which was a revert of r350891) with a fix.

The test case had a parse error that was causing the condition string to be misreported. We now have better fallback code for error cases.

llvm-svn: 351470
This commit is contained in:
Aaron Ballman 2019-01-17 20:21:34 +00:00
parent 5f73474657
commit 65e96bdaca
4 changed files with 198 additions and 122 deletions

View File

@ -1816,8 +1816,8 @@ public:
void CheckEndOfDirective(const char *DirType, bool EnableMacros = false);
/// Read and discard all tokens remaining on the current line until
/// the tok::eod token is found.
void DiscardUntilEndOfDirective();
/// the tok::eod token is found. Returns the range of the skipped tokens.
SourceRange DiscardUntilEndOfDirective();
/// Returns true if the preprocessor has seen a use of
/// __DATE__ or __TIME__ in the file so far.
@ -1982,6 +1982,9 @@ private:
/// True if the expression contained identifiers that were undefined.
bool IncludedUndefinedIds;
/// The source range for the expression.
SourceRange ExprRange;
};
/// Evaluate an integer constant expression that may occur after a

View File

@ -76,18 +76,24 @@ Preprocessor::AllocateVisibilityMacroDirective(SourceLocation Loc,
bool isPublic) {
return new (BP) VisibilityMacroDirective(Loc, isPublic);
}
/// Read and discard all tokens remaining on the current line until
/// the tok::eod token is found.
void Preprocessor::DiscardUntilEndOfDirective() {
Token Tmp;
do {
LexUnexpandedToken(Tmp);
assert(Tmp.isNot(tok::eof) && "EOF seen while discarding directive tokens");
} while (Tmp.isNot(tok::eod));
}
/// Enumerates possible cases of #define/#undef a reserved identifier.
/// Read and discard all tokens remaining on the current line until
/// the tok::eod token is found.
SourceRange Preprocessor::DiscardUntilEndOfDirective() {
Token Tmp;
SourceRange Res;
LexUnexpandedToken(Tmp);
Res.setBegin(Tmp.getLocation());
while (Tmp.isNot(tok::eod)) {
assert(Tmp.isNot(tok::eof) && "EOF seen while discarding directive tokens");
LexUnexpandedToken(Tmp);
}
Res.setEnd(Tmp.getLocation());
return Res;
}
/// Enumerates possible cases of #define/#undef a reserved identifier.
enum MacroDiag {
MD_NoWarn, //> Not a reserved identifier
MD_KeywordDef, //> Macro hides keyword, enabled by default
@ -535,25 +541,25 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
// If this is in a skipping block or if we're already handled this #if
// block, don't bother parsing the condition.
if (CondInfo.WasSkipping || CondInfo.FoundNonSkip) {
DiscardUntilEndOfDirective();
} else {
const SourceLocation CondBegin = CurPPLexer->getSourceLocation();
// Restore the value of LexingRawMode so that identifiers are
// looked up, etc, inside the #elif expression.
assert(CurPPLexer->LexingRawMode && "We have to be skipping here!");
CurPPLexer->LexingRawMode = false;
IdentifierInfo *IfNDefMacro = nullptr;
const bool CondValue = EvaluateDirectiveExpression(IfNDefMacro).Conditional;
CurPPLexer->LexingRawMode = true;
if (Callbacks) {
const SourceLocation CondEnd = CurPPLexer->getSourceLocation();
Callbacks->Elif(Tok.getLocation(),
SourceRange(CondBegin, CondEnd),
(CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False), CondInfo.IfLoc);
}
// If this condition is true, enter it!
if (CondValue) {
if (CondInfo.WasSkipping || CondInfo.FoundNonSkip) {
DiscardUntilEndOfDirective();
} else {
// Restore the value of LexingRawMode so that identifiers are
// looked up, etc, inside the #elif expression.
assert(CurPPLexer->LexingRawMode && "We have to be skipping here!");
CurPPLexer->LexingRawMode = false;
IdentifierInfo *IfNDefMacro = nullptr;
DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);
const bool CondValue = DER.Conditional;
CurPPLexer->LexingRawMode = true;
if (Callbacks) {
Callbacks->Elif(
Tok.getLocation(), DER.ExprRange,
(CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False),
CondInfo.IfLoc);
}
// If this condition is true, enter it!
if (CondValue) {
CondInfo.FoundNonSkip = true;
break;
}
@ -1113,25 +1119,30 @@ void Preprocessor::HandleLineDirective() {
// If the StrTok is "eod", then it wasn't present. Otherwise, it must be a
// string followed by eod.
if (StrTok.is(tok::eod))
; // ok
else if (StrTok.isNot(tok::string_literal)) {
Diag(StrTok, diag::err_pp_line_invalid_filename);
return DiscardUntilEndOfDirective();
} else if (StrTok.hasUDSuffix()) {
Diag(StrTok, diag::err_invalid_string_udl);
return DiscardUntilEndOfDirective();
} else {
// Parse and validate the string, converting it into a unique ID.
StringLiteralParser Literal(StrTok, *this);
assert(Literal.isAscii() && "Didn't allow wide strings in");
if (Literal.hadError)
return DiscardUntilEndOfDirective();
if (Literal.Pascal) {
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
return DiscardUntilEndOfDirective();
}
FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
; // ok
else if (StrTok.isNot(tok::string_literal)) {
Diag(StrTok, diag::err_pp_line_invalid_filename);
DiscardUntilEndOfDirective();
return;
} else if (StrTok.hasUDSuffix()) {
Diag(StrTok, diag::err_invalid_string_udl);
DiscardUntilEndOfDirective();
return;
} else {
// Parse and validate the string, converting it into a unique ID.
StringLiteralParser Literal(StrTok, *this);
assert(Literal.isAscii() && "Didn't allow wide strings in");
if (Literal.hadError) {
DiscardUntilEndOfDirective();
return;
}
if (Literal.Pascal) {
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
DiscardUntilEndOfDirective();
return;
}
FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
// Verify that there is nothing after the string, other than EOD. Because
// of C99 6.10.4p5, macros that expand to empty tokens are ok.
CheckEndOfDirective("line", true);
@ -1258,25 +1269,30 @@ void Preprocessor::HandleDigitDirective(Token &DigitTok) {
// string followed by eod.
if (StrTok.is(tok::eod)) {
// Treat this like "#line NN", which doesn't change file characteristics.
FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
} else if (StrTok.isNot(tok::string_literal)) {
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
return DiscardUntilEndOfDirective();
} else if (StrTok.hasUDSuffix()) {
Diag(StrTok, diag::err_invalid_string_udl);
return DiscardUntilEndOfDirective();
} else {
// Parse and validate the string, converting it into a unique ID.
StringLiteralParser Literal(StrTok, *this);
assert(Literal.isAscii() && "Didn't allow wide strings in");
if (Literal.hadError)
return DiscardUntilEndOfDirective();
if (Literal.Pascal) {
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
return DiscardUntilEndOfDirective();
}
FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
} else if (StrTok.isNot(tok::string_literal)) {
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
DiscardUntilEndOfDirective();
return;
} else if (StrTok.hasUDSuffix()) {
Diag(StrTok, diag::err_invalid_string_udl);
DiscardUntilEndOfDirective();
return;
} else {
// Parse and validate the string, converting it into a unique ID.
StringLiteralParser Literal(StrTok, *this);
assert(Literal.isAscii() && "Didn't allow wide strings in");
if (Literal.hadError) {
DiscardUntilEndOfDirective();
return;
}
if (Literal.Pascal) {
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
DiscardUntilEndOfDirective();
return;
}
FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
// If a filename was present, read any flags that are present.
if (ReadLineMarkerFlags(IsFileEntry, IsFileExit, FileKind, *this))
return;
@ -1340,13 +1356,14 @@ void Preprocessor::HandleIdentSCCSDirective(Token &Tok) {
DiscardUntilEndOfDirective();
return;
}
if (StrTok.hasUDSuffix()) {
Diag(StrTok, diag::err_invalid_string_udl);
return DiscardUntilEndOfDirective();
}
// Verify that there is nothing after the string, other than EOD.
if (StrTok.hasUDSuffix()) {
Diag(StrTok, diag::err_invalid_string_udl);
DiscardUntilEndOfDirective();
return;
}
// Verify that there is nothing after the string, other than EOD.
CheckEndOfDirective("ident");
if (Callbacks) {
@ -2788,31 +2805,29 @@ void Preprocessor::HandleIfDirective(Token &IfToken,
const Token &HashToken,
bool ReadAnyTokensBeforeDirective) {
++NumIf;
// Parse and evaluate the conditional expression.
IdentifierInfo *IfNDefMacro = nullptr;
const SourceLocation ConditionalBegin = CurPPLexer->getSourceLocation();
const DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);
const bool ConditionalTrue = DER.Conditional;
const SourceLocation ConditionalEnd = CurPPLexer->getSourceLocation();
// If this condition is equivalent to #ifndef X, and if this is the first
// directive seen, handle it for the multiple-include optimization.
// Parse and evaluate the conditional expression.
IdentifierInfo *IfNDefMacro = nullptr;
const DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);
const bool ConditionalTrue = DER.Conditional;
// If this condition is equivalent to #ifndef X, and if this is the first
// directive seen, handle it for the multiple-include optimization.
if (CurPPLexer->getConditionalStackDepth() == 0) {
if (!ReadAnyTokensBeforeDirective && IfNDefMacro && ConditionalTrue)
// FIXME: Pass in the location of the macro name, not the 'if' token.
CurPPLexer->MIOpt.EnterTopLevelIfndef(IfNDefMacro, IfToken.getLocation());
else
CurPPLexer->MIOpt.EnterTopLevelConditional();
}
if (Callbacks)
Callbacks->If(IfToken.getLocation(),
SourceRange(ConditionalBegin, ConditionalEnd),
(ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));
// Should we include the stuff contained by this directive?
if (PPOpts->SingleFileParseMode && DER.IncludedUndefinedIds) {
}
if (Callbacks)
Callbacks->If(
IfToken.getLocation(), DER.ExprRange,
(ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));
// Should we include the stuff contained by this directive?
if (PPOpts->SingleFileParseMode && DER.IncludedUndefinedIds) {
// In 'single-file-parse mode' undefined identifiers trigger parsing of all
// the directive blocks.
CurPPLexer->pushConditionalLevel(IfToken.getLocation(), /*wasskip*/false,
@ -2899,15 +2914,13 @@ void Preprocessor::HandleElifDirective(Token &ElifToken,
const Token &HashToken) {
++NumElse;
// #elif directive in a non-skipping conditional... start skipping.
// We don't care what the condition is, because we will always skip it (since
// the block immediately before it was included).
const SourceLocation ConditionalBegin = CurPPLexer->getSourceLocation();
DiscardUntilEndOfDirective();
const SourceLocation ConditionalEnd = CurPPLexer->getSourceLocation();
PPConditionalInfo CI;
if (CurPPLexer->popConditionalLevel(CI)) {
// #elif directive in a non-skipping conditional... start skipping.
// We don't care what the condition is, because we will always skip it (since
// the block immediately before it was included).
SourceRange ConditionRange = DiscardUntilEndOfDirective();
PPConditionalInfo CI;
if (CurPPLexer->popConditionalLevel(CI)) {
Diag(ElifToken, diag::pp_err_elif_without_if);
return;
}
@ -2917,14 +2930,13 @@ void Preprocessor::HandleElifDirective(Token &ElifToken,
CurPPLexer->MIOpt.EnterTopLevelConditional();
// If this is a #elif with a #else before it, report the error.
if (CI.FoundElse) Diag(ElifToken, diag::pp_err_elif_after_else);
if (Callbacks)
Callbacks->Elif(ElifToken.getLocation(),
SourceRange(ConditionalBegin, ConditionalEnd),
PPCallbacks::CVK_NotEvaluated, CI.IfLoc);
if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) {
if (CI.FoundElse) Diag(ElifToken, diag::pp_err_elif_after_else);
if (Callbacks)
Callbacks->Elif(ElifToken.getLocation(), ConditionRange,
PPCallbacks::CVK_NotEvaluated, CI.IfLoc);
if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) {
// In 'single-file-parse mode' undefined identifiers trigger parsing of all
// the directive blocks.
CurPPLexer->pushConditionalLevel(ElifToken.getLocation(), /*wasskip*/false,

View File

@ -152,8 +152,8 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
return true;
}
// Consume the ).
Result.setEnd(PeekTok.getLocation());
PP.LexNonComment(PeekTok);
Result.setEnd(PeekTok.getLocation());
} else {
// Consume identifier.
Result.setEnd(PeekTok.getLocation());
@ -842,14 +842,22 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) {
PPValue ResVal(BitWidth);
DefinedTracker DT;
SourceLocation ExprStartLoc = SourceMgr.getExpansionLoc(Tok.getLocation());
if (EvaluateValue(ResVal, Tok, DT, true, *this)) {
// Parse error, skip the rest of the macro line.
SourceRange ConditionRange = ExprStartLoc;
if (Tok.isNot(tok::eod))
DiscardUntilEndOfDirective();
ConditionRange = DiscardUntilEndOfDirective();
// Restore 'DisableMacroExpansion'.
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
return {false, DT.IncludedUndefinedIds};
// We cannot trust the source range from the value because there was a
// parse error. Track the range manually -- the end of the directive is the
// end of the condition range.
return {false,
DT.IncludedUndefinedIds,
{ExprStartLoc, ConditionRange.getEnd()}};
}
// If we are at the end of the expression after just parsing a value, there
@ -863,7 +871,7 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) {
// Restore 'DisableMacroExpansion'.
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
return {ResVal.Val != 0, DT.IncludedUndefinedIds};
return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};
}
// Otherwise, we must have a binary operator (e.g. "#if 1 < 2"), so parse the
@ -876,7 +884,7 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) {
// Restore 'DisableMacroExpansion'.
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
return {false, DT.IncludedUndefinedIds};
return {false, DT.IncludedUndefinedIds, ResVal.getRange()};
}
// If we aren't at the tok::eod token, something bad happened, like an extra
@ -888,5 +896,5 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) {
// Restore 'DisableMacroExpansion'.
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
return {ResVal.Val != 0, DT.IncludedUndefinedIds};
return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};
}

View File

@ -431,16 +431,69 @@ TEST_F(PPCallbacksTest, OpenCLExtensionPragmaDisabled) {
}
TEST_F(PPCallbacksTest, DirectiveExprRanges) {
const auto &Results1 = DirectiveExprRange("#if FLUZZY_FLOOF\n#endif\n");
EXPECT_EQ(Results1.size(), 1);
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results1[0].ConditionRange, false)),
"FLUZZY_FLOOF");
const auto &Results2 = DirectiveExprRange("#if 1 + 4 < 7\n#endif\n");
EXPECT_EQ(Results2.size(), 1);
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results2[0].ConditionRange, false)),
"1 + 4 < 7");
const auto &Results3 = DirectiveExprRange("#if 1 + \\\n 2\n#endif\n");
EXPECT_EQ(Results3.size(), 1);
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results3[0].ConditionRange, false)),
"1 + \\\n 2");
const auto &Results4 = DirectiveExprRange("#if 0\n#elif FLOOFY\n#endif\n");
EXPECT_EQ(Results4.size(), 2);
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results4[0].ConditionRange, false)),
"0");
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results4[1].ConditionRange, false)),
"FLOOFY");
const auto &Results5 = DirectiveExprRange("#if 1\n#elif FLOOFY\n#endif\n");
EXPECT_EQ(Results5.size(), 2);
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results5[0].ConditionRange, false)),
"1");
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results5[1].ConditionRange, false)),
"FLOOFY");
const auto &Results6 =
DirectiveExprRange("#if defined(FLUZZY_FLOOF)\n#endif\n");
EXPECT_EQ(Results6.size(), 1);
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results6[0].ConditionRange, false)),
"defined(FLUZZY_FLOOF)");
const auto &Results7 =
DirectiveExprRange("#if 1\n#elif defined(FLOOFY)\n#endif\n");
EXPECT_EQ(Results7.size(), 2);
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results7[0].ConditionRange, false)),
"1");
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results7[1].ConditionRange, false)),
"defined(FLOOFY)");
const auto &Results8 =
DirectiveExprRange("#define FLOOFY 0\n#if __FILE__ > FLOOFY\n#endif\n");
EXPECT_EQ(Results8.size(), 1U);
EXPECT_EQ(
GetSourceStringToEnd(CharSourceRange(Results8[0].ConditionRange, false)),
" __FILE__ > FLOOFY\n#");
"__FILE__ > FLOOFY");
EXPECT_EQ(
Lexer::getSourceText(CharSourceRange(Results8[0].ConditionRange, false),
SourceMgr, LangOpts),
" __FILE__ > FLOOFY\n");
"__FILE__ > FLOOFY");
}
} // namespace