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

4 files changed, 46 insertions, 103 deletions

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