Revert r351209 (which was a revert of r350891) with a fix.

The test case had a parse error that was causing the condition string to be misreported. We now have better fallback code for error cases.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@351470 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h
index 64ddb5307fb0e8dcb4b401471564823bf16530bf..36cb71857081ee44c0a04a13bf1b6f21e7830eaf 100644 (file)
--- 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.
-  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

diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp
index d62a3513c777025d8a3e9199ca3bb114059d9007..3bee026b0439fe21a8794f08f831101b7fc73770 100644 (file)
--- a/lib/Lex/PPDirectives.cpp
+++ b/lib/Lex/PPDirectives.cpp
@@ -76,18 +76,24 @@ Preprocessor::AllocateVisibilityMacroDirective(SourceLocation Loc,
                                                bool isPublic) {
   return new (BP) VisibilityMacroDirective(Loc, isPublic);
 }
-
-/// Read and discard all tokens remaining on the current line until
-/// the tok::eod token is found.
-void Preprocessor::DiscardUntilEndOfDirective() {
-  Token Tmp;
-  do {
-    LexUnexpandedToken(Tmp);
-    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.
+\r
+/// Read and discard all tokens remaining on the current line until\r
+/// the tok::eod token is found.\r
+SourceRange Preprocessor::DiscardUntilEndOfDirective() {\r
+  Token Tmp;\r
+  SourceRange Res;\r
+\r
+  LexUnexpandedToken(Tmp);\r
+  Res.setBegin(Tmp.getLocation());\r
+  while (Tmp.isNot(tok::eod)) {\r
+    assert(Tmp.isNot(tok::eof) && "EOF seen while discarding directive tokens");\r
+    LexUnexpandedToken(Tmp);\r
+  }\r
+  Res.setEnd(Tmp.getLocation());\r
+  return Res;\r
+}\r
+\r
+/// Enumerates possible cases of #define/#undef a reserved identifier.\r
 enum MacroDiag {
   MD_NoWarn,        //> Not a reserved identifier
   MD_KeywordDef,    //> Macro hides keyword, enabled by default
@@ -535,25 +541,25 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
 
         // If this is in a skipping block or if we're already handled this #if
         // block, don't bother parsing the condition.
-        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;
-          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);
-          }
-          // If this condition is true, enter it!
-          if (CondValue) {
+        if (CondInfo.WasSkipping || CondInfo.FoundNonSkip) {\r
+          DiscardUntilEndOfDirective();\r
+        } else {\r
+          // Restore the value of LexingRawMode so that identifiers are\r
+          // looked up, etc, inside the #elif expression.\r
+          assert(CurPPLexer->LexingRawMode && "We have to be skipping here!");\r
+          CurPPLexer->LexingRawMode = false;\r
+          IdentifierInfo *IfNDefMacro = nullptr;\r
+          DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);\r
+          const bool CondValue = DER.Conditional;\r
+          CurPPLexer->LexingRawMode = true;\r
+          if (Callbacks) {\r
+            Callbacks->Elif(\r
+                Tok.getLocation(), DER.ExprRange,\r
+                (CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False),\r
+                CondInfo.IfLoc);\r
+          }\r
+          // If this condition is true, enter it!\r
+          if (CondValue) {\r
             CondInfo.FoundNonSkip = true;
             break;
           }
@@ -1113,25 +1119,30 @@ void Preprocessor::HandleLineDirective() {
   // If the StrTok is "eod", then it wasn't present.  Otherwise, it must be a
   // string followed by eod.
   if (StrTok.is(tok::eod))
-    ; // ok
-  else if (StrTok.isNot(tok::string_literal)) {
-    Diag(StrTok, diag::err_pp_line_invalid_filename);
-    return DiscardUntilEndOfDirective();
-  } else if (StrTok.hasUDSuffix()) {
-    Diag(StrTok, diag::err_invalid_string_udl);
-    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)
-      return DiscardUntilEndOfDirective();
-    if (Literal.Pascal) {
-      Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
-      return DiscardUntilEndOfDirective();
-    }
-    FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
-
+    ; // ok\r
+  else if (StrTok.isNot(tok::string_literal)) {\r
+    Diag(StrTok, diag::err_pp_line_invalid_filename);\r
+    DiscardUntilEndOfDirective();\r
+    return;\r
+  } else if (StrTok.hasUDSuffix()) {\r
+    Diag(StrTok, diag::err_invalid_string_udl);\r
+    DiscardUntilEndOfDirective();\r
+    return;\r
+  } else {\r
+    // Parse and validate the string, converting it into a unique ID.\r
+    StringLiteralParser Literal(StrTok, *this);\r
+    assert(Literal.isAscii() && "Didn't allow wide strings in");\r
+    if (Literal.hadError) {\r
+      DiscardUntilEndOfDirective();\r
+      return;\r
+    }\r
+    if (Literal.Pascal) {\r
+      Diag(StrTok, diag::err_pp_linemarker_invalid_filename);\r
+      DiscardUntilEndOfDirective();\r
+      return;\r
+    }\r
+    FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());\r
+\r
     // Verify that there is nothing after the string, other than EOD.  Because
     // of C99 6.10.4p5, macros that expand to empty tokens are ok.
     CheckEndOfDirective("line", true);
@@ -1258,25 +1269,30 @@ void Preprocessor::HandleDigitDirective(Token &DigitTok) {
   // string followed by eod.
   if (StrTok.is(tok::eod)) {
     // Treat this like "#line NN", which doesn't change file characteristics.
-    FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
-  } else if (StrTok.isNot(tok::string_literal)) {
-    Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
-    return DiscardUntilEndOfDirective();
-  } else if (StrTok.hasUDSuffix()) {
-    Diag(StrTok, diag::err_invalid_string_udl);
-    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)
-      return DiscardUntilEndOfDirective();
-    if (Literal.Pascal) {
-      Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
-      return DiscardUntilEndOfDirective();
-    }
-    FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
-
+    FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());\r
+  } else if (StrTok.isNot(tok::string_literal)) {\r
+    Diag(StrTok, diag::err_pp_linemarker_invalid_filename);\r
+    DiscardUntilEndOfDirective();\r
+    return;\r
+  } else if (StrTok.hasUDSuffix()) {\r
+    Diag(StrTok, diag::err_invalid_string_udl);\r
+    DiscardUntilEndOfDirective();\r
+    return;\r
+  } else {\r
+    // Parse and validate the string, converting it into a unique ID.\r
+    StringLiteralParser Literal(StrTok, *this);\r
+    assert(Literal.isAscii() && "Didn't allow wide strings in");\r
+    if (Literal.hadError) {\r
+      DiscardUntilEndOfDirective();\r
+      return;\r
+    }\r
+    if (Literal.Pascal) {\r
+      Diag(StrTok, diag::err_pp_linemarker_invalid_filename);\r
+      DiscardUntilEndOfDirective();\r
+      return;\r
+    }\r
+    FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());\r
+\r
     // If a filename was present, read any flags that are present.
     if (ReadLineMarkerFlags(IsFileEntry, IsFileExit, FileKind, *this))
       return;
@@ -1340,13 +1356,14 @@ void Preprocessor::HandleIdentSCCSDirective(Token &Tok) {
       DiscardUntilEndOfDirective();
     return;
   }
-
-  if (StrTok.hasUDSuffix()) {
-    Diag(StrTok, diag::err_invalid_string_udl);
-    return DiscardUntilEndOfDirective();
-  }
-
-  // Verify that there is nothing after the string, other than EOD.
+\r
+  if (StrTok.hasUDSuffix()) {\r
+    Diag(StrTok, diag::err_invalid_string_udl);\r
+    DiscardUntilEndOfDirective();\r
+    return;\r
+  }\r
+\r
+  // Verify that there is nothing after the string, other than EOD.\r
   CheckEndOfDirective("ident");
 
   if (Callbacks) {
@@ -2788,31 +2805,29 @@ void Preprocessor::HandleIfDirective(Token &IfToken,
                                      const Token &HashToken,
                                      bool ReadAnyTokensBeforeDirective) {
   ++NumIf;
-
-  // 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.
+\r
+  // Parse and evaluate the conditional expression.\r
+  IdentifierInfo *IfNDefMacro = nullptr;\r
+  const DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);\r
+  const bool ConditionalTrue = DER.Conditional;\r
+\r
+  // If this condition is equivalent to #ifndef X, and if this is the first\r
+  // directive seen, handle it for the multiple-include optimization.\r
   if (CurPPLexer->getConditionalStackDepth() == 0) {
     if (!ReadAnyTokensBeforeDirective && IfNDefMacro && ConditionalTrue)
       // FIXME: Pass in the location of the macro name, not the 'if' token.
       CurPPLexer->MIOpt.EnterTopLevelIfndef(IfNDefMacro, IfToken.getLocation());
     else
       CurPPLexer->MIOpt.EnterTopLevelConditional();
-  }
-
-  if (Callbacks)
-    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) {
+  }\r
+\r
+  if (Callbacks)\r
+    Callbacks->If(\r
+        IfToken.getLocation(), DER.ExprRange,\r
+        (ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));\r
+\r
+  // Should we include the stuff contained by this directive?\r
+  if (PPOpts->SingleFileParseMode && DER.IncludedUndefinedIds) {\r
     // In 'single-file-parse mode' undefined identifiers trigger parsing of all
     // the directive blocks.
     CurPPLexer->pushConditionalLevel(IfToken.getLocation(), /*wasskip*/false,
@@ -2899,15 +2914,13 @@ void Preprocessor::HandleElifDirective(Token &ElifToken,
                                        const Token &HashToken) {
   ++NumElse;
 
-  // #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();
-
-  PPConditionalInfo CI;
-  if (CurPPLexer->popConditionalLevel(CI)) {
+  // #elif directive in a non-skipping conditional... start skipping.\r
+  // We don't care what the condition is, because we will always skip it (since\r
+  // the block immediately before it was included).\r
+  SourceRange ConditionRange = DiscardUntilEndOfDirective();\r
+\r
+  PPConditionalInfo CI;\r
+  if (CurPPLexer->popConditionalLevel(CI)) {\r
     Diag(ElifToken, diag::pp_err_elif_without_if);
     return;
   }
@@ -2917,14 +2930,13 @@ void Preprocessor::HandleElifDirective(Token &ElifToken,
     CurPPLexer->MIOpt.EnterTopLevelConditional();
 
   // If this is a #elif with a #else before it, report the error.
-  if (CI.FoundElse) Diag(ElifToken, diag::pp_err_elif_after_else);
-
-  if (Callbacks)
-    Callbacks->Elif(ElifToken.getLocation(),
-                    SourceRange(ConditionalBegin, ConditionalEnd),
-                    PPCallbacks::CVK_NotEvaluated, CI.IfLoc);
-
-  if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) {
+  if (CI.FoundElse) Diag(ElifToken, diag::pp_err_elif_after_else);\r
+\r
+  if (Callbacks)\r
+    Callbacks->Elif(ElifToken.getLocation(), ConditionRange,\r
+                    PPCallbacks::CVK_NotEvaluated, CI.IfLoc);\r
+\r
+  if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) {\r
     // In 'single-file-parse mode' undefined identifiers trigger parsing of all
     // the directive blocks.
     CurPPLexer->pushConditionalLevel(ElifToken.getLocation(), /*wasskip*/false,

diff --git a/lib/Lex/PPExpressions.cpp b/lib/Lex/PPExpressions.cpp
index ac01efad9bf69deaf6fbabc358cf8780507cc226..01ded9c1e2bf0caad33d5ea5bf41352436c059fa 100644 (file)
--- 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 ).
-    Result.setEnd(PeekTok.getLocation());
     PP.LexNonComment(PeekTok);
+    Result.setEnd(PeekTok.getLocation());
   } else {
     // Consume identifier.
     Result.setEnd(PeekTok.getLocation());
@@ -842,14 +842,22 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) {
 
   PPValue ResVal(BitWidth);
   DefinedTracker DT;
+  SourceLocation ExprStartLoc = SourceMgr.getExpansionLoc(Tok.getLocation());
   if (EvaluateValue(ResVal, Tok, DT, true, *this)) {
     // Parse error, skip the rest of the macro line.
+    SourceRange ConditionRange = ExprStartLoc;
     if (Tok.isNot(tok::eod))
-      DiscardUntilEndOfDirective();
+      ConditionRange = DiscardUntilEndOfDirective();
 
     // Restore 'DisableMacroExpansion'.
     DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
-    return {false, DT.IncludedUndefinedIds};
+
+    // We cannot trust the source range from the value because there was a
+    // parse error. Track the range manually -- the end of the directive is the
+    // end of the condition range.
+    return {false,
+            DT.IncludedUndefinedIds,
+            {ExprStartLoc, ConditionRange.getEnd()}};
   }
 
   // If we are at the end of the expression after just parsing a value, there
@@ -863,7 +871,7 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) {
 
     // 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 +884,7 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) {
 
     // 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 +896,5 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro) {
 
   // Restore 'DisableMacroExpansion'.
   DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
-  return {ResVal.Val != 0, DT.IncludedUndefinedIds};
+  return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};
 }

diff --git a/unittests/Lex/PPCallbacksTest.cpp b/unittests/Lex/PPCallbacksTest.cpp
index 838e033e3df490493a86b811ed61d33081ad4bff..e19b99cd6d56c30f4dd940728e4186f5ccc0680b 100644 (file)
--- a/unittests/Lex/PPCallbacksTest.cpp
+++ b/unittests/Lex/PPCallbacksTest.cpp
@@ -431,16 +431,69 @@ TEST_F(PPCallbacksTest, OpenCLExtensionPragmaDisabled) {
 }
 
 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)");
+
   const auto &Results8 =
       DirectiveExprRange("#define FLOOFY 0\n#if __FILE__ > FLOOFY\n#endif\n");
   EXPECT_EQ(Results8.size(), 1U);
   EXPECT_EQ(
       GetSourceStringToEnd(CharSourceRange(Results8[0].ConditionRange, false)),
-      " __FILE__ > FLOOFY\n#");
+      "__FILE__ > FLOOFY");
   EXPECT_EQ(
       Lexer::getSourceText(CharSourceRange(Results8[0].ConditionRange, false),
                            SourceMgr, LangOpts),
-      " __FILE__ > FLOOFY\n");
+      "__FILE__ > FLOOFY");
 }
 
 } // namespace