<div dir="ltr"><div dir="ltr"><div>../../clang/lib/Lex/PPExpressions.cpp:852:43: warning: missing field 'ExprRange' initializer [-Wmissing-field-initializers]</div><div>    return {false, DT.IncludedUndefinedIds};</div><div>                                          ^</div></div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jan 10, 2019 at 4:26 PM Aaron Ballman via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: aaronballman<br>
Date: Thu Jan 10 13:22:13 2019<br>
New Revision: 350891<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=350891&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=350891&view=rev</a><br>
Log:<br>
Correct the source range returned from preprocessor callbacks.<br>
<br>
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.<br>
<br>
Modified:<br>
    cfe/trunk/include/clang/Lex/Preprocessor.h<br>
    cfe/trunk/lib/Lex/PPDirectives.cpp<br>
    cfe/trunk/lib/Lex/PPExpressions.cpp<br>
    cfe/trunk/unittests/Lex/PPCallbacksTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Lex/Preprocessor.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=350891&r1=350890&r2=350891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=350891&r1=350890&r2=350891&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)<br>
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Thu Jan 10 13:22:13 2019<br>
@@ -1816,8 +1816,8 @@ public:<br>
   void CheckEndOfDirective(const char *DirType, bool EnableMacros = false);<br>
<br>
   /// Read and discard all tokens remaining on the current line until<br>
-  /// the tok::eod token is found.<br>
-  void DiscardUntilEndOfDirective();<br>
+  /// the tok::eod token is found. Returns the range of the skipped tokens.<br>
+  SourceRange DiscardUntilEndOfDirective();<br>
<br>
   /// Returns true if the preprocessor has seen a use of<br>
   /// __DATE__ or __TIME__ in the file so far.<br>
@@ -1982,6 +1982,9 @@ private:<br>
<br>
     /// True if the expression contained identifiers that were undefined.<br>
     bool IncludedUndefinedIds;<br>
+<br>
+    /// The source range for the expression.<br>
+    SourceRange ExprRange;<br>
   };<br>
<br>
   /// Evaluate an integer constant expression that may occur after a<br>
<br>
Modified: cfe/trunk/lib/Lex/PPDirectives.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=350891&r1=350890&r2=350891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=350891&r1=350890&r2=350891&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)<br>
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Thu Jan 10 13:22:13 2019<br>
@@ -79,12 +79,18 @@ Preprocessor::AllocateVisibilityMacroDir<br>
<br>
 /// Read and discard all tokens remaining on the current line until<br>
 /// the tok::eod token is found.<br>
