Skip to content

Conversation

@vsapsai
Copy link
Collaborator

@vsapsai vsapsai commented May 2, 2025

Restore the behavior existing prior to fe2eefc. Make reporting of unevaluated directive source range more consistent and with fewer assumptions. In case of a failed evaluation don't assume any specific token and don't assume correct PPValue range tracking.

… file parse mode.

Restore the behavior existing prior to fe2eefc.
Make reporting of unevaluated directive source range more consistent and
with fewer assumptions. In case of a failed evaluation don't assume any
specific token and don't assume correct `PPValue` range tracking.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 2, 2025
@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-clang

Author: Volodymyr Sapsai (vsapsai)

Changes

Restore the behavior existing prior to fe2eefc. Make reporting of unevaluated directive source range more consistent and with fewer assumptions. In case of a failed evaluation don't assume any specific token and don't assume correct PPValue range tracking.


Full diff: https://github.com/llvm/llvm-project/pull/138358.diff

2 Files Affected:

  • (modified) clang/lib/Lex/PPExpressions.cpp (+6-5)
  • (modified) clang/unittests/Lex/PPCallbacksTest.cpp (+19-2)
diff --git a/clang/lib/Lex/PPExpressions.cpp b/clang/lib/Lex/PPExpressions.cpp
index a202af774256a..cf7e32bee2e71 100644
--- a/clang/lib/Lex/PPExpressions.cpp
+++ b/clang/lib/Lex/PPExpressions.cpp
@@ -903,9 +903,8 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro,
   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))
-      ConditionRange = DiscardUntilEndOfDirective(Tok);
+      DiscardUntilEndOfDirective(Tok);
 
     // Restore 'DisableMacroExpansion'.
     DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
@@ -916,7 +915,7 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro,
     return {std::nullopt,
             false,
             DT.IncludedUndefinedIds,
-            {ExprStartLoc, ConditionRange.getEnd()}};
+            {ExprStartLoc, Tok.getLocation()}};
   }
 
   EvaluatedDefined = DT.State != DefinedTracker::Unknown;
@@ -948,8 +947,10 @@ Preprocessor::EvaluateDirectiveExpression(IdentifierInfo *&IfNDefMacro,
 
     // Restore 'DisableMacroExpansion'.
     DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
-    SourceRange ValRange = ResVal.getRange();
-    return {std::nullopt, false, DT.IncludedUndefinedIds, ValRange};
+    return {std::nullopt,
+            false,
+            DT.IncludedUndefinedIds,
+            {ExprStartLoc, Tok.getLocation()}};
   }
 
   if (CheckForEoD) {
diff --git a/clang/unittests/Lex/PPCallbacksTest.cpp b/clang/unittests/Lex/PPCallbacksTest.cpp
index 15385c13879d3..c2a84d974dd39 100644
--- a/clang/unittests/Lex/PPCallbacksTest.cpp
+++ b/clang/unittests/Lex/PPCallbacksTest.cpp
@@ -237,14 +237,13 @@ class PPCallbacksTest : public ::testing::Test {
   }
 
   std::vector<CondDirectiveCallbacks::Result>
-  DirectiveExprRange(StringRef SourceText) {
+  DirectiveExprRange(StringRef SourceText, PreprocessorOptions PPOpts = {}) {
     HeaderSearchOptions HSOpts;
     TrivialModuleLoader ModLoader;
     std::unique_ptr<llvm::MemoryBuffer> Buf =
         llvm::MemoryBuffer::getMemBuffer(SourceText);
     SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
     HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, Target.get());
-    PreprocessorOptions PPOpts;
     Preprocessor PP(PPOpts, Diags, LangOpts, SourceMgr, HeaderInfo, ModLoader,
                     /*IILookup=*/nullptr, /*OwnsHeaderSearch=*/false);
     PP.Initialize(*Target);
@@ -569,6 +568,24 @@ TEST_F(PPCallbacksTest, DirectiveExprRanges) {
       Lexer::getSourceText(CharSourceRange(Results8[0].ConditionRange, false),
                            SourceMgr, LangOpts),
       "__FILE__ > FLOOFY");
+
+  const char *MultiExprIf = "#if defined(FLOOFY) || defined(FLUZZY)\n#endif\n";
+  const auto &Results9 = DirectiveExprRange(MultiExprIf);
+  EXPECT_EQ(Results9.size(), 1U);
+  EXPECT_EQ(
+      Lexer::getSourceText(CharSourceRange(Results9[0].ConditionRange, false),
+                           SourceMgr, LangOpts),
+      "defined(FLOOFY) || defined(FLUZZY)");
+
+  PreprocessorOptions PPOptsSingleFileParse;
+  PPOptsSingleFileParse.SingleFileParseMode = true;
+  const auto &Results10 =
+      DirectiveExprRange(MultiExprIf, PPOptsSingleFileParse);
+  EXPECT_EQ(Results10.size(), 1U);
+  EXPECT_EQ(
+      Lexer::getSourceText(CharSourceRange(Results10[0].ConditionRange, false),
+                           SourceMgr, LangOpts),
+      "defined(FLOOFY) || defined(FLUZZY)");
 }
 
 } // namespace

@vsapsai vsapsai merged commit c296b12 into llvm:main May 5, 2025
14 checks passed
@vsapsai vsapsai deleted the single-file-parse-directive-range branch May 5, 2025 19:07
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
… file parse mode. (llvm#138358)

Restore the behavior existing prior to
fe2eefc. Make reporting of unevaluated
directive source range more consistent and with fewer assumptions. In
case of a failed evaluation don't assume any specific token and don't
assume correct `PPValue` range tracking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants