Skip to content

[clang-format][NFC] Clean up AlignConsecutiveStyle #111285

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

Merged
merged 3 commits into from
Oct 8, 2024

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Oct 6, 2024

  • Add a CHECK_PARSE for AcrossComments.
  • Add a CHECK_PARSE_NESTED_BOOL for AlignFunctionPointers.
  • Remove redundant statements.
  • Clean up documentation.

@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

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

4 Files Affected:

  • (modified) clang/lib/Format/Format.cpp (+2-16)
  • (modified) clang/unittests/Format/ConfigParseTest.cpp (+10-12)
  • (modified) clang/unittests/Format/FormatTest.cpp (+1-4)
  • (modified) clang/unittests/Format/FormatTestVerilog.cpp (+1)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 5350c66ea5132b..d691d198f8025d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -44,11 +44,7 @@ struct ScalarEnumerationTraits<FormatStyle::BreakBeforeNoexceptSpecifierStyle> {
 
 template <> struct MappingTraits<FormatStyle::AlignConsecutiveStyle> {
   static void enumInput(IO &IO, FormatStyle::AlignConsecutiveStyle &Value) {
-    IO.enumCase(Value, "None",
-                FormatStyle::AlignConsecutiveStyle(
-                    {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
-                     /*AcrossComments=*/false, /*AlignCompound=*/false,
-                     /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+    IO.enumCase(Value, "None", FormatStyle::AlignConsecutiveStyle({}));
     IO.enumCase(Value, "Consecutive",
                 FormatStyle::AlignConsecutiveStyle(
                     {/*Enabled=*/true, /*AcrossEmptyLines=*/false,
@@ -76,11 +72,7 @@ template <> struct MappingTraits<FormatStyle::AlignConsecutiveStyle> {
                     {/*Enabled=*/true, /*AcrossEmptyLines=*/false,
                      /*AcrossComments=*/false, /*AlignCompound=*/false,
                      /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
-    IO.enumCase(Value, "false",
-                FormatStyle::AlignConsecutiveStyle(
-                    {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
-                     /*AcrossComments=*/false, /*AlignCompound=*/false,
-                     /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+    IO.enumCase(Value, "false", FormatStyle::AlignConsecutiveStyle({}));
   }
 
   static void mapping(IO &IO, FormatStyle::AlignConsecutiveStyle &Value) {
@@ -1441,12 +1433,6 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
   LLVMStyle.AlignArrayOfStructures = FormatStyle::AIAS_None;
   LLVMStyle.AlignConsecutiveAssignments = {};
-  LLVMStyle.AlignConsecutiveAssignments.AcrossComments = false;
-  LLVMStyle.AlignConsecutiveAssignments.AcrossEmptyLines = false;
-  LLVMStyle.AlignConsecutiveAssignments.AlignCompound = false;
-  LLVMStyle.AlignConsecutiveAssignments.AlignFunctionPointers = false;
-  LLVMStyle.AlignConsecutiveAssignments.Enabled = false;
-  LLVMStyle.AlignConsecutiveAssignments.PadOperators = true;
   LLVMStyle.AlignConsecutiveBitFields = {};
   LLVMStyle.AlignConsecutiveDeclarations = {};
   LLVMStyle.AlignConsecutiveMacros = {};
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index b8bdfaaa74e10e..4d723981d73926 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -300,12 +300,8 @@ TEST(ConfigParseTest, ParsesConfiguration) {
 #define CHECK_ALIGN_CONSECUTIVE(FIELD)                                         \
   do {                                                                         \
     Style.FIELD.Enabled = true;                                                \
-    CHECK_PARSE(                                                               \
-        #FIELD ": None", FIELD,                                                \
-        FormatStyle::AlignConsecutiveStyle(                                    \
-            {/*Enabled=*/false, /*AcrossEmptyLines=*/false,                    \
-             /*AcrossComments=*/false, /*AlignCompound=*/false,                \
-             /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));        \
+    CHECK_PARSE(#FIELD ": None", FIELD,                                        \
+                FormatStyle::AlignConsecutiveStyle({}));                       \
     CHECK_PARSE(                                                               \
         #FIELD ": Consecutive", FIELD,                                         \
         FormatStyle::AlignConsecutiveStyle(                                    \
@@ -319,18 +315,20 @@ TEST(ConfigParseTest, ParsesConfiguration) {
              /*AcrossComments=*/false, /*AlignCompound=*/false,                \
              /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));        \
     CHECK_PARSE(                                                               \
-        #FIELD ": AcrossEmptyLinesAndComments", FIELD,                         \
+        #FIELD ": AcrossComments", FIELD,                                      \
         FormatStyle::AlignConsecutiveStyle(                                    \
-            {/*Enabled=*/true, /*AcrossEmptyLines=*/true,                      \
+            {/*Enabled=*/true, /*AcrossEmptyLines=*/false,                     \
              /*AcrossComments=*/true, /*AlignCompound=*/false,                 \
              /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));        \
-    /* For backwards compability, false / true should still parse */           \
     CHECK_PARSE(                                                               \
-        #FIELD ": false", FIELD,                                               \
+        #FIELD ": AcrossEmptyLinesAndComments", FIELD,                         \
         FormatStyle::AlignConsecutiveStyle(                                    \
-            {/*Enabled=*/false, /*AcrossEmptyLines=*/false,                    \
-             /*AcrossComments=*/false, /*AlignCompound=*/false,                \
+            {/*Enabled=*/true, /*AcrossEmptyLines=*/true,                      \
+             /*AcrossComments=*/true, /*AlignCompound=*/false,                 \
              /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));        \
+    /* For backwards compability, false / true should still parse */           \
+    CHECK_PARSE(#FIELD ": false", FIELD,                                       \
+                FormatStyle::AlignConsecutiveStyle({}));                       \
     CHECK_PARSE(                                                               \
         #FIELD ": true", FIELD,                                                \
         FormatStyle::AlignConsecutiveStyle(                                    \
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 5d386c1bbdbcd9..72350a50c4a80b 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -20261,10 +20261,7 @@ TEST_F(FormatTest, AlignWithLineBreaks) {
   auto Style = getLLVMStyleWithColumns(120);
 
   EXPECT_EQ(Style.AlignConsecutiveAssignments,
-            FormatStyle::AlignConsecutiveStyle(
-                {/*Enabled=*/false, /*AcrossEmptyLines=*/false,
-                 /*AcrossComments=*/false, /*AlignCompound=*/false,
-                 /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+            FormatStyle::AlignConsecutiveStyle({}));
   EXPECT_EQ(Style.AlignConsecutiveDeclarations,
             FormatStyle::AlignConsecutiveStyle({}));
   verifyFormat("void foo() {\n"
diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp
index fbaf289fbc4d6d..f719fcebb2f55a 100644
--- a/clang/unittests/Format/FormatTestVerilog.cpp
+++ b/clang/unittests/Format/FormatTestVerilog.cpp
@@ -56,6 +56,7 @@ TEST_F(FormatTestVerilog, Align) {
                "sfdbddfbdfbb <= x;",
                Style);
   Style.AlignConsecutiveAssignments.AlignCompound = true;
+  Style.AlignConsecutiveAssignments.PadOperators = true;
   verifyFormat("x            <= x;\n"
                "sfdbddfbdfbb <= x;",
                Style);

@owenca owenca force-pushed the AlignConsecutiveStyle branch from 7745a5d to b1bcd59 Compare October 6, 2024 19:31
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Oct 6, 2024
@owenca owenca removed the clang Clang issues not falling into any other category label Oct 6, 2024
bradh352 added a commit to bradh352/llvm-project that referenced this pull request Oct 6, 2024
* Changes requested to some existing code that is also part of
  PR llvm#111285.
* Removal of redundant test case
* Add of boolean check for AlignFunctionDeclarations
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Oct 7, 2024
@owenca owenca removed the clang Clang issues not falling into any other category label Oct 8, 2024
@owenca owenca merged commit 6568827 into llvm:main Oct 8, 2024
10 checks passed
@owenca owenca deleted the AlignConsecutiveStyle branch October 8, 2024 02:12
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.

3 participants