r350891 - Correct the source range returned from preprocessor callbacks.

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 10 15:18:16 PST 2019


../../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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190110/ccffc158/attachment-0001.html>


More information about the cfe-commits mailing list