[clang-format] Correctly annotate braces in macros #87953
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also fix unit tests and reformat polly. Fixes llvm#86550.
|
@llvm/pr-subscribers-clang-format Author: Owen Pan (owenca) ChangesAlso fix unit tests and reformat polly. Fixes #86550. Full diff: https://github.com/llvm/llvm-project/pull/87953.diff 4 Files Affected:
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index af57b1420c6ede..c1f7e2874beb24 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -538,16 +538,6 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
if (Style.Language == FormatStyle::LK_Proto) {
ProbablyBracedList = NextTok->isOneOf(tok::comma, tok::r_square);
} else {
- // Skip NextTok over preprocessor lines, otherwise we may not
- // properly diagnose the block as a braced intializer
- // if the comma separator appears after the pp directive.
- while (NextTok->is(tok::hash)) {
- ScopedMacroState MacroState(*Line, Tokens, NextTok);
- do {
- NextTok = Tokens->getNextToken();
- } while (NextTok->isNot(tok::eof));
- }
-
// Using OriginalColumn to distinguish between ObjC methods and
// binary operators is a bit hacky.
bool NextIsObjCMethod = NextTok->isOneOf(tok::plus, tok::minus) &&
@@ -606,6 +596,16 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
NextTok = Tokens->getNextToken();
ProbablyBracedList = NextTok->isNot(tok::l_square);
}
+
+ // Cpp macro definition body that is a nonempty braced list or block:
+ if (IsCpp && Line->InMacroBody && PrevTok != FormatTok &&
+ !FormatTok->Previous && NextTok->is(tok::eof) &&
+ // A statement can end with only `;` (simple statement), a block
+ // closing brace (compound statement), or `:` (label statement).
+ // If PrevTok is a block opening brace, Tok ends an empty block.
+ !PrevTok->isOneOf(tok::semi, BK_Block, tok::colon)) {
+ ProbablyBracedList = true;
+ }
}
if (ProbablyBracedList) {
Tok->setBlockKind(BK_BracedInit);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 91a8ff11889d6f..1d8745428009e0 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -1865,6 +1865,13 @@ TEST_F(FormatTest, UnderstandsMacros) {
verifyFormat("MACRO(co_return##something)");
verifyFormat("#define A x:");
+
+ verifyFormat("#define Foo(Bar) {#Bar}", "#define Foo(Bar) \\\n"
+ " { \\\n"
+ " #Bar \\\n"
+ " }");
+ verifyFormat("#define Foo(Bar) {#Bar}", "#define Foo(Bar) \\\n"
+ " { #Bar }");
}
TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
@@ -11035,7 +11042,7 @@ TEST_F(FormatTest, UnderstandsTemplateParameters) {
verifyFormat("some_templated_type<decltype([](int i) { return i; })>");
verifyFormat("#define FOO(typeName, realClass) \\\n"
- " { #typeName, foo<FooType>(new foo<realClass>(#typeName)) }",
+ " {#typeName, foo<FooType>(new foo<realClass>(#typeName))}",
getLLVMStyleWithColumns(60));
}
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 5ba5e0fbd16f9e..251e317c7499cf 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1933,14 +1933,20 @@ TEST_F(TokenAnnotatorTest, UnderstandHashInMacro) {
" #Bar \\\n"
" }");
ASSERT_EQ(Tokens.size(), 11u) << Tokens;
- EXPECT_BRACE_KIND(Tokens[6], BK_Block);
- EXPECT_BRACE_KIND(Tokens[9], BK_Block);
+ EXPECT_BRACE_KIND(Tokens[6], BK_BracedInit);
+ EXPECT_BRACE_KIND(Tokens[9], BK_BracedInit);
Tokens = annotate("#define Foo(Bar) \\\n"
" { #Bar }");
ASSERT_EQ(Tokens.size(), 11u) << Tokens;
- EXPECT_BRACE_KIND(Tokens[6], BK_Block);
- EXPECT_BRACE_KIND(Tokens[9], BK_Block);
+ EXPECT_BRACE_KIND(Tokens[6], BK_BracedInit);
+ EXPECT_BRACE_KIND(Tokens[9], BK_BracedInit);
+
+ Tokens = annotate("#define FOO(typeName, realClass) \\\n"
+ " {#typeName, foo<Foo>(new foo<realClass>(#typeName))}");
+ ASSERT_EQ(Tokens.size(), 29u) << Tokens;
+ EXPECT_BRACE_KIND(Tokens[8], BK_BracedInit);
+ EXPECT_BRACE_KIND(Tokens[27], BK_BracedInit);
}
TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacros) {
@@ -2822,6 +2828,13 @@ TEST_F(TokenAnnotatorTest, BraceKind) {
EXPECT_BRACE_KIND(Tokens[0], BK_Block);
EXPECT_BRACE_KIND(Tokens[7], BK_BracedInit);
EXPECT_BRACE_KIND(Tokens[21], BK_BracedInit);
+
+ Tokens =
+ annotate("#define SCOP_STAT(NAME, DESC) \\\n"
+ " {\"polly\", #NAME, \"Number of rejected regions: \" DESC}");
+ ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+ EXPECT_BRACE_KIND(Tokens[8], BK_BracedInit);
+ EXPECT_BRACE_KIND(Tokens[16], BK_BracedInit);
}
TEST_F(TokenAnnotatorTest, StreamOperator) {
diff --git a/polly/lib/Analysis/ScopDetectionDiagnostic.cpp b/polly/lib/Analysis/ScopDetectionDiagnostic.cpp
index 30fbd17c78bfe7..d2fbcf7319856a 100644
--- a/polly/lib/Analysis/ScopDetectionDiagnostic.cpp
+++ b/polly/lib/Analysis/ScopDetectionDiagnostic.cpp
@@ -45,7 +45,7 @@ using namespace llvm;
#define DEBUG_TYPE "polly-detect"
#define SCOP_STAT(NAME, DESC) \
- { "polly-detect", "NAME", "Number of rejected regions: " DESC }
+ {"polly-detect", "NAME", "Number of rejected regions: " DESC}
static Statistic RejectStatistics[] = {
SCOP_STAT(CFG, ""),
|
You can test this locally with the following command:git-clang-format --diff 943db678dadd6088629d08ec3e582bea0595f2d2 6d0c7e5602a227b1b7310be46553aa689e6a93e7 -- clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/FormatTest.cpp clang/unittests/Format/TokenAnnotatorTest.cpp polly/lib/Analysis/ScopDetectionDiagnostic.cppView the diff from clang-format here.diff --git a/polly/lib/Analysis/ScopDetectionDiagnostic.cpp b/polly/lib/Analysis/ScopDetectionDiagnostic.cpp
index d2fbcf7319..30fbd17c78 100644
--- a/polly/lib/Analysis/ScopDetectionDiagnostic.cpp
+++ b/polly/lib/Analysis/ScopDetectionDiagnostic.cpp
@@ -45,7 +45,7 @@ using namespace llvm;
#define DEBUG_TYPE "polly-detect"
#define SCOP_STAT(NAME, DESC) \
- {"polly-detect", "NAME", "Number of rejected regions: " DESC}
+ { "polly-detect", "NAME", "Number of rejected regions: " DESC }
static Statistic RejectStatistics[] = {
SCOP_STAT(CFG, ""),
|
owenca
commented
Apr 8, 2024
Comment on lines
-544
to
-549
| while (NextTok->is(tok::hash)) { | ||
| ScopedMacroState MacroState(*Line, Tokens, NextTok); | ||
| do { | ||
| NextTok = Tokens->getNextToken(); | ||
| } while (NextTok->isNot(tok::eof)); | ||
| } |
There was a problem hiding this comment.
This loop is superfluous due to line 498 above.
|
The polly formatting failure should be ignored as CI is using clang-format 18.1.1 instead of the version built from this patch. |
HazardyKnusperkeks
approved these changes
Apr 9, 2024
|
@tstellar somehow this is not in 18.1.4, but the LLVM Release Status in the Projects box says "Status: Done". Did I miss something here? |
|
@owenca Is there a PR for the cherry-pick ? |
|
Ah, I still need to do that. |
|
/cherry-pick 58323de |
|
Failed to cherry-pick: 58323de https://github.com/llvm/llvm-project/actions/runs/8756326045 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
owenca
added a commit
to owenca/llvm-project
that referenced
this pull request
Apr 20, 2024
owenca
added a commit
to owenca/llvm-project
that referenced
this pull request
Apr 25, 2024
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also fix unit tests and reformat polly.
Fixes #86550.