-void Preprocessor::DiscardUntilEndOfDirective() {<br>
+SourceRange Preprocessor::DiscardUntilEndOfDirective() {<br>
   Token Tmp;<br>
-  do {<br>
-    LexUnexpandedToken(Tmp);<br>
+  SourceRange Res;<br>
+<br>
+  LexUnexpandedToken(Tmp);<br>
+  Res.setBegin(Tmp.getLocation());<br>
+  while (Tmp.isNot(tok::eod)) {<br>
     assert(Tmp.isNot(tok::eof) && "EOF seen while discarding directive tokens");<br>
-  } while (Tmp.isNot(tok::eod));<br>
+    LexUnexpandedToken(Tmp);<br>
+  }<br>
+  Res.setEnd(Tmp.getLocation());<br>
+  return Res;<br>
 }<br>
<br>
 /// Enumerates possible cases of #define/#undef a reserved identifier.<br>
@@ -538,19 +544,19 @@ void Preprocessor::SkipExcludedCondition<br>
         if (CondInfo.WasSkipping || CondInfo.FoundNonSkip) {<br>
           DiscardUntilEndOfDirective();<br>
         } else {<br>
-          const SourceLocation CondBegin = CurPPLexer->getSourceLocation();<br>
           // Restore the value of LexingRawMode so that identifiers are<br>
           // looked up, etc, inside the #elif expression.<br>
           assert(CurPPLexer->LexingRawMode && "We have to be skipping here!");<br>
           CurPPLexer->LexingRawMode = false;<br>
           IdentifierInfo *IfNDefMacro = nullptr;<br>
-          const bool CondValue = EvaluateDirectiveExpression(IfNDefMacro).Conditional;<br>
+          DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);<br>
+          const bool CondValue = DER.Conditional;<br>
           CurPPLexer->LexingRawMode = true;<br>
           if (Callbacks) {<br>
-            const SourceLocation CondEnd = CurPPLexer->getSourceLocation();<br>
-            Callbacks->Elif(Tok.getLocation(),<br>
-                            SourceRange(CondBegin, CondEnd),<br>
-                            (CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False), CondInfo.IfLoc);<br>
+            Callbacks->Elif(<br>
+                Tok.getLocation(), DER.ExprRange,<br>
+                (CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False),<br>
+                CondInfo.IfLoc);<br>
           }<br>
           // If this condition is true, enter it!<br>
           if (CondValue) {<br>
@@ -1116,19 +1122,24 @@ void Preprocessor::HandleLineDirective()<br>
     ; // ok<br>
   else if (StrTok.isNot(tok::string_literal)) {<br>
     Diag(StrTok, diag::err_pp_line_invalid_filename);<br>
-    return DiscardUntilEndOfDirective();<br>
+    DiscardUntilEndOfDirective();<br>
+    return;<br>
   } else if (StrTok.hasUDSuffix()) {<br>
     Diag(StrTok, diag::err_invalid_string_udl);<br>
-    return DiscardUntilEndOfDirective();<br>
+    DiscardUntilEndOfDirective();<br>
+    return;<br>
   } else {<br>
     // Parse and validate the string, converting it into a unique ID.<br>
     StringLiteralParser Literal(StrTok, *this);<br>
     assert(Literal.isAscii() && "Didn't allow wide strings in");<br>
-    if (Literal.hadError)<br>
-      return DiscardUntilEndOfDirective();<br>
+    if (Literal.hadError) {<br>
+      DiscardUntilEndOfDirective();<br>
+      return;<br>
+    }<br>
     if (Literal.Pascal) {<br>
       Diag(StrTok, diag::err_pp_linemarker_invalid_filename);<br>
-      return DiscardUntilEndOfDirective();<br>
+      DiscardUntilEndOfDirective();<br>
+      return;<br>
     }<br>
     FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());<br>
<br>
@@ -1261,19 +1272,24 @@ void Preprocessor::HandleDigitDirective(<br>
     FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());<br>
   } else if (StrTok.isNot(tok::string_literal)) {<br>
     Diag(StrTok, diag::err_pp_linemarker_invalid_filename);<br>
-    return DiscardUntilEndOfDirective();<br>
+    DiscardUntilEndOfDirective();<br>
+    return;<br>
   } else if (StrTok.hasUDSuffix()) {<br>
     Diag(StrTok, diag::err_invalid_string_udl);<br>
-    return DiscardUntilEndOfDirective();<br>
+    DiscardUntilEndOfDirective();<br>
+    return;<br>
   } else {<br>
     // Parse and validate the string, converting it into a unique ID.<br>
     StringLiteralParser Literal(StrTok, *this);<br>
     assert(Literal.isAscii() && "Didn't allow wide strings in");<br>
-    if (Literal.hadError)<br>
-      return DiscardUntilEndOfDirective();<br>
+    if (Literal.hadError) {<br>
+      DiscardUntilEndOfDirective();<br>
+      return;<br>
+    }<br>
     if (Literal.Pascal) {<br>
       Diag(StrTok, diag::err_pp_linemarker_invalid_filename);<br>
-      return DiscardUntilEndOfDirective();<br>
+      DiscardUntilEndOfDirective();<br>
+      return;<br>
     }<br>
     FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());<br>
<br>
@@ -1343,7 +1359,8 @@ void Preprocessor::HandleIdentSCCSDirect<br>
<br>
   if (StrTok.hasUDSuffix()) {<br>
     Diag(StrTok, diag::err_invalid_string_udl);<br>
-    return DiscardUntilEndOfDirective();<br>
+    DiscardUntilEndOfDirective();<br>
+    return;<br>
   }<br>
<br>
   // Verify that there is nothing after the string, other than EOD.<br>
@@ -2783,10 +2800,8 @@ void Preprocessor::HandleIfDirective(Tok<br>
<br>
   // Parse and evaluate the conditional expression.<br>
   IdentifierInfo *IfNDefMacro = nullptr;<br>
-  const SourceLocation ConditionalBegin = CurPPLexer->getSourceLocation();<br>
   const DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);<br>
   const bool ConditionalTrue = DER.Conditional;<br>
-  const SourceLocation ConditionalEnd = CurPPLexer->getSourceLocation();<br>
<br>
   // If this condition is equivalent to #ifndef X, and if this is the first<br>
   // directive seen, handle it for the multiple-include optimization.<br>
@@ -2799,9 +2814,9 @@ void Preprocessor::HandleIfDirective(Tok<br>
   }<br>
<br>
   if (Callbacks)<br>
-    Callbacks->If(IfToken.getLocation(),<br>
-                  SourceRange(ConditionalBegin, ConditionalEnd),<br>
-                  (ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));<br>
+    Callbacks->If(<br>
+        IfToken.getLocation(), DER.ExprRange,<br>
+        (ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));<br>
<br>
   // Should we include the stuff contained by this directive?<br>
   if (PPOpts->SingleFileParseMode && DER.IncludedUndefinedIds) {<br>
@@ -2894,9 +2909,7 @@ void Preprocessor::HandleElifDirective(T<br>
   // #elif directive in a non-skipping conditional... start skipping.<br>
   // We don't care what the condition is, because we will always skip it (since<br>
   // the block immediately before it was included).<br>
-  const SourceLocation ConditionalBegin = CurPPLexer->getSourceLocation();<br>
-  DiscardUntilEndOfDirective();<br>
-  const SourceLocation ConditionalEnd = CurPPLexer->getSourceLocation();<br>
+  SourceRange ConditionRange = DiscardUntilEndOfDirective();<br>
<br>
   PPConditionalInfo CI;<br>
   if (CurPPLexer->popConditionalLevel(CI)) {<br>
@@ -2912,8 +2925,7 @@ void Preprocessor::HandleElifDirective(T<br>
   if (CI.FoundElse) Diag(ElifToken, diag::pp_err_elif_after_else);<br>
<br>
   if (Callbacks)<br>
-    Callbacks->Elif(ElifToken.getLocation(),<br>
-                    SourceRange(ConditionalBegin, ConditionalEnd),<br>
+    Callbacks->Elif(ElifToken.getLocation(), ConditionRange,<br>
                     PPCallbacks::CVK_NotEvaluated, CI.IfLoc);<br>
<br>
   if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) {<br>
<br>
Modified: cfe/trunk/lib/Lex/PPExpressions.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=350891&r1=350890&r2=350891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=350891&r1=350890&r2=350891&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Lex/PPExpressions.cpp (original)<br>
+++ cfe/trunk/lib/Lex/PPExpressions.cpp Thu Jan 10 13:22:13 2019<br>
@@ -152,8 +152,8 @@ static bool EvaluateDefined(PPValue &Res<br>
       return true;<br>
     }<br>
     // Consume the ).<br>
-    Result.setEnd(PeekTok.getLocation());<br>
     PP.LexNonComment(PeekTok);<br>
+    Result.setEnd(PeekTok.getLocation());<br>
   } else {<br>
     // Consume identifier.<br>
     Result.setEnd(PeekTok.getLocation());<br>
@@ -863,7 +863,7 @@ Preprocessor::EvaluateDirectiveExpressio<br>
<br>
     // Restore 'DisableMacroExpansion'.<br>
     DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;<br>
-    return {ResVal.Val != 0, DT.IncludedUndefinedIds};<br>
+    return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};<br>
   }<br>
<br>
   // Otherwise, we must have a binary operator (e.g. "#if 1 < 2"), so parse the<br>
@@ -876,7 +876,7 @@ Preprocessor::EvaluateDirectiveExpressio<br>
<br>
     // Restore 'DisableMacroExpansion'.<br>
     DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;<br>
-    return {false, DT.IncludedUndefinedIds};<br>
+    return {false, DT.IncludedUndefinedIds, ResVal.getRange()};<br>
   }<br>
<br>
   // If we aren't at the tok::eod token, something bad happened, like an extra<br>
@@ -888,5 +888,5 @@ Preprocessor::EvaluateDirectiveExpressio<br>
<br>
   // Restore 'DisableMacroExpansion'.<br>
   DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;<br>
-  return {ResVal.Val != 0, DT.IncludedUndefinedIds};<br>
+  return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};<br>
 }<br>
<br>
Modified: cfe/trunk/unittests/Lex/PPCallbacksTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/PPCallbacksTest.cpp?rev=350891&r1=350890&r2=350891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/PPCallbacksTest.cpp?rev=350891&r1=350890&r2=350891&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Lex/PPCallbacksTest.cpp (original)<br>
+++ cfe/trunk/unittests/Lex/PPCallbacksTest.cpp Thu Jan 10 13:22:13 2019<br>
@@ -65,6 +65,29 @@ public:<br>
   SrcMgr::CharacteristicKind FileType;<br>
 };<br>
