[clang] 82a90ca - [clang-format] Correctly format goto labels followed by blocks
via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 30 15:35:26 PDT 2023
Author: sstwcw
Date: 2023-04-30T22:25:48Z
New Revision: 82a90caa88fdc632ea2c8495ea3237065272a3a0
URL: https://github.com/llvm/llvm-project/commit/82a90caa88fdc632ea2c8495ea3237065272a3a0
DIFF: https://github.com/llvm/llvm-project/commit/82a90caa88fdc632ea2c8495ea3237065272a3a0.diff
LOG: [clang-format] Correctly format goto labels followed by blocks
There doesn't seem to be an issue on GitHub. But previously, a space
would be inserted before the goto colon in the code below.
switch (x) {
case 0:
goto_0: {
action();
break;
}
}
Previously, the colon following a goto label would be annotated as
`TT_InheritanceColon`. A goto label followed by an opening brace
wasn't recognized. It is easy to add another line to have
`spaceRequiredBefore` function recognize the case, but I believed it
is more proper to avoid doing the same thing in `UnwrappedLineParser`
and `TokenAnnotator`. So now the label colons would be labeled in
`UnwrappedLineParser`, and `spaceRequiredBefore` would rely on that.
Previously we had the type `TT_GotoLabelColon` intended for both goto
labels and case labels. But since handling of goto labels and case
labels differ somewhat, I split it into separate types for goto and
case labels.
This patch doesn't change the behavior for case labels. I added the
lines annotating case labels because they would previously be
mistakenly annotated as `TT_InheritanceColon` just like goto labels.
And since I added the annotations, the checks for the `case` and
`default` keywords in `spaceRequiredBefore` are not necessary anymore.
Reviewed By: MyDeveloperDay
Differential Revision: https://reviews.llvm.org/D148484
Added:
Modified:
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 834ebf6f8b204..4672fac9d02ec 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -37,6 +37,8 @@ namespace format {
TYPE(BitFieldColon) \
TYPE(BlockComment) \
TYPE(BracedListLBrace) \
+ /* The colon at the end of a case label. */ \
+ TYPE(CaseLabelColon) \
TYPE(CastRParen) \
TYPE(ClassLBrace) \
TYPE(CompoundRequirementLBrace) \
@@ -73,8 +75,7 @@ namespace format {
TYPE(FunctionTypeLParen) \
/* The colons as part of a C11 _Generic selection */ \
TYPE(GenericSelectionColon) \
- /* The colon at the end of a goto label or a case label. Currently only used \
- * for Verilog. */ \
+ /* The colon at the end of a goto label. */ \
TYPE(GotoLabelColon) \
TYPE(IfMacro) \
TYPE(ImplicitStringLiteral) \
@@ -1803,7 +1804,7 @@ struct AdditionalKeywords {
bool isVerilogEndOfLabel(const FormatToken &Tok) const {
const FormatToken *Next = Tok.getNextNonComment();
// In Verilog the colon in a default label is optional.
- return Tok.is(TT_GotoLabelColon) ||
+ return Tok.is(TT_CaseLabelColon) ||
(Tok.is(tok::kw_default) &&
!(Next && Next->isOneOf(tok::colon, tok::semi, kw_clocking, kw_iff,
kw_input, kw_output, kw_sequence)));
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 06593355d4d5a..0bf7e9ad2edc3 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -970,6 +970,10 @@ class AnnotatingParser {
case tok::colon:
if (!Tok->Previous)
return false;
+ // Goto labels and case labels are already identified in
+ // UnwrappedLineParser.
+ if (Tok->isTypeFinalized())
+ break;
// Colons from ?: are handled in parseConditional().
if (Style.isJavaScript()) {
if (Contexts.back().ColonIsForRangeExpr || // colon in for loop
@@ -1008,7 +1012,7 @@ class AnnotatingParser {
// In Verilog a case label doesn't have the case keyword. We
// assume a colon following an expression is a case label.
// Colons from ?: are annotated in parseConditional().
- Tok->setType(TT_GotoLabelColon);
+ Tok->setType(TT_CaseLabelColon);
if (Line.Level > 1 || (!Line.InPPDirective && Line.Level > 0))
--Line.Level;
}
@@ -4541,13 +4545,12 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
Style.BitFieldColonSpacing == FormatStyle::BFCS_After;
}
if (Right.is(tok::colon)) {
- if (Right.is(TT_GotoLabelColon) ||
- (!Style.isVerilog() &&
- Line.First->isOneOf(tok::kw_default, tok::kw_case))) {
+ if (Right.is(TT_CaseLabelColon))
return Style.SpaceBeforeCaseColon;
- }
- const FormatToken *Next = Right.getNextNonComment();
- if (!Next || Next->is(tok::semi))
+ if (Right.is(TT_GotoLabelColon))
+ return false;
+ // `private:` and `public:`.
+ if (!Right.getNextNonComment())
return false;
if (Right.is(TT_ObjCMethodExpr))
return false;
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 5c6816d6df30f..2e5bdf96deb88 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1493,6 +1493,7 @@ void UnwrappedLineParser::parseStructuralElement(
}
nextToken();
if (FormatTok->is(tok::colon)) {
+ FormatTok->setFinalizedType(TT_CaseLabelColon);
parseLabel();
return;
}
@@ -1925,6 +1926,7 @@ void UnwrappedLineParser::parseStructuralElement(
if (!Style.isVerilog() && FormatTok->is(tok::colon) &&
!Line->MustBeDeclaration) {
Line->Tokens.begin()->Tok->MustBreakBefore = true;
+ FormatTok->setFinalizedType(TT_GotoLabelColon);
parseLabel(!Style.IndentGotoLabels);
if (HasLabel)
*HasLabel = true;
@@ -3113,7 +3115,11 @@ void UnwrappedLineParser::parseCaseLabel() {
// FIXME: fix handling of complex expressions here.
do {
nextToken();
- } while (!eof() && !FormatTok->is(tok::colon));
+ if (FormatTok->is(tok::colon)) {
+ FormatTok->setFinalizedType(TT_CaseLabelColon);
+ break;
+ }
+ } while (!eof());
parseLabel();
}
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 5db947abff8c8..099a5cb913643 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2998,6 +2998,17 @@ TEST_F(FormatTest, FormatsLabels) {
"test_label:;\n"
" int i = 0;\n"
"}");
+ verifyFormat("{\n"
+ " some_code();\n"
+ "test_label: { some_other_code(); }\n"
+ "}");
+ verifyFormat("{\n"
+ " some_code();\n"
+ "test_label: {\n"
+ " some_other_code();\n"
+ " some_other_code();\n"
+ "}\n"
+ "}");
FormatStyle Style = getLLVMStyle();
Style.IndentGotoLabels = false;
verifyFormat("void f() {\n"
@@ -3022,6 +3033,23 @@ TEST_F(FormatTest, FormatsLabels) {
"test_label:;\n"
" int i = 0;\n"
"}");
+ verifyFormat("{\n"
+ " some_code();\n"
+ "test_label: { some_other_code(); }\n"
+ "}",
+ 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();
+ Style.BreakBeforeBraces = FormatStyle::BraceBreakingStyle::BS_Allman;
+ verifyFormat("{\n"
+ " some_code();\n"
+ "test_label:\n"
+ "{\n"
+ " some_other_code();\n"
+ "}\n"
+ "}",
+ Style);
}
TEST_F(FormatTest, MultiLineControlStatements) {
@@ -16962,6 +16990,19 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeColon) {
"}\n"
"}",
CaseStyle);
+ // Goto labels should not be affected.
+ verifyFormat("switch (x) {\n"
+ "goto_label:\n"
+ "default :\n"
+ "}",
+ CaseStyle);
+ verifyFormat("switch (x) {\n"
+ "goto_label: { break; }\n"
+ "default : {\n"
+ " break;\n"
+ "}\n"
+ "}",
+ CaseStyle);
FormatStyle NoSpaceStyle = getLLVMStyle();
EXPECT_EQ(NoSpaceStyle.SpaceBeforeCaseColon, false);
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index cd39661f7f8de..62a83596600e3 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1621,7 +1621,7 @@ TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
" x;\n"
"endcase\n");
ASSERT_EQ(Tokens.size(), 10u) << Tokens;
- EXPECT_TOKEN(Tokens[5], tok::colon, TT_GotoLabelColon);
+ EXPECT_TOKEN(Tokens[5], tok::colon, TT_CaseLabelColon);
Tokens = Annotate("case (x)\n"
" x ? x : x:\n"
" x;\n"
@@ -1629,7 +1629,7 @@ TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
ASSERT_EQ(Tokens.size(), 14u) << Tokens;
EXPECT_TOKEN(Tokens[5], tok::question, TT_ConditionalExpr);
EXPECT_TOKEN(Tokens[7], tok::colon, TT_ConditionalExpr);
- EXPECT_TOKEN(Tokens[9], tok::colon, TT_GotoLabelColon);
+ EXPECT_TOKEN(Tokens[9], tok::colon, TT_CaseLabelColon);
// Non-blocking assignments.
Tokens = Annotate("a <= b;");
ASSERT_EQ(Tokens.size(), 5u) << Tokens;
@@ -1752,6 +1752,21 @@ TEST_F(TokenAnnotatorTest, CSharpNullableTypes) {
EXPECT_TOKEN(Tokens[1], tok::question, TT_ConditionalExpr);
}
+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);
+}
+
} // namespace
} // namespace format
} // namespace clang
More information about the cfe-commits
mailing list