r350891 - Correct the source range returned from preprocessor callbacks.
Matt Morehouse via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 10 15:51:06 PST 2019
This revision has broken
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/18904.
Please take a look.
On Thu, Jan 10, 2019 at 3:18 PM Nico Weber via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> ../../clang/lib/Lex/PPExpressions.cpp:852:43: warning: missing field
> 'ExprRange' initializer [-Wmissing-field-initializers]
> return {false, DT.IncludedUndefinedIds};
> ^
>
> On Thu, Jan 10, 2019 at 4:26 PM Aaron Ballman via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: aaronballman
>> Date: Thu Jan 10 13:22:13 2019
>> New Revision: 350891
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=350891&view=rev
>> Log:
>> Correct the source range returned from preprocessor callbacks.
>>
>> This adjusts the source range passed in to the preprocessor callbacks to
>> only include the condition range itself, rather than all of the
>> conditionally skipped tokens.
>>
>> Modified:
>> cfe/trunk/include/clang/Lex/Preprocessor.h
>> cfe/trunk/lib/Lex/PPDirectives.cpp
>> cfe/trunk/lib/Lex/PPExpressions.cpp
>> cfe/trunk/unittests/Lex/PPCallbacksTest.cpp
>>
>> Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=350891&r1=350890&r2=350891&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
>> +++ cfe/trunk/include/clang/Lex/Preprocessor.h Thu Jan 10 13:22:13 2019
>> @@ -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
>>
>> Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=350891&r1=350890&r2=350891&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
>> +++ cfe/trunk/lib/Lex/PPDirectives.cpp Thu Jan 10 13:22:13 2019
>> @@ -79,12 +79,18 @@ Preprocessor::AllocateVisibilityMacroDir
>>
>> /// Read and discard all tokens remaining on the current line until
>> /// the tok::eod token is found.
>> -void Preprocessor::DiscardUntilEndOfDirective() {
>> +SourceRange Preprocessor::DiscardUntilEndOfDirective() {
>> Token Tmp;
>> - do {
>> - LexUnexpandedToken(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");
>> - } while (Tmp.isNot(tok::eod));
>> + LexUnexpandedToken(Tmp);
>> + }
>> + Res.setEnd(Tmp.getLocation());
>> + return Res;
>> }
>>
>> /// Enumerates possible cases of #define/#undef a reserved identifier.
>> @@ -538,19 +544,19 @@ void Preprocessor::SkipExcludedCondition
>> 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;
>> + DirectiveEvalResult DER =
>> EvaluateDirectiveExpression(IfNDefMacro);
>> + const bool CondValue = DER.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);
>> + Callbacks->Elif(
>> + Tok.getLocation(), DER.ExprRange,
>> + (CondValue ? PPCallbacks::CVK_True :
>> PPCallbacks::CVK_False),
>> + CondInfo.IfLoc);
>> }
>> // If this condition is true, enter it!
>> if (CondValue) {
>> @@ -1116,19 +1122,24 @@ void Preprocessor::HandleLineDirective()
>> ; // ok
>> else if (StrTok.isNot(tok::string_literal)) {
>> Diag(StrTok, diag::err_pp_line_invalid_filename);
>> - return DiscardUntilEndOfDirective();
>> + DiscardUntilEndOfDirective();
>> + return;
>> } else if (StrTok.hasUDSuffix()) {
>> Diag(StrTok, diag::err_invalid_string_udl);
>> - return DiscardUntilEndOfDirective();
>> + 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)
>> - return DiscardUntilEndOfDirective();
>> + if (Literal.hadError) {
>> + DiscardUntilEndOfDirective();
>> + return;
>> + }
>> if (Literal.Pascal) {
>> Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
>> - return DiscardUntilEndOfDirective();
>> + DiscardUntilEndOfDirective();
>> + return;
>> }
>> FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
>>
>> @@ -1261,19 +1272,24 @@ void Preprocessor::HandleDigitDirective(
>> FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
>> } else if (StrTok.isNot(tok::string_literal)) {
>> Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
>> - return DiscardUntilEndOfDirective();
>> + DiscardUntilEndOfDirective();
>> + return;
>> } else if (StrTok.hasUDSuffix()) {
>> Diag(StrTok, diag::err_invalid_string_udl);
>> - return DiscardUntilEndOfDirective();
>> + 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)
>> - return DiscardUntilEndOfDirective();
>> + if (Literal.hadError) {
>> + DiscardUntilEndOfDirective();
>> + return;
>> + }
>> if (Literal.Pascal) {
>> Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
>> - return DiscardUntilEndOfDirective();
>> + DiscardUntilEndOfDirective();
>> + return;
>> }
>> FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
>>
>> @@ -1343,7 +1359,8 @@ void Preprocessor::HandleIdentSCCSDirect
>>
>> if (StrTok.hasUDSuffix()) {
>> Diag(StrTok, diag::err_invalid_string_udl);
>> - return DiscardUntilEndOfDirective();
>> + DiscardUntilEndOfDirective();
>> + return;
>> }
>>
>> // Verify that there is nothing after the string, other than EOD.
>> @@ -2783,10 +2800,8 @@ void Preprocessor::HandleIfDirective(Tok
>>
>> // 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.
>> @@ -2799,9 +2814,9 @@ void Preprocessor::HandleIfDirective(Tok
>> }
>>
>> if (Callbacks)
>> - Callbacks->If(IfToken.getLocation(),
>> - SourceRange(ConditionalBegin, ConditionalEnd),
>> - (ConditionalTrue ? PPCallbacks::CVK_True :
>> PPCallbacks::CVK_False));
>> + 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) {
>> @@ -2894,9 +2909,7 @@ void Preprocessor::HandleElifDirective(T
>> // #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();
>> + SourceRange ConditionRange = DiscardUntilEndOfDirective();
>>
>> PPConditionalInfo CI;
>> if (CurPPLexer->popConditionalLevel(CI)) {
>> @@ -2912,8 +2925,7 @@ void Preprocessor::HandleElifDirective(T
>> if (CI.FoundElse) Diag(ElifToken, diag::pp_err_elif_after_else);
>>
>> if (Callbacks)
>> - Callbacks->Elif(ElifToken.getLocation(),
>> - SourceRange(ConditionalBegin, ConditionalEnd),
>> + Callbacks->Elif(ElifToken.getLocation(), ConditionRange,
>> PPCallbacks::CVK_NotEvaluated, CI.IfLoc);
>>
>> if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) {
>>
>> Modified: cfe/trunk/lib/Lex/PPExpressions.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=350891&r1=350890&r2=350891&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Lex/PPExpressions.cpp (original)
>> +++ cfe/trunk/lib/Lex/PPExpressions.cpp Thu Jan 10 13:22:13 2019
>> @@ -152,8 +152,8 @@ static bool EvaluateDefined(PPValue &Res
>> return true;
>> }
>> // Consume the ).
>> - Result.setEnd(PeekTok.getLocation());
>> PP.LexNonComment(PeekTok);
>> + Result.setEnd(PeekTok.getLocation());
>> } else {
>> // Consume identifier.
>> Result.setEnd(PeekTok.getLocation());
>> @@ -863,7 +863,7 @@ Preprocessor::EvaluateDirectiveExpressio
>>
>> // 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 +876,7 @@ Preprocessor::EvaluateDirectiveExpressio
>>
>> // 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 +888,5 @@ Preprocessor::EvaluateDirectiveExpressio
>>
>> // Restore 'DisableMacroExpansion'.
>> DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
>> - return {ResVal.Val != 0, DT.IncludedUndefinedIds};
>> + return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};
>> }
>>
>> Modified: cfe/trunk/unittests/Lex/PPCallbacksTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/PPCallbacksTest.cpp?rev=350891&r1=350890&r2=350891&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/unittests/Lex/PPCallbacksTest.cpp (original)
>> +++ cfe/trunk/unittests/Lex/PPCallbacksTest.cpp Thu Jan 10 13:22:13 2019
>> @@ -65,6 +65,29 @@ public:
>> SrcMgr::CharacteristicKind FileType;
>> };
>>
>> +class CondDirectiveCallbacks : public PPCallbacks {
>> +public:
>> + struct Result {
>> + SourceRange ConditionRange;
>> + ConditionValueKind ConditionValue;
>> +
>> + Result(SourceRange R, ConditionValueKind K)
>> + : ConditionRange(R), ConditionValue(K) {}
>> + };
>> +
>> + std::vector<Result> Results;
>> +
>> + void If(SourceLocation Loc, SourceRange ConditionRange,
>> + ConditionValueKind ConditionValue) override {
>> + Results.emplace_back(ConditionRange, ConditionValue);
>> + }
>> +
>> + void Elif(SourceLocation Loc, SourceRange ConditionRange,
>> + ConditionValueKind ConditionValue, SourceLocation IfLoc)
>> override {
>> + Results.emplace_back(ConditionRange, ConditionValue);
>> + }
>> +};
>> +
>> // Stub to collect data from PragmaOpenCLExtension callbacks.
>> class PragmaOpenCLExtensionCallbacks : public PPCallbacks {
>> public:
>> @@ -137,6 +160,15 @@ protected:
>> return StringRef(B, E - B);
>> }
>>
>> + StringRef GetSourceStringToEnd(CharSourceRange Range) {
>> + const char *B = SourceMgr.getCharacterData(Range.getBegin());
>> + const char *E = SourceMgr.getCharacterData(Range.getEnd());
>> +
>> + return StringRef(
>> + B,
>> + E - B + Lexer::MeasureTokenLength(Range.getEnd(), SourceMgr,
>> LangOpts));
>> + }
>> +
>> // Run lexer over SourceText and collect FilenameRange from
>> // the InclusionDirective callback.
>> CharSourceRange InclusionDirectiveFilenameRange(const char *SourceText,
>> @@ -199,6 +231,36 @@ protected:
>> return Callbacks;
>> }
>>
>> + std::vector<CondDirectiveCallbacks::Result>
>> + DirectiveExprRange(StringRef SourceText) {
>> + TrivialModuleLoader ModLoader;
>> + MemoryBufferCache PCMCache;
>> + std::unique_ptr<llvm::MemoryBuffer> Buf =
>> + llvm::MemoryBuffer::getMemBuffer(SourceText);
>> + SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
>> + HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(),
>> SourceMgr,
>> + Diags, LangOpts, Target.get());
>> + Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags,
>> LangOpts,
>> + SourceMgr, PCMCache, HeaderInfo, ModLoader,
>> + /*IILookup =*/nullptr,
>> + /*OwnsHeaderSearch =*/false);
>> + PP.Initialize(*Target);
>> + auto *Callbacks = new CondDirectiveCallbacks;
>> + PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));
>> +
>> + // Lex source text.
>> + PP.EnterMainSourceFile();
>> +
>> + while (true) {
>> + Token Tok;
>> + PP.Lex(Tok);
>> + if (Tok.is(tok::eof))
>> + break;
>> + }
>> +
>> + return Callbacks->Results;
>> + }
>> +
>> PragmaOpenCLExtensionCallbacks::CallbackParameters
>> PragmaOpenCLExtensionCall(const char *SourceText) {
>> LangOptions OpenCLLangOpts;
>> @@ -368,4 +430,59 @@ TEST_F(PPCallbacksTest, OpenCLExtensionP
>> ASSERT_EQ(ExpectedState, Parameters.State);
>> }
>>
>> -} // anonoymous namespace
>> +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)");
>> +}
>> +
>> +} // namespace
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190110/61ed8167/attachment-0001.html>
More information about the cfe-commits
mailing list