<br>
+class CondDirectiveCallbacks : public PPCallbacks {<br>
+public:<br>
+  struct Result {<br>
+    SourceRange ConditionRange;<br>
+    ConditionValueKind ConditionValue;<br>
+<br>
+    Result(SourceRange R, ConditionValueKind K)<br>
+        : ConditionRange(R), ConditionValue(K) {}<br>
+  };<br>
+<br>
+  std::vector<Result> Results;<br>
+<br>
+  void If(SourceLocation Loc, SourceRange ConditionRange,<br>
+          ConditionValueKind ConditionValue) override {<br>
+    Results.emplace_back(ConditionRange, ConditionValue);<br>
+  }<br>
+<br>
+  void Elif(SourceLocation Loc, SourceRange ConditionRange,<br>
+            ConditionValueKind ConditionValue, SourceLocation IfLoc) override {<br>
+    Results.emplace_back(ConditionRange, ConditionValue);<br>
+  }<br>
+};<br>
+<br>
 // Stub to collect data from PragmaOpenCLExtension callbacks.<br>
 class PragmaOpenCLExtensionCallbacks : public PPCallbacks {<br>
 public:<br>
@@ -137,6 +160,15 @@ protected:<br>
     return StringRef(B, E - B);<br>
   }<br>
<br>
+  StringRef GetSourceStringToEnd(CharSourceRange Range) {<br>
+    const char *B = SourceMgr.getCharacterData(Range.getBegin());<br>
+    const char *E = SourceMgr.getCharacterData(Range.getEnd());<br>
+<br>
+    return StringRef(<br>
+        B,<br>
+        E - B + Lexer::MeasureTokenLength(Range.getEnd(), SourceMgr, LangOpts));<br>
+  }<br>
+<br>
   // Run lexer over SourceText and collect FilenameRange from<br>
   // the InclusionDirective callback.<br>
   CharSourceRange InclusionDirectiveFilenameRange(const char *SourceText,<br>
@@ -199,6 +231,36 @@ protected:<br>
     return Callbacks;<br>
   }<br>
