From: Aaron Ballman Date: Thu, 17 Jan 2019 20:21:34 +0000 (+0000) Subject: Revert r351209 (which was a revert of r350891) with a fix. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ed77615d4200c93cbfffa1d557623991ffb6a090;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@351470 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h index 64ddb5307f..36cb718570 100644 --- a/include/clang/Lex/Preprocessor.h +++ b/include/clang/Lex/Preprocessor.h @@ -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 diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp index d62a3513c7..3bee026b04 100644 --- a/lib/Lex/PPDirectives.cpp +++ b/lib/Lex/PPDirectives.cpp @@ -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, diff --git a/lib/Lex/PPExpressions.cpp b/lib/Lex/PPExpressions.cpp index ac01efad9b..01ded9c1e2 100644 --- a/lib/Lex/PPExpressions.cpp +++ b/lib/Lex/PPExpressions.cpp @@ -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()}; } diff --git a/unittests/Lex/PPCallbacksTest.cpp b/unittests/Lex/PPCallbacksTest.cpp index 838e033e3d..e19b99cd6d 100644 --- a/unittests/Lex/PPCallbacksTest.cpp +++ b/unittests/Lex/PPCallbacksTest.cpp @@ -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