[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
owenca wrote: > @owenca Do you have a release note we can put in the release announcement? Release note: Fixes a clang-format regression (since 17.0.6) on formatting `goto` labels in macro definitions. https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
https://github.com/tstellar closed https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
tstellar wrote: @owenca Do you have a release note we can put in the release announcement? https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
https://github.com/tstellar updated
https://github.com/llvm/llvm-project/pull/93494
>From 768118d1ad38bf13c545828f67bd6b474d61fc55 Mon Sep 17 00:00:00 2001
From: Owen Pan
Date: Tue, 21 May 2024 01:35:31 -0700
Subject: [PATCH] [clang-format] Fix a bug in formatting goto labels in macros
(#92494)
Fixes #92300.
(cherry picked from commit d89f20058b45e3836527e816af7ed7372e1d554d)
---
clang/lib/Format/UnwrappedLineParser.cpp | 9 ++---
clang/unittests/Format/FormatTest.cpp | 8
clang/unittests/Format/TokenAnnotatorTest.cpp | 13 +
3 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp
b/clang/lib/Format/UnwrappedLineParser.cpp
index f70affb732a0d..179d77bf00491 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1185,12 +1185,6 @@ void UnwrappedLineParser::parsePPDefine() {
return;
}
- if (FormatTok->is(tok::identifier) &&
- Tokens->peekNextToken()->is(tok::colon)) {
-nextToken();
-nextToken();
- }
-
// Errors during a preprocessor directive can only affect the layout of the
// preprocessor directive, and thus we ignore them. An alternative approach
// would be to use the same approach we use on the file level (no
@@ -1671,7 +1665,8 @@ void UnwrappedLineParser::parseStructuralElement(
if (!Style.isJavaScript() && !Style.isVerilog() && !Style.isTableGen() &&
Tokens->peekNextToken()->is(tok::colon) && !Line->MustBeDeclaration) {
nextToken();
- Line->Tokens.begin()->Tok->MustBreakBefore = true;
+ if (!Line->InMacroBody || CurrentLines->size() > 1)
+Line->Tokens.begin()->Tok->MustBreakBefore = true;
FormatTok->setFinalizedType(TT_GotoLabelColon);
parseLabel(!Style.IndentGotoLabels);
if (HasLabel)
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index d69632f7f0f8c..11ae41bc78ace 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -3118,6 +3118,7 @@ TEST_F(FormatTest, FormatsLabels) {
"g();\n"
" }\n"
"}");
+
FormatStyle Style = getLLVMStyle();
Style.IndentGotoLabels = false;
verifyFormat("void f() {\n"
@@ -3157,6 +3158,13 @@ TEST_F(FormatTest, FormatsLabels) {
" }\n"
"}",
Style);
+
+ Style.ColumnLimit = 15;
+ verifyFormat("#define FOO \\\n"
+ "label:\\\n"
+ " break;",
+ Style);
+
// The opening brace may either be on the same unwrapped line as the colon or
// on a separate one. The formatter should recognize both.
Style = getLLVMStyle();
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 44ebad9d5a872..dfa9c22430f36 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2353,15 +2353,28 @@ TEST_F(TokenAnnotatorTest, UnderstandsLabels) {
auto Tokens = annotate("{ x: break; }");
ASSERT_EQ(Tokens.size(), 7u) << Tokens;
EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+
Tokens = annotate("{ case x: break; }");
ASSERT_EQ(Tokens.size(), 8u) << Tokens;
EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+
Tokens = annotate("{ x: { break; } }");
ASSERT_EQ(Tokens.size(), 9u) << Tokens;
EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+
Tokens = annotate("{ case x: { break; } }");
ASSERT_EQ(Tokens.size(), 10u) << Tokens;
EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+
+ Tokens = annotate("#define FOO label:");
+ ASSERT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon);
+
+ Tokens = annotate("#define FOO \\\n"
+"label: \\\n"
+" break;");
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon);
}
TEST_F(TokenAnnotatorTest, UnderstandsNestedBlocks) {
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
owenca wrote: @tstellar > 1. How risky is this fix? Very low IMO. (@mydeveloperday @HazardyKnusperkeks @rymiel can chime in if anyone thinks otherwise.) > 2. When was the last known working version of clang-format? 17.0.6 (regressed since 18.1.1). https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
tstellar wrote: @owenca OK, can you give me more details: 1) How risky is this fix? 2) When was the last known working version of clang-format? https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
owenca wrote: @tstellar this is a regression fix as labeled by the issue the original pr/commit fixed. :) https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
tstellar wrote: @owenca I'm only going to take regression fixes for 18.1.7. https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
https://github.com/owenca approved this pull request. https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
owenca wrote: @tstellar can we get this one in if we are doing another point release? https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
llvmbot wrote:
@llvm/pr-subscribers-clang-format
Author: None (llvmbot)
Changes
Backport d89f20058b45e3836527e816af7ed7372e1d554d
Requested by: @owenca
---
Full diff: https://github.com/llvm/llvm-project/pull/93494.diff
3 Files Affected:
- (modified) clang/lib/Format/UnwrappedLineParser.cpp (+2-7)
- (modified) clang/unittests/Format/FormatTest.cpp (+8)
- (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+13)
``diff
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp
b/clang/lib/Format/UnwrappedLineParser.cpp
index f70affb732a0d..179d77bf00491 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1185,12 +1185,6 @@ void UnwrappedLineParser::parsePPDefine() {
return;
}
- if (FormatTok->is(tok::identifier) &&
- Tokens->peekNextToken()->is(tok::colon)) {
-nextToken();
-nextToken();
- }
-
// Errors during a preprocessor directive can only affect the layout of the
// preprocessor directive, and thus we ignore them. An alternative approach
// would be to use the same approach we use on the file level (no
@@ -1671,7 +1665,8 @@ void UnwrappedLineParser::parseStructuralElement(
if (!Style.isJavaScript() && !Style.isVerilog() && !Style.isTableGen() &&
Tokens->peekNextToken()->is(tok::colon) && !Line->MustBeDeclaration) {
nextToken();
- Line->Tokens.begin()->Tok->MustBreakBefore = true;
+ if (!Line->InMacroBody || CurrentLines->size() > 1)
+Line->Tokens.begin()->Tok->MustBreakBefore = true;
FormatTok->setFinalizedType(TT_GotoLabelColon);
parseLabel(!Style.IndentGotoLabels);
if (HasLabel)
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 0161a1685eb12..df730838738d1 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -3118,6 +3118,7 @@ TEST_F(FormatTest, FormatsLabels) {
"g();\n"
" }\n"
"}");
+
FormatStyle Style = getLLVMStyle();
Style.IndentGotoLabels = false;
verifyFormat("void f() {\n"
@@ -3157,6 +3158,13 @@ TEST_F(FormatTest, FormatsLabels) {
" }\n"
"}",
Style);
+
+ Style.ColumnLimit = 15;
+ verifyFormat("#define FOO \\\n"
+ "label:\\\n"
+ " break;",
+ Style);
+
// The opening brace may either be on the same unwrapped line as the colon or
// on a separate one. The formatter should recognize both.
Style = getLLVMStyle();
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 44ebad9d5a872..dfa9c22430f36 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2353,15 +2353,28 @@ TEST_F(TokenAnnotatorTest, UnderstandsLabels) {
auto Tokens = annotate("{ x: break; }");
ASSERT_EQ(Tokens.size(), 7u) << Tokens;
EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+
Tokens = annotate("{ case x: break; }");
ASSERT_EQ(Tokens.size(), 8u) << Tokens;
EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+
Tokens = annotate("{ x: { break; } }");
ASSERT_EQ(Tokens.size(), 9u) << Tokens;
EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+
Tokens = annotate("{ case x: { break; } }");
ASSERT_EQ(Tokens.size(), 10u) << Tokens;
EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+
+ Tokens = annotate("#define FOO label:");
+ ASSERT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon);
+
+ Tokens = annotate("#define FOO \\\n"
+"label: \\\n"
+" break;");
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon);
}
TEST_F(TokenAnnotatorTest, UnderstandsNestedBlocks) {
``
https://github.com/llvm/llvm-project/pull/93494
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
https://github.com/llvmbot created
https://github.com/llvm/llvm-project/pull/93494
Backport d89f20058b45e3836527e816af7ed7372e1d554d
Requested by: @owenca
>From ad6e7384693cf38ed89dc02fc6e2e0ac215fbd66 Mon Sep 17 00:00:00 2001
From: Owen Pan
Date: Tue, 21 May 2024 01:35:31 -0700
Subject: [PATCH] [clang-format] Fix a bug in formatting goto labels in macros
(#92494)
Fixes #92300.
(cherry picked from commit d89f20058b45e3836527e816af7ed7372e1d554d)
---
clang/lib/Format/UnwrappedLineParser.cpp | 9 ++---
clang/unittests/Format/FormatTest.cpp | 8
clang/unittests/Format/TokenAnnotatorTest.cpp | 13 +
3 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp
b/clang/lib/Format/UnwrappedLineParser.cpp
index f70affb732a0d..179d77bf00491 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1185,12 +1185,6 @@ void UnwrappedLineParser::parsePPDefine() {
return;
}
- if (FormatTok->is(tok::identifier) &&
- Tokens->peekNextToken()->is(tok::colon)) {
-nextToken();
-nextToken();
- }
-
// Errors during a preprocessor directive can only affect the layout of the
// preprocessor directive, and thus we ignore them. An alternative approach
// would be to use the same approach we use on the file level (no
@@ -1671,7 +1665,8 @@ void UnwrappedLineParser::parseStructuralElement(
if (!Style.isJavaScript() && !Style.isVerilog() && !Style.isTableGen() &&
Tokens->peekNextToken()->is(tok::colon) && !Line->MustBeDeclaration) {
nextToken();
- Line->Tokens.begin()->Tok->MustBreakBefore = true;
+ if (!Line->InMacroBody || CurrentLines->size() > 1)
+Line->Tokens.begin()->Tok->MustBreakBefore = true;
FormatTok->setFinalizedType(TT_GotoLabelColon);
parseLabel(!Style.IndentGotoLabels);
if (HasLabel)
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 0161a1685eb12..df730838738d1 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -3118,6 +3118,7 @@ TEST_F(FormatTest, FormatsLabels) {
"g();\n"
" }\n"
"}");
+
FormatStyle Style = getLLVMStyle();
Style.IndentGotoLabels = false;
verifyFormat("void f() {\n"
@@ -3157,6 +3158,13 @@ TEST_F(FormatTest, FormatsLabels) {
" }\n"
"}",
Style);
+
+ Style.ColumnLimit = 15;
+ verifyFormat("#define FOO \\\n"
+ "label:\\\n"
+ " break;",
+ Style);
+
// The opening brace may either be on the same unwrapped line as the colon or
// on a separate one. The formatter should recognize both.
Style = getLLVMStyle();
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 44ebad9d5a872..dfa9c22430f36 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2353,15 +2353,28 @@ TEST_F(TokenAnnotatorTest, UnderstandsLabels) {
auto Tokens = annotate("{ x: break; }");
ASSERT_EQ(Tokens.size(), 7u) << Tokens;
EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+
Tokens = annotate("{ case x: break; }");
ASSERT_EQ(Tokens.size(), 8u) << Tokens;
EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+
Tokens = annotate("{ x: { break; } }");
ASSERT_EQ(Tokens.size(), 9u) << Tokens;
EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+
Tokens = annotate("{ case x: { break; } }");
ASSERT_EQ(Tokens.size(), 10u) << Tokens;
EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+
+ Tokens = annotate("#define FOO label:");
+ ASSERT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon);
+
+ Tokens = annotate("#define FOO \\\n"
+"label: \\\n"
+" break;");
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon);
}
TEST_F(TokenAnnotatorTest, UnderstandsNestedBlocks) {
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
llvmbot wrote: @mydeveloperday What do you think about merging this PR to the release branch? https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
https://github.com/llvmbot milestoned https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
