From: Benjamin Kramer Date: Tue, 15 Jan 2019 17:20:05 +0000 (+0000) Subject: Revert "Correct the source range returned from preprocessor callbacks." X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7c400363af5d53be336f3ff078036dfc3387d512;p=clang Revert "Correct the source range returned from preprocessor callbacks." This reverts commit r350891. Also add a test case that would return an empty string with r350891. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@351209 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h index 36cb718570..64ddb5307f 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. Returns the range of the skipped tokens. - SourceRange DiscardUntilEndOfDirective(); + /// the tok::eod token is found. + void DiscardUntilEndOfDirective(); /// Returns true if the preprocessor has seen a use of /// __DATE__ or __TIME__ in the file so far. @@ -1982,9 +1982,6 @@ 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 c6baa99e90..15fc086f8a 100644 --- a/lib/Lex/PPDirectives.cpp +++ b/lib/Lex/PPDirectives.cpp @@ -79,18 +79,12 @@ Preprocessor::AllocateVisibilityMacroDirective(SourceLocation Loc, /// Read and discard all tokens remaining on the current line until /// the tok::eod token is found. -SourceRange Preprocessor::DiscardUntilEndOfDirective() { +void 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"); + do { LexUnexpandedToken(Tmp); - } - Res.setEnd(Tmp.getLocation()); - return Res; + 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. @@ -544,19 +538,19 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc, 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; - DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro); - const bool CondValue = DER.Conditional; + const bool CondValue = EvaluateDirectiveExpression(IfNDefMacro).Conditional; CurPPLexer->LexingRawMode = true; if (Callbacks) { - Callbacks->Elif( - Tok.getLocation(), DER.ExprRange, - (CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False), - CondInfo.IfLoc); + 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) { @@ -1122,24 +1116,19 @@ void Preprocessor::HandleLineDirective() { ; // ok else if (StrTok.isNot(tok::string_literal)) { Diag(StrTok, diag::err_pp_line_invalid_filename); - DiscardUntilEndOfDirective(); - return; + return DiscardUntilEndOfDirective(); } else if (StrTok.hasUDSuffix()) { Diag(StrTok, diag::err_invalid_string_udl); - DiscardUntilEndOfDirective(); - return; + 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) { - DiscardUntilEndOfDirective(); - return; - } + if (Literal.hadError) + return DiscardUntilEndOfDirective(); if (Literal.Pascal) { Diag(StrTok, diag::err_pp_linemarker_invalid_filename); - DiscardUntilEndOfDirective(); - return; + return DiscardUntilEndOfDirective(); } FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString()); @@ -1272,24 +1261,19 @@ void Preprocessor::HandleDigitDirective(Token &DigitTok) { FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation()); } else if (StrTok.isNot(tok::string_literal)) { Diag(StrTok, diag::err_pp_linemarker_invalid_filename); - DiscardUntilEndOfDirective(); - return; + return DiscardUntilEndOfDirective(); } else if (StrTok.hasUDSuffix()) { Diag(StrTok, diag::err_invalid_string_udl); - DiscardUntilEndOfDirective(); - return; + 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) { - DiscardUntilEndOfDirective(); - return; - } + if (Literal.hadError) + return DiscardUntilEndOfDirective(); if (Literal.Pascal) { Diag(StrTok, diag::err_pp_linemarker_invalid_filename); - DiscardUntilEndOfDirective(); - return; + return DiscardUntilEndOfDirective(); } FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString()); @@ -1359,8 +1343,7 @@ void Preprocessor::HandleIdentSCCSDirective(Token &Tok) { if (StrTok.hasUDSuffix()) { Diag(StrTok, diag::err_invalid_string_udl); - DiscardUntilEndOfDirective(); - return; + return DiscardUntilEndOfDirective(); } // Verify that there is nothing after the string, other than EOD. @@ -2800,8 +2783,10 @@ void Preprocessor::HandleIfDirective(Token &IfToken, // 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. @@ -2814,9 +2799,9 @@ void Preprocessor::HandleIfDirective(Token &IfToken, } if (Callbacks) - Callbacks->If( - IfToken.getLocation(), DER.ExprRange, - (ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False)); + 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) { @@ -2909,7 +2894,9 @@ void Preprocessor::HandleElifDirective(Token &ElifToken, // #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(); + const SourceLocation ConditionalBegin = CurPPLexer->getSourceLocation(); + DiscardUntilEndOfDirective(); + const SourceLocation ConditionalEnd = CurPPLexer->getSourceLocation(); PPConditionalInfo CI; if (CurPPLexer->popConditionalLevel(CI)) { @@ -2925,7 +2912,8 @@ void Preprocessor::HandleElifDirective(Token &ElifToken, if (CI.FoundElse) Diag(ElifToken, diag::pp_err_elif_after_else); if (Callbacks) - Callbacks->Elif(ElifToken.getLocation(), ConditionRange, + Callbacks->Elif(ElifToken.getLocation(), + SourceRange(ConditionalBegin, ConditionalEnd), PPCallbacks::CVK_NotEvaluated, CI.IfLoc); if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) { diff --git a/lib/Lex/PPExpressions.cpp b/lib/Lex/PPExpressions.cpp index d40cd5cba7..ac01efad9b 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 ). - PP.LexNonComment(PeekTok); Result.setEnd(PeekTok.getLocation()); + PP.LexNonComment(PeekTok); } else { // Consume identifier. Result.setEnd(PeekTok.getLocation()); @@ -849,7 +849,7 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) { // Restore 'DisableMacroExpansion'. DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective; - return {false, DT.IncludedUndefinedIds, {}}; + return {false, DT.IncludedUndefinedIds}; } // If we are at the end of the expression after just parsing a value, there @@ -863,7 +863,7 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) { // Restore 'DisableMacroExpansion'. DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective; - return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()}; + return {ResVal.Val != 0, DT.IncludedUndefinedIds}; } // Otherwise, we must have a binary operator (e.g. "#if 1 < 2"), so parse the @@ -876,7 +876,7 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) { // Restore 'DisableMacroExpansion'. DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective; - return {false, DT.IncludedUndefinedIds, ResVal.getRange()}; + return {false, DT.IncludedUndefinedIds}; } // If we aren't at the tok::eod token, something bad happened, like an extra @@ -888,5 +888,5 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) { // Restore 'DisableMacroExpansion'. DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective; - return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()}; + return {ResVal.Val != 0, DT.IncludedUndefinedIds}; } diff --git a/unittests/Lex/PPCallbacksTest.cpp b/unittests/Lex/PPCallbacksTest.cpp index 940328842d..838e033e3d 100644 --- a/unittests/Lex/PPCallbacksTest.cpp +++ b/unittests/Lex/PPCallbacksTest.cpp @@ -431,58 +431,16 @@ TEST_F(PPCallbacksTest, OpenCLExtensionPragmaDisabled) { } TEST_F(PPCallbacksTest, DirectiveExprRanges) { - const auto &Results1 = DirectiveExprRange("#if FLUZZY_FLOOF\n#endif\n"); - EXPECT_EQ(Results1.size(), 1U); + const auto &Results8 = + DirectiveExprRange("#define FLOOFY 0\n#if __FILE__ > FLOOFY\n#endif\n"); + EXPECT_EQ(Results8.size(), 1U); 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(), 1U); - 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(), 1U); - 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(), 2U); - 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(), 2U); - 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(), 1U); - 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(), 2U); - EXPECT_EQ( - GetSourceStringToEnd(CharSourceRange(Results7[0].ConditionRange, false)), - "1"); + GetSourceStringToEnd(CharSourceRange(Results8[0].ConditionRange, false)), + " __FILE__ > FLOOFY\n#"); EXPECT_EQ( - GetSourceStringToEnd(CharSourceRange(Results7[1].ConditionRange, false)), - "defined(FLOOFY)"); + Lexer::getSourceText(CharSourceRange(Results8[0].ConditionRange, false), + SourceMgr, LangOpts), + " __FILE__ > FLOOFY\n"); } } // namespace