<br>
+  std::vector<CondDirectiveCallbacks::Result><br>
+  DirectiveExprRange(StringRef SourceText) {<br>
+    TrivialModuleLoader ModLoader;<br>
+    MemoryBufferCache PCMCache;<br>
+    std::unique_ptr<llvm::MemoryBuffer> Buf =<br>
+        llvm::MemoryBuffer::getMemBuffer(SourceText);<br>
+    SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));<br>
+    HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,<br>
+                            Diags, LangOpts, Target.get());<br>
+    Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,<br>
+                    SourceMgr, PCMCache, HeaderInfo, ModLoader,<br>
+                    /*IILookup =*/nullptr,<br>
+                    /*OwnsHeaderSearch =*/false);<br>
+    PP.Initialize(*Target);<br>
+    auto *Callbacks = new CondDirectiveCallbacks;<br>
+    PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));<br>
+<br>
+    // Lex source text.<br>
+    PP.EnterMainSourceFile();<br>
+<br>
+    while (true) {<br>
+      Token Tok;<br>
+      PP.Lex(Tok);<br>
+      if (Tok.is(tok::eof))<br>
+        break;<br>
+    }<br>
+<br>
+    return Callbacks->Results;<br>
+  }<br>
+<br>
   PragmaOpenCLExtensionCallbacks::CallbackParameters<br>
   PragmaOpenCLExtensionCall(const char *SourceText) {<br>
     LangOptions OpenCLLangOpts;<br>
@@ -368,4 +430,59 @@ TEST_F(PPCallbacksTest, OpenCLExtensionP<br>
   ASSERT_EQ(ExpectedState, Parameters.State);<br>
 }<br>
<br>
-} // anonoymous namespace<br>
+TEST_F(PPCallbacksTest, DirectiveExprRanges) {<br>
+  const auto &Results1 = DirectiveExprRange("#if FLUZZY_FLOOF\n#endif\n");<br>
+  EXPECT_EQ(Results1.size(), 1);<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results1[0].ConditionRange, false)),<br>
+      "FLUZZY_FLOOF");<br>
+<br>
+  const auto &Results2 = DirectiveExprRange("#if 1 + 4 < 7\n#endif\n");<br>
+  EXPECT_EQ(Results2.size(), 1);<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results2[0].ConditionRange, false)),<br>
+      "1 + 4 < 7");<br>
+<br>
+  const auto &Results3 = DirectiveExprRange("#if 1 + \\\n  2\n#endif\n");<br>
+  EXPECT_EQ(Results3.size(), 1);<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results3[0].ConditionRange, false)),<br>
+      "1 + \\\n  2");<br>
+<br>
+  const auto &Results4 = DirectiveExprRange("#if 0\n#elif FLOOFY\n#endif\n");<br>
+  EXPECT_EQ(Results4.size(), 2);<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results4[0].ConditionRange, false)),<br>
+      "0");<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results4[1].ConditionRange, false)),<br>
+      "FLOOFY");<br>
+<br>
+  const auto &Results5 = DirectiveExprRange("#if 1\n#elif FLOOFY\n#endif\n");<br>
+  EXPECT_EQ(Results5.size(), 2);<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results5[0].ConditionRange, false)),<br>
+      "1");<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results5[1].ConditionRange, false)),<br>
+      "FLOOFY");<br>
+<br>
+  const auto &Results6 =<br>
+      DirectiveExprRange("#if defined(FLUZZY_FLOOF)\n#endif\n");<br>
+  EXPECT_EQ(Results6.size(), 1);<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results6[0].ConditionRange, false)),<br>
+      "defined(FLUZZY_FLOOF)");<br>
+<br>
+  const auto &Results7 =<br>
+      DirectiveExprRange("#if 1\n#elif defined(FLOOFY)\n#endif\n");<br>
+  EXPECT_EQ(Results7.size(), 2);<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results7[0].ConditionRange, false)),<br>
+      "1");<br>
+  EXPECT_EQ(<br>
+      GetSourceStringToEnd(CharSourceRange(Results7[1].ConditionRange, false)),<br>
+      "defined(FLOOFY)");<br>
+}<br>
+<br>
+} // namespace<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>