-
Notifications
You must be signed in to change notification settings - Fork 11.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang-format] Correctly annotate braces in macro definition (#107352) #107531
Conversation
@llvm/pr-subscribers-clang-format Author: Owen Pan (owenca) ChangesThis reverts commit 2d90e8f and backports commit 616a8ce. Full diff: https://github.com/llvm/llvm-project/pull/107531.diff 2 Files Affected:
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 60e65aaa83e9c1..7813d86ff0ea10 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -570,7 +570,8 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
NextTok->isOneOf(Keywords.kw_of, Keywords.kw_in,
Keywords.kw_as));
ProbablyBracedList =
- ProbablyBracedList || (IsCpp && NextTok->is(tok::l_paren));
+ ProbablyBracedList || (IsCpp && (PrevTok->Tok.isLiteral() ||
+ NextTok->is(tok::l_paren)));
// If there is a comma, semicolon or right paren after the closing
// brace, we assume this is a braced initializer list.
@@ -609,8 +610,9 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
ProbablyBracedList = NextTok->isNot(tok::l_square);
}
- // Cpp macro definition body containing nonempty braced list or block:
+ // 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.
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index db580d70058811..dd58fbc70cb91e 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -3219,6 +3219,21 @@ TEST_F(TokenAnnotatorTest, BraceKind) {
EXPECT_TOKEN(Tokens[11], tok::r_brace, TT_StructRBrace);
EXPECT_BRACE_KIND(Tokens[11], BK_Block);
+ Tokens = annotate("#define MACRO \\\n"
+ " struct hash<type> { \\\n"
+ " void f() { return; } \\\n"
+ " };");
+ ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+ EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_StructLBrace);
+ EXPECT_BRACE_KIND(Tokens[8], BK_Block);
+ EXPECT_TOKEN(Tokens[10], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[11], tok::l_paren, TT_FunctionDeclarationLParen);
+ EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
+ EXPECT_BRACE_KIND(Tokens[13], BK_Block);
+ EXPECT_BRACE_KIND(Tokens[16], BK_Block);
+ EXPECT_TOKEN(Tokens[17], tok::r_brace, TT_StructRBrace);
+ EXPECT_BRACE_KIND(Tokens[17], BK_Block);
+
Tokens = annotate("#define MEMBER(NAME) NAME{\"\"}");
ASSERT_EQ(Tokens.size(), 11u) << Tokens;
EXPECT_BRACE_KIND(Tokens[7], BK_BracedInit);
|
Any hope that this gets reviewed and merged for the release? |
Hi, since we are wrapping up LLVM 19.1.0 we are very strict with the fixes we pick at this point. Can you please respond to the following questions to help me understand if this has to be included in the final release or not. Is this PR a fix for a regression or a critical issue? What is the risk of accepting this into the release branch? What is the risk of NOT accepting this into the release branch? |
This is a fix for a regression that you merged in |
This is a fix for a regression as labeled.
The risk of accepting this is very low IMO.
The risk of NOT accepting this is 100%. |
@owenca (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
This reverts commit 2d90e8f and backports commit 616a8ce.