[clang] 9c7b7ee - [clang-format] Fix crashes on directives in discarded macro call args (#221501)

via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 29 12:48:08 PDT 2026


Author: Andre Sun
Date: 2026-09-29T21:48:01+02:00
New Revision: 9c7b7eef6af2d93ab3317e0689715188ae981cc5

URL: https://github.com/llvm/llvm-project/commit/9c7b7eef6af2d93ab3317e0689715188ae981cc5
DIFF: https://github.com/llvm/llvm-project/commit/9c7b7eef6af2d93ab3317e0689715188ae981cc5.diff

LOG: [clang-format] Fix crashes on directives in discarded macro call args (#221501)

Rewinding discarded macro arguments processes their preprocessor
directives twice, duplicating lines and causing crashes. Restore the
preprocessor bookkeeping on rewind, but retain the first set of
directive lines and discard duplicates during replay.

Exclude expansion EOF tokens from directive ranges so directives before
empty expansions are still formatted.

Fixes #131157

Assisted-by: Claude Code

Added: 
    

Modified: 
    clang/lib/Format/AffectedRangeManager.cpp
    clang/lib/Format/UnwrappedLineParser.cpp
    clang/lib/Format/UnwrappedLineParser.h
    clang/unittests/Format/FormatTestMacroExpansion.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/AffectedRangeManager.cpp b/clang/lib/Format/AffectedRangeManager.cpp
index 67108f3540191..7e9acc9fbbb04 100644
--- a/clang/lib/Format/AffectedRangeManager.cpp
+++ b/clang/lib/Format/AffectedRangeManager.cpp
@@ -35,7 +35,8 @@ bool AffectedRangeManager::computeAffectedLines(
     if (Line->InPPDirective) {
       FormatToken *Last = Line->Last;
       const auto *PPEnd = I + 1;
-      while (PPEnd != E && !(*PPEnd)->First->HasUnescapedNewline) {
+      while (PPEnd != E && !(*PPEnd)->First->HasUnescapedNewline &&
+             (*PPEnd)->First->isNot(tok::eof)) {
         Last = (*PPEnd)->Last;
         ++PPEnd;
       }

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 2d2fe729fbc2f..4825e825af1fa 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -95,13 +95,17 @@ std::ostream &operator<<(std::ostream &Stream, const UnwrappedLine &Line) {
 
 class ScopedLineState {
 public:
+  // With \c DiscardLines, the lines added while in scope are discarded.
   ScopedLineState(UnwrappedLineParser &Parser,
-                  bool SwitchToPreprocessorLines = false)
-      : Parser(Parser), OriginalLines(Parser.CurrentLines) {
+                  bool SwitchToPreprocessorLines = false,
+                  bool DiscardLines = false)
+      : Parser(Parser), OriginalLines(Parser.CurrentLines),
+        DiscardLines(DiscardLines) {
     if (SwitchToPreprocessorLines)
       Parser.CurrentLines = &Parser.PreprocessorDirectives;
     else if (!Parser.Line->Tokens.empty())
       Parser.CurrentLines = &Parser.Line->Tokens.back().Children;
+    OriginalNumLines = Parser.CurrentLines->size();
     PreBlockLine = std::move(Parser.Line);
     Parser.Line = std::make_unique<UnwrappedLine>();
     Parser.Line->Level = PreBlockLine->Level;
@@ -115,9 +119,11 @@ class ScopedLineState {
     if (!Parser.Line->Tokens.empty())
       Parser.addUnwrappedLine();
     assert(Parser.Line->Tokens.empty());
+    if (DiscardLines)
+      Parser.CurrentLines->truncate(OriginalNumLines);
     Parser.Line = std::move(PreBlockLine);
     if (Parser.CurrentLines == &Parser.PreprocessorDirectives)
-      Parser.AtEndOfPPLine = true;
+      Parser.PP.AtEndOfPPLine = true;
     Parser.CurrentLines = OriginalLines;
   }
 
@@ -126,6 +132,8 @@ class ScopedLineState {
 
   std::unique_ptr<UnwrappedLine> PreBlockLine;
   SmallVectorImpl<UnwrappedLine> *OriginalLines;
+  size_t OriginalNumLines;
+  bool DiscardLines;
 };
 
 class CompoundStatementIndenter {
@@ -157,30 +165,30 @@ UnwrappedLineParser::UnwrappedLineParser(
     ArrayRef<FormatToken *> Tokens, UnwrappedLineConsumer &Callback,
     llvm::SpecificBumpPtrAllocator<FormatToken> &Allocator,
     IdentifierTable &IdentTable)
-    : Line(new UnwrappedLine), AtEndOfPPLine(false), CurrentLines(&Lines),
-      Style(Style), IsCpp(Style.isCpp()),
-      LangOpts(getFormattingLangOpts(Style)), Keywords(Keywords),
-      CommentPragmasRegex(Style.CommentPragmas), Tokens(nullptr),
-      Callback(Callback), AllTokens(Tokens), PPBranchLevel(-1),
-      IncludeGuard(getIncludeGuardState(Style.IndentPPDirectives)),
-      IncludeGuardToken(nullptr), FirstStartColumn(FirstStartColumn),
+    : Line(new UnwrappedLine), CurrentLines(&Lines), Style(Style),
+      IsCpp(Style.isCpp()), LangOpts(getFormattingLangOpts(Style)),
+      Keywords(Keywords), CommentPragmasRegex(Style.CommentPragmas),
+      Tokens(nullptr), Callback(Callback), AllTokens(Tokens),
+      PP(getIncludeGuardState(Style.IndentPPDirectives)),
+      FirstStartColumn(FirstStartColumn),
       Macros(Style.Macros, SourceMgr, Style, Allocator, IdentTable) {}
 
 void UnwrappedLineParser::reset() {
-  PPBranchLevel = -1;
-  IncludeGuard = getIncludeGuardState(Style.IndentPPDirectives);
-  IncludeGuardToken = nullptr;
+  PP.BranchLevel = -1;
+  PP.IncludeGuard = getIncludeGuardState(Style.IndentPPDirectives);
+  PP.IncludeGuardToken = nullptr;
+  ParsedPPDirectives.clear();
   Line.reset(new UnwrappedLine);
   CommentsBeforeNextToken.clear();
   FormatTok = nullptr;
-  AtEndOfPPLine = false;
+  PP.AtEndOfPPLine = false;
   IsDecltypeAutoFunction = false;
   PreprocessorDirectives.clear();
   CurrentLines = &Lines;
   DeclarationScopeStack.clear();
   NestedTooDeep.clear();
   NestedLambdas.clear();
-  PPStack.clear();
+  PP.Stack.clear();
   Line->FirstStartColumn = FirstStartColumn;
 
   if (!Unexpanded.empty())
@@ -207,7 +215,7 @@ void UnwrappedLineParser::parse() {
 
     // If we found an include guard then all preprocessor directives (other than
     // the guard) are over-indented by one.
-    if (IncludeGuard == IG_Found) {
+    if (PP.IncludeGuard == IG_Found) {
       for (auto &Line : Lines)
         if (Line.InPPDirective && Line.Level > 0)
           --Line.Level;
@@ -246,17 +254,17 @@ void UnwrappedLineParser::parse() {
     }
     Callback.finishRun();
     Lines.clear();
-    while (!PPLevelBranchIndex.empty() &&
-           PPLevelBranchIndex.back() + 1 >= PPLevelBranchCount.back()) {
-      PPLevelBranchIndex.resize(PPLevelBranchIndex.size() - 1);
-      PPLevelBranchCount.resize(PPLevelBranchCount.size() - 1);
+    while (!PP.LevelBranchIndex.empty() &&
+           PP.LevelBranchIndex.back() + 1 >= PP.LevelBranchCount.back()) {
+      PP.LevelBranchIndex.resize(PP.LevelBranchIndex.size() - 1);
+      PP.LevelBranchCount.resize(PP.LevelBranchCount.size() - 1);
     }
-    if (!PPLevelBranchIndex.empty()) {
-      ++PPLevelBranchIndex.back();
-      assert(PPLevelBranchIndex.size() == PPLevelBranchCount.size());
-      assert(PPLevelBranchIndex.back() <= PPLevelBranchCount.back());
+    if (!PP.LevelBranchIndex.empty()) {
+      ++PP.LevelBranchIndex.back();
+      assert(PP.LevelBranchIndex.size() == PP.LevelBranchCount.size());
+      assert(PP.LevelBranchIndex.back() <= PP.LevelBranchCount.back());
     }
-  } while (!PPLevelBranchIndex.empty());
+  } while (!PP.LevelBranchIndex.empty());
 }
 
 void UnwrappedLineParser::parseFile() {
@@ -666,7 +674,7 @@ static inline void hash_combine(std::size_t &seed, const T &v) {
 
 size_t UnwrappedLineParser::computePPHash() const {
   size_t h = 0;
-  for (const auto &i : PPStack) {
+  for (const auto &i : PP.Stack) {
     hash_combine(h, size_t(i.Kind));
     hash_combine(h, i.Line);
   }
@@ -1056,49 +1064,50 @@ void UnwrappedLineParser::conditionalCompilationCondition(bool Unreachable) {
     Line += Lines.size();
 
   if (Unreachable ||
-      (!PPStack.empty() && PPStack.back().Kind == PP_Unreachable)) {
-    PPStack.push_back({PP_Unreachable, Line});
+      (!PP.Stack.empty() && PP.Stack.back().Kind == PP_Unreachable)) {
+    PP.Stack.push_back({PP_Unreachable, Line});
   } else {
-    PPStack.push_back({PP_Conditional, Line});
+    PP.Stack.push_back({PP_Conditional, Line});
   }
 }
 
 void UnwrappedLineParser::conditionalCompilationStart(bool Unreachable) {
-  ++PPBranchLevel;
-  assert(PPBranchLevel >= 0 && PPBranchLevel <= (int)PPLevelBranchIndex.size());
-  if (PPBranchLevel == (int)PPLevelBranchIndex.size()) {
-    PPLevelBranchIndex.push_back(0);
-    PPLevelBranchCount.push_back(0);
-  }
-  PPChainBranchIndex.push(Unreachable ? -1 : 0);
-  bool Skip = PPLevelBranchIndex[PPBranchLevel] > 0;
+  ++PP.BranchLevel;
+  assert(PP.BranchLevel >= 0 &&
+         PP.BranchLevel <= (int)PP.LevelBranchIndex.size());
+  if (PP.BranchLevel == (int)PP.LevelBranchIndex.size()) {
+    PP.LevelBranchIndex.push_back(0);
+    PP.LevelBranchCount.push_back(0);
+  }
+  PP.ChainBranchIndex.push(Unreachable ? -1 : 0);
+  bool Skip = PP.LevelBranchIndex[PP.BranchLevel] > 0;
   conditionalCompilationCondition(Unreachable || Skip);
 }
 
 void UnwrappedLineParser::conditionalCompilationAlternative() {
-  if (!PPStack.empty())
-    PPStack.pop_back();
-  assert(PPBranchLevel < (int)PPLevelBranchIndex.size());
-  if (!PPChainBranchIndex.empty())
-    ++PPChainBranchIndex.top();
+  if (!PP.Stack.empty())
+    PP.Stack.pop_back();
+  assert(PP.BranchLevel < (int)PP.LevelBranchIndex.size());
+  if (!PP.ChainBranchIndex.empty())
+    ++PP.ChainBranchIndex.top();
   conditionalCompilationCondition(
-      PPBranchLevel >= 0 && !PPChainBranchIndex.empty() &&
-      PPLevelBranchIndex[PPBranchLevel] != PPChainBranchIndex.top());
+      PP.BranchLevel >= 0 && !PP.ChainBranchIndex.empty() &&
+      PP.LevelBranchIndex[PP.BranchLevel] != PP.ChainBranchIndex.top());
 }
 
 void UnwrappedLineParser::conditionalCompilationEnd() {
-  assert(PPBranchLevel < (int)PPLevelBranchIndex.size());
-  if (PPBranchLevel >= 0 && !PPChainBranchIndex.empty()) {
-    if (PPChainBranchIndex.top() + 1 > PPLevelBranchCount[PPBranchLevel])
-      PPLevelBranchCount[PPBranchLevel] = PPChainBranchIndex.top() + 1;
+  assert(PP.BranchLevel < (int)PP.LevelBranchIndex.size());
+  if (PP.BranchLevel >= 0 && !PP.ChainBranchIndex.empty()) {
+    if (PP.ChainBranchIndex.top() + 1 > PP.LevelBranchCount[PP.BranchLevel])
+      PP.LevelBranchCount[PP.BranchLevel] = PP.ChainBranchIndex.top() + 1;
   }
   // Guard against #endif's without #if.
-  if (PPBranchLevel > -1)
-    --PPBranchLevel;
-  if (!PPChainBranchIndex.empty())
-    PPChainBranchIndex.pop();
-  if (!PPStack.empty())
-    PPStack.pop_back();
+  if (PP.BranchLevel > -1)
+    --PP.BranchLevel;
+  if (!PP.ChainBranchIndex.empty())
+    PP.ChainBranchIndex.pop();
+  if (!PP.Stack.empty())
+    PP.Stack.pop_back();
 }
 
 void UnwrappedLineParser::parsePPIf(bool IfDef) {
@@ -1114,36 +1123,36 @@ void UnwrappedLineParser::parsePPIf(bool IfDef) {
   // If there's a #ifndef on the first line, and the only lines before it are
   // comments, it could be an include guard.
   bool MaybeIncludeGuard = IfNDef;
-  if (IncludeGuard == IG_Inited && MaybeIncludeGuard) {
+  if (PP.IncludeGuard == IG_Inited && MaybeIncludeGuard) {
     for (auto &Line : Lines) {
       if (Line.Tokens.front().Tok->isNot(tok::comment)) {
         MaybeIncludeGuard = false;
-        IncludeGuard = IG_Rejected;
+        PP.IncludeGuard = IG_Rejected;
         break;
       }
     }
   }
-  --PPBranchLevel;
+  --PP.BranchLevel;
   parsePPUnknown();
-  ++PPBranchLevel;
-  if (IncludeGuard == IG_Inited && MaybeIncludeGuard) {
-    IncludeGuard = IG_IfNdefed;
-    IncludeGuardToken = IfCondition;
+  ++PP.BranchLevel;
+  if (PP.IncludeGuard == IG_Inited && MaybeIncludeGuard) {
+    PP.IncludeGuard = IG_IfNdefed;
+    PP.IncludeGuardToken = IfCondition;
   }
 }
 
 void UnwrappedLineParser::parsePPElse() {
   // If a potential include guard has an #else, it's not an include guard.
-  if (IncludeGuard == IG_Defined && PPBranchLevel == 0)
-    IncludeGuard = IG_Rejected;
+  if (PP.IncludeGuard == IG_Defined && PP.BranchLevel == 0)
+    PP.IncludeGuard = IG_Rejected;
   // Don't crash when there is an #else without an #if.
-  assert(PPBranchLevel >= -1);
-  if (PPBranchLevel == -1)
+  assert(PP.BranchLevel >= -1);
+  if (PP.BranchLevel == -1)
     conditionalCompilationStart(/*Unreachable=*/true);
   conditionalCompilationAlternative();
-  --PPBranchLevel;
+  --PP.BranchLevel;
   parsePPUnknown();
-  ++PPBranchLevel;
+  ++PP.BranchLevel;
 }
 
 void UnwrappedLineParser::parsePPEndIf() {
@@ -1155,24 +1164,24 @@ void UnwrappedLineParser::parsePPDefine() {
   nextToken();
 
   if (!FormatTok->Tok.getIdentifierInfo()) {
-    IncludeGuard = IG_Rejected;
-    IncludeGuardToken = nullptr;
+    PP.IncludeGuard = IG_Rejected;
+    PP.IncludeGuardToken = nullptr;
     parsePPUnknown();
     return;
   }
 
   bool MaybeIncludeGuard = false;
-  if (IncludeGuard == IG_IfNdefed &&
-      IncludeGuardToken->TokenText == FormatTok->TokenText) {
-    IncludeGuard = IG_Defined;
-    IncludeGuardToken = nullptr;
+  if (PP.IncludeGuard == IG_IfNdefed &&
+      PP.IncludeGuardToken->TokenText == FormatTok->TokenText) {
+    PP.IncludeGuard = IG_Defined;
+    PP.IncludeGuardToken = nullptr;
     for (auto &Line : Lines) {
       if (Line.Tokens.front().Tok->isNoneOf(tok::comment, tok::hash)) {
-        IncludeGuard = IG_Rejected;
+        PP.IncludeGuard = IG_Rejected;
         break;
       }
     }
-    MaybeIncludeGuard = IncludeGuard == IG_Defined;
+    MaybeIncludeGuard = PP.IncludeGuard == IG_Defined;
   }
 
   // In the context of a define, even keywords should be treated as normal
@@ -1186,16 +1195,16 @@ void UnwrappedLineParser::parsePPDefine() {
 
   // IncludeGuard can't have a non-empty macro definition.
   if (MaybeIncludeGuard && !eof())
-    IncludeGuard = IG_Rejected;
+    PP.IncludeGuard = IG_Rejected;
 
   if (FormatTok->is(tok::l_paren) && !FormatTok->hasWhitespaceBefore())
     parseParens();
   if (Style.IndentPPDirectives != FormatStyle::PPDIS_None)
-    Line->Level += PPBranchLevel + 1;
+    Line->Level += PP.BranchLevel + 1;
   addUnwrappedLine();
   ++Line->Level;
 
-  Line->PPLevel = PPBranchLevel + (IncludeGuard == IG_Defined ? 0 : 1);
+  Line->PPLevel = PP.BranchLevel + (PP.IncludeGuard == IG_Defined ? 0 : 1);
   assert((int)Line->PPLevel >= 0);
 
   if (eof())
@@ -1233,7 +1242,7 @@ void UnwrappedLineParser::parsePPUnknown() {
   while (!eof())
     nextToken();
   if (Style.IndentPPDirectives != FormatStyle::PPDIS_None)
-    Line->Level += PPBranchLevel + 1;
+    Line->Level += PP.BranchLevel + 1;
   addUnwrappedLine();
 }
 
@@ -4766,7 +4775,7 @@ void UnwrappedLineParser::addUnwrappedLine(LineLevel AdjustLevel) {
   } else {
     // At the top level we only get here when no unexpansion is going on, or
     // when conditional formatting led to unfinished macro reconstructions.
-    assert(!Reconstruct || (CurrentLines != &Lines) || !PPStack.empty());
+    assert(!Reconstruct || (CurrentLines != &Lines) || !PP.Stack.empty());
     CurrentLines->push_back(std::move(*Line));
   }
   Line->Tokens.clear();
@@ -5051,10 +5060,15 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
       }
       distributeComments(Comments, FormatTok);
       Comments.clear();
+      // If the directive was parsed before the token stream was rewound (see
+      // parseMacroCall()), its lines were kept. Parse it again only for its
+      // effect on the preprocessor bookkeeping and discard the new lines.
+      const bool ParsedBefore = !ParsedPPDirectives.insert(FormatTok).second;
       // If there is an unfinished unwrapped line, we flush the preprocessor
       // directives only after that unwrapped line was finished later.
       bool SwitchToPreprocessorLines = !Line->Tokens.empty();
-      ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
+      ScopedLineState BlockState(*this, SwitchToPreprocessorLines,
+                                 /*DiscardLines=*/ParsedBefore);
       assert((LevelDifference >= 0 ||
               static_cast<unsigned>(-LevelDifference) <= Line->Level) &&
              "LevelDifference makes Line->Level negative");
@@ -5063,8 +5077,8 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
       // before the preprocessor directive, at the same level as the
       // preprocessor directive, as we consider them to apply to the directive.
       if (Style.IndentPPDirectives == FormatStyle::PPDIS_BeforeHash &&
-          PPBranchLevel > 0) {
-        Line->Level += PPBranchLevel;
+          PP.BranchLevel > 0) {
+        Line->Level += PP.BranchLevel;
       }
       assert(Line->Level >= Line->UnbracedBodyLevel);
       Line->Level -= Line->UnbracedBodyLevel;
@@ -5076,16 +5090,16 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
           FirstNonCommentOnLine, *FormatTok, PreviousWasComment);
       // If the #endif of a potential include guard is the last thing in the
       // file, then we found an include guard.
-      if (IsEndIf && IncludeGuard == IG_Defined && PPBranchLevel == -1 &&
+      if (IsEndIf && PP.IncludeGuard == IG_Defined && PP.BranchLevel == -1 &&
           getIncludeGuardState(Style.IndentPPDirectives) == IG_Inited &&
           (eof() ||
            (PreviousWasComment &&
             Tokens->peekNextToken(/*SkipComment=*/true)->is(tok::eof)))) {
-        IncludeGuard = IG_Found;
+        PP.IncludeGuard = IG_Found;
       }
     }
 
-    if (!PPStack.empty() && (PPStack.back().Kind == PP_Unreachable) &&
+    if (!PP.Stack.empty() && (PP.Stack.back().Kind == PP_Unreachable) &&
         !Line->InPPDirective) {
       continue;
     }
@@ -5096,6 +5110,11 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
         !Line->InPPDirective) {
       FormatToken *ID = FormatTok;
       unsigned Position = Tokens->getPosition();
+      // Parsing the arguments of the call may parse preprocessor directives,
+      // which are parsed again if the token stream is rewound because the
+      // arguments are discarded. The preprocessor bookkeeping is restored
+      // whenever that happens.
+      const auto SavedPPState = PP;
 
       // To correctly parse the code, we need to replace the tokens of the macro
       // call with its expansion.
@@ -5104,7 +5123,7 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
       bool OldInExpansion = InExpansion;
       InExpansion = true;
       // We parse the macro call into a new line.
-      auto Args = parseMacroCall();
+      auto Args = parseMacroCall(SavedPPState);
       InExpansion = OldInExpansion;
       assert(Line->Tokens.front().Tok == ID);
       // And remember the unexpanded macro call tokens.
@@ -5139,6 +5158,7 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
         Tokens->setPosition(Position);
         // Not nextToken(), which would push the stale FormatTok onto the line.
         FormatTok = Tokens->getNextToken();
+        PP = SavedPPState;
         assert(!Args && Macros.objectLike(ID->TokenText));
       }
       if ((!Args && Macros.objectLike(ID->TokenText)) ||
@@ -5169,6 +5189,7 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
         });
         Tokens->setPosition(Position);
         FormatTok = ID;
+        PP = SavedPPState;
       }
     }
 
@@ -5198,7 +5219,7 @@ void pushTokens(Iterator Begin, Iterator End,
 } // namespace
 
 std::optional<llvm::SmallVector<llvm::SmallVector<FormatToken *, 8>, 1>>
-UnwrappedLineParser::parseMacroCall() {
+UnwrappedLineParser::parseMacroCall(const PPState &SavedPPState) {
   std::optional<llvm::SmallVector<llvm::SmallVector<FormatToken *, 8>, 1>> Args;
   assert(Line->Tokens.empty());
   // Not nextToken(), which would already expand a directly following macro
@@ -5257,17 +5278,18 @@ UnwrappedLineParser::parseMacroCall() {
   Line->Tokens.resize(1);
   Tokens->setPosition(Position);
   FormatTok = Tok;
+  PP = SavedPPState;
   return {};
 }
 
 void UnwrappedLineParser::pushToken(FormatToken *Tok) {
   Line->Tokens.push_back(UnwrappedLineNode(Tok));
-  if (AtEndOfPPLine) {
+  if (PP.AtEndOfPPLine) {
     auto &Tok = *Line->Tokens.back().Tok;
     Tok.MustBreakBefore = true;
     Tok.MustBreakBeforeFinalized = true;
     Tok.FirstAfterPPLine = true;
-    AtEndOfPPLine = false;
+    PP.AtEndOfPPLine = false;
   }
 }
 

diff  --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 8fa4e9f7540d5..5b93c8f346d75 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -16,6 +16,7 @@
 #define LLVM_CLANG_LIB_FORMAT_UNWRAPPEDLINEPARSER_H
 
 #include "Macros.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include <stack>
 
 namespace clang {
@@ -38,7 +39,7 @@ struct UnwrappedLine {
   /// The indent level of the \c UnwrappedLine.
   unsigned Level = 0;
 
-  /// The \c PPBranchLevel (adjusted for header guards) if this line is a
+  /// The \c PPState::BranchLevel (adjusted for header guards) if this line is a
   /// \c InMacroBody line, and 0 otherwise.
   unsigned PPLevel = 0;
 
@@ -213,8 +214,9 @@ class UnwrappedLineParser {
   void parseVerilogExtern();
   // Skip things that can precede the keywords like module.
   void skipVerilogQualifiers();
+  struct PPState;
   std::optional<llvm::SmallVector<llvm::SmallVector<FormatToken *, 8>, 1>>
-  parseMacroCall();
+  parseMacroCall(const PPState &SavedPPState);
 
   // Used by addUnwrappedLine to denote whether to keep or remove a level
   // when resetting the line state.
@@ -310,9 +312,6 @@ class UnwrappedLineParser {
 
   FormatToken *FormatTok = nullptr;
 
-  // Has just finished parsing a preprocessor line.
-  bool AtEndOfPPLine;
-
   // The parsed lines. Only added to through \c CurrentLines.
   SmallVector<UnwrappedLine, 8> Lines;
 
@@ -368,30 +367,6 @@ class UnwrappedLineParser {
     size_t Line;
   };
 
-  // Keeps a stack of currently active preprocessor branching directives.
-  SmallVector<PPBranch, 16> PPStack;
-
-  // The \c UnwrappedLineParser re-parses the code for each combination
-  // of preprocessor branches that can be taken.
-  // To that end, we take the same branch (#if, #else, or one of the #elif
-  // branches) for each nesting level of preprocessor branches.
-  // \c PPBranchLevel stores the current nesting level of preprocessor
-  // branches during one pass over the code.
-  int PPBranchLevel;
-
-  // Contains the current branch (#if, #else or one of the #elif branches)
-  // for each nesting level.
-  SmallVector<int, 8> PPLevelBranchIndex;
-
-  // Contains the maximum number of branches at each nesting level.
-  SmallVector<int, 8> PPLevelBranchCount;
-
-  // Contains the number of branches per nesting level we are currently
-  // in while parsing a preprocessor branch sequence.
-  // This is used to update PPLevelBranchCount at the end of a branch
-  // sequence.
-  std::stack<int> PPChainBranchIndex;
-
   // Include guard search state. Used to fixup preprocessor indent levels
   // so that include guards do not participate in indentation.
   enum IncludeGuardState {
@@ -402,9 +377,6 @@ class UnwrappedLineParser {
     IG_Rejected, // Search failed or never started.
   };
 
-  // Current state of include guard search.
-  IncludeGuardState IncludeGuard;
-
   IncludeGuardState
   getIncludeGuardState(FormatStyle::PPDirectiveIndentStyle Style) const {
     return Style == FormatStyle::PPDIS_None || Style == FormatStyle::PPDIS_Leave
@@ -412,9 +384,53 @@ class UnwrappedLineParser {
                : IG_Inited;
   }
 
-  // Points to the #ifndef condition for a potential include guard. Null unless
-  // IncludeGuardState == IG_IfNdefed.
-  FormatToken *IncludeGuardToken;
+  // The preprocessor bookkeeping that is rolled back when the token stream is
+  // rewound over preprocessor directives, which happens if the speculatively
+  // parsed arguments of a macro call are discarded. See readToken().
+  struct PPState {
+    explicit PPState(IncludeGuardState IncludeGuard)
+        : IncludeGuard(IncludeGuard) {}
+
+    // Keeps a stack of currently active preprocessor branching directives.
+    SmallVector<PPBranch, 16> Stack;
+
+    // The \c UnwrappedLineParser re-parses the code for each combination
+    // of preprocessor branches that can be taken.
+    // To that end, we take the same branch (#if, #else, or one of the #elif
+    // branches) for each nesting level of preprocessor branches.
+    // \c BranchLevel stores the current nesting level of preprocessor
+    // branches during one pass over the code.
+    int BranchLevel = -1;
+
+    // Contains the current branch (#if, #else or one of the #elif branches)
+    // for each nesting level.
+    SmallVector<int, 8> LevelBranchIndex;
+
+    // Contains the maximum number of branches at each nesting level.
+    SmallVector<int, 8> LevelBranchCount;
+
+    // Contains the number of branches per nesting level we are currently
+    // in while parsing a preprocessor branch sequence.
+    // This is used to update LevelBranchCount at the end of a branch
+    // sequence.
+    std::stack<int> ChainBranchIndex;
+
+    // Current state of include guard search.
+    IncludeGuardState IncludeGuard;
+
+    // Points to the #ifndef condition for a potential include guard. Null
+    // unless IncludeGuardState == IG_IfNdefed.
+    FormatToken *IncludeGuardToken = nullptr;
+
+    // Has just finished parsing a preprocessor line.
+    bool AtEndOfPPLine = false;
+  } PP;
+
+  // The hash tokens of the parsed preprocessor directives. The lines of a
+  // directive are kept when the token stream is rewound over it, so it is then
+  // parsed again only for its effect on the preprocessor bookkeeping. See
+  // readToken().
+  llvm::SmallPtrSet<const FormatToken *, 16> ParsedPPDirectives;
 
   // Contains the first start column where the source begins. This is zero for
   // normal source code and may be nonzero when formatting a code fragment that

diff  --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp
index 26d7aa05e1561..ca8a5180c32d5 100644
--- a/clang/unittests/Format/FormatTestMacroExpansion.cpp
+++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp
@@ -299,6 +299,100 @@ TEST_F(FormatTestMacroExpansion, IndentChildrenWithinMacroCall) {
                Style);
 }
 
+TEST_F(FormatTestMacroExpansion, PPDirectiveInDiscardedMacroArgs) {
+  FormatStyle Style = getLLVMStyle();
+  Style.Macros = {"A=a", "ID(x)=x", "PAIR(x, y)=x y", "STMT=f();", "EMPTY="};
+
+  verifyIncompleteFormat("A(\n"
+                         "#endif",
+                         Style);
+  verifyIncompleteFormat("ID(\n"
+                         "#endif",
+                         Style);
+  verifyIncompleteFormat("ID(\n"
+                         "#define X 1",
+                         Style);
+  verifyFormat("A(\n"
+               "#if X\n"
+               "    b;\n"
+               "#endif\n"
+               ")",
+               Style);
+  verifyFormat("ID(a,\n"
+               "#if X\n"
+               "   b\n"
+               "#endif\n"
+               ");",
+               Style);
+  verifyFormat("PAIR(\n"
+               "#define X ,\n"
+               "    a)",
+               Style);
+  verifyFormat("ID(\n"
+               "#if 0\n"
+               ",\n"
+               "#endif\n"
+               "    if (a) {\n"
+               "      f();\n"
+               "    })",
+               Style);
+  verifyFormat("STMT\n"
+               "#define F(x) g(x)\n"
+               "b;",
+               "STMT\n"
+               "#define F(x) g( x )\n"
+               "b;",
+               Style);
+  verifyFormat("EMPTY(\n"
+               "#define F(x) g(x)\n"
+               "1)",
+               "EMPTY(\n"
+               "#define F(x)  g(x)\n"
+               "1)",
+               Style);
+  EXPECT_EQ("A(\n"
+            "ID(\n"
+            "#define F(x) g(x)\n"
+            "))",
+            format("A(ID(\n"
+                   "#define F(x)  g(x)\n"
+                   "))",
+                   Style, SC_ExpectIncomplete));
+
+  Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+  verifyFormat("#if OUTER\n"
+               "EMPTY(\n"
+               "  #define X 1\n"
+               ")\n"
+               "#endif",
+               Style);
+  verifyFormat("void f() {\n"
+               "  if (x) {\n"
+               "    ID(a,\n"
+               "#if Y\n"
+               "  #define Z 1\n"
+               "#endif\n"
+               "    );\n"
+               "  }\n"
+               "}",
+               Style);
+}
+
+TEST_F(FormatTestMacroExpansion, PPDirectiveBeforeEmptyExpansion) {
+  FormatStyle Style = getLLVMStyle();
+  Style.Macros.push_back("ID(x)=x");
+  verifyFormat("#define X 1\n"
+               "ID()",
+               "#define X   1\n"
+               "ID()",
+               Style);
+  verifyFormat("#define X 1\n"
+               "ID()",
+               "#define X   1\n"
+               "ID()",
+               Style, {tooling::Range(0, 13)}); // line 1
+}
+
 TEST_F(FormatTestMacroExpansion, ObjectLikeMacroCalledWithArgsDoesNotHang) {
   FormatStyle Style = getLLVMStyle();
   Style.Macros.push_back("CASE=case");


        


More information about the cfe-commits mailing list