Skip to content

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Aug 21, 2025

Fixes #154683

@llvmbot
Copy link
Member

llvmbot commented Aug 21, 2025

@llvm/pr-subscribers-clang-format

Author: owenca (owenca)

Changes

Fixes #154683


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

2 Files Affected:

  • (modified) clang/lib/Format/UnwrappedLineParser.cpp (+6-3)
  • (modified) clang/unittests/Format/FormatTest.cpp (+4)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 91b8fdc8a3c38..23938f8cb9d00 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1182,10 +1182,8 @@ void UnwrappedLineParser::parsePPDefine() {
   if (MaybeIncludeGuard && !eof())
     IncludeGuard = IG_Rejected;
 
-  if (FormatTok->Tok.getKind() == tok::l_paren &&
-      !FormatTok->hasWhitespaceBefore()) {
+  if (FormatTok->is(tok::l_paren) && !FormatTok->hasWhitespaceBefore())
     parseParens();
-  }
   if (Style.IndentPPDirectives != FormatStyle::PPDIS_None)
     Line->Level += PPBranchLevel + 1;
   addUnwrappedLine();
@@ -1196,6 +1194,11 @@ void UnwrappedLineParser::parsePPDefine() {
   Line->InMacroBody = true;
 
   if (Style.SkipMacroDefinitionBody) {
+    if (auto *Prev = Tokens->getPreviousToken(); Prev->is(tok::comment) &&
+                                                 Prev->NewlinesBefore > 0 &&
+                                                 !Prev->HasUnescapedNewline) {
+      Prev->Finalized = true;
+    }
     while (!eof()) {
       FormatTok->Finalized = true;
       FormatTok = Tokens->getNextToken();
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 83c664c3b81f3..1ef6877941c7d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -26022,6 +26022,10 @@ TEST_F(FormatTest, SkipMacroDefinitionBody) {
                  "  A  a \\\n "
                  " A  a",
                  Style);
+  verifyNoChange("#define MY_MACRO          \\\n"
+                 " /* this is a comment */ \\\n"
+                 "   1",
+                 Style);
 }
 
 TEST_F(FormatTest, VeryLongNamespaceCommentSplit) {

@owenca owenca merged commit 0b1d353 into llvm:main Aug 23, 2025
9 checks passed
@owenca owenca deleted the 154683 branch August 23, 2025 02:37
@anonymouspc
Copy link

Still has bug.

Before (Matrix.h):

#define EIGEN_MAKE_TYPEDEFS(Size, SizeSuffix)                    \
  /** \ingroup matrixtypedefs */                                 \
  /** \brief \cpp11 `Size`×`Size` matrix of type `Type`.*/ \
  template <typename Type>                                       \
  using Matrix##SizeSuffix = Matrix<Type, Size, Size>;           \

Version (updated to newest after this PR, build from souce, Linux):

clang-format --version
clang-format version 22.0.0git (https://github.com/llvm/llvm-project f8f6965ceece9e330ddb66db5f402ecfb5e3ad34)

Command:

clang-format --style="{SkipMacroDefinitionBody: true}" -i Matrix.h

After:

#define EIGEN_MAKE_TYPEDEFS(Size, SizeSuffix) // <-- NO '\' HERE!
/** \ingroup matrixtypedefs */                                 \
  /** \brief \cpp11 `Size`&times;`Size` matrix of type `Type`.*/ \
  template <typename Type>                                       \
  using Matrix##SizeSuffix = Matrix<Type, Size, Size>;           \

I tried best to reduce it into this case, you may reduce it further :)
Thank you :)

@owenca
Copy link
Contributor Author

owenca commented Aug 26, 2025

See #155346.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang-format] wrongly formats macro when SkipMacroDefinitionBody is true
4 participants