https://github.com/tomekpaszek updated https://github.com/llvm/llvm-project/pull/70338
>From 25ca978ef9caf372997f2ebf227fb2b2ca443aa0 Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Sat, 11 Nov 2023 19:38:00 +0100 Subject: [PATCH 01/15] Added an option to ignore macro definitions. --- clang/include/clang/Format/Format.h | 5 +++++ clang/lib/Format/Format.cpp | 2 ++ clang/lib/Format/UnwrappedLineFormatter.cpp | 2 ++ clang/unittests/Format/ConfigParseTest.cpp | 3 ++- clang/unittests/Format/FormatTest.cpp | 16 ++++++++++++++++ 5 files changed, 27 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 37cb3b5cc06732e..fce68d70f4dd690 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -2512,6 +2512,10 @@ struct FormatStyle { /// <https://github.com/capnproto/capnproto/blob/master/kjdoc/tour.md#maybes>`_ /// \version 13 std::vector<std::string> IfMacros; + + /// Ignore formatting in preprocessor definitions. + /// \version 18 + bool IgnorePPDefinitions; /// Specify whether access modifiers should have their own indentation level. /// @@ -4790,6 +4794,7 @@ struct FormatStyle { R.IncludeStyle.IncludeIsMainRegex && IncludeStyle.IncludeIsMainSourceRegex == R.IncludeStyle.IncludeIsMainSourceRegex && + IgnorePPDefinitions == R.IgnorePPDefinitions && IndentAccessModifiers == R.IndentAccessModifiers && IndentCaseBlocks == R.IndentCaseBlocks && IndentCaseLabels == R.IndentCaseLabels && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index db0cb8a31084952..a6e451595527e73 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1002,6 +1002,7 @@ template <> struct MappingTraits<FormatStyle> { IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments); IO.mapOptional("ForEachMacros", Style.ForEachMacros); IO.mapOptional("IfMacros", Style.IfMacros); + IO.mapOptional("IgnorePPDefinitions", Style.IgnorePPDefinitions); IO.mapOptional("IncludeBlocks", Style.IncludeStyle.IncludeBlocks); IO.mapOptional("IncludeCategories", Style.IncludeStyle.IncludeCategories); IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex); @@ -1507,6 +1508,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.ForEachMacros.push_back("Q_FOREACH"); LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH"); LLVMStyle.IfMacros.push_back("KJ_IF_MAYBE"); + LLVMStyle.IgnorePPDefinitions = false; LLVMStyle.IncludeStyle.IncludeCategories = { {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0, false}, {"^(<|\"(gtest|gmock|isl|json)/)", 3, 0, false}, diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 280485d9a90d1bf..bbf6383ff7673f6 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1355,6 +1355,8 @@ unsigned UnwrappedLineFormatter::format( bool FixIndentation = (FixBadIndentation || ContinueFormatting) && Indent != TheLine.First->OriginalColumn; bool ShouldFormat = TheLine.Affected || FixIndentation; + if (Style.IgnorePPDefinitions && TheLine.Type == LT_PreprocessorDirective) + ShouldFormat = false; // We cannot format this line; if the reason is that the line had a // parsing error, remember that. if (ShouldFormat && TheLine.Type == LT_Invalid && Status) { diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 0c9f68f303d8653..fa4e1469e8cc136 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -167,6 +167,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(DerivePointerAlignment); CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding"); CHECK_PARSE_BOOL(DisableFormat); + CHECK_PARSE_BOOL(IgnorePPDefinitions); CHECK_PARSE_BOOL(IndentAccessModifiers); CHECK_PARSE_BOOL(IndentCaseLabels); CHECK_PARSE_BOOL(IndentCaseBlocks); @@ -199,7 +200,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon); CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets); CHECK_PARSE_BOOL(VerilogBreakBetweenInstancePorts); - + CHECK_PARSE_NESTED_BOOL(AlignConsecutiveShortCaseStatements, Enabled); CHECK_PARSE_NESTED_BOOL(AlignConsecutiveShortCaseStatements, AcrossEmptyLines); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 420afe5992f2a0b..e04ffc146404f17 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24219,6 +24219,22 @@ TEST_F(FormatTest, WhitespaceSensitiveMacros) { verifyNoChange("FOO(String-ized&Messy+But: :Still=Intentional);", Style); } +TEST_F(FormatTest, IgnorePPDefinitions) { + FormatStyle Style = getLLVMStyle(); + Style.IgnorePPDefinitions = true; + + verifyNoChange("#define A", Style); + verifyNoChange("#define A b", Style); + verifyNoChange("#define A ( args )", Style); + verifyNoChange("#define A ( args ) = func ( args )", Style); + verifyNoChange("#define TEXT Text . With periods.", Style); + verifyNoChange("#define TEXT \\\nLine number one . \\\nNumber two .", + Style); + + // TODO + // verifyNoChange("/* comment */ #define A ( args )", Style); +} + TEST_F(FormatTest, VeryLongNamespaceCommentSplit) { // These tests are not in NamespaceEndCommentsFixerTest because that doesn't // test its interaction with line wrapping >From e19a3a7ac17a8afd57a3776aa52699e8d4e2c01f Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Fri, 27 Oct 2023 13:57:50 +0200 Subject: [PATCH 02/15] Added edge cases to the test and code that fixes handling them --- clang/lib/Format/UnwrappedLineParser.cpp | 8 ++++++++ clang/unittests/Format/FormatTest.cpp | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 7b4ec25a8938fc0..bd4e15feb0d289e 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -1138,6 +1138,14 @@ void UnwrappedLineParser::parsePPDefine() { return; } + if (Style.IgnorePPDefinitions) { + do { + nextToken(); + } while (!eof()); + addUnwrappedLine(); + return; + } + if (IncludeGuard == IG_IfNdefed && IncludeGuardToken->TokenText == FormatTok->TokenText) { IncludeGuard = IG_Defined; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index e04ffc146404f17..9338739334d3d73 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24227,9 +24227,10 @@ TEST_F(FormatTest, IgnorePPDefinitions) { verifyNoChange("#define A b", Style); verifyNoChange("#define A ( args )", Style); verifyNoChange("#define A ( args ) = func ( args )", Style); - verifyNoChange("#define TEXT Text . With periods.", Style); verifyNoChange("#define TEXT \\\nLine number one . \\\nNumber two .", Style); + verifyNoChange("#define A x:", Style); + verifyNoChange("#define A a. b", Style); // TODO // verifyNoChange("/* comment */ #define A ( args )", Style); >From 4ec984677c5779eafc34e456b894b032ef10dc50 Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Sat, 11 Nov 2023 19:28:25 +0100 Subject: [PATCH 03/15] Added more tests --- clang/unittests/Format/ConfigParseTest.cpp | 2 +- clang/unittests/Format/FormatTest.cpp | 40 +++++++++++++++++++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index fa4e1469e8cc136..7a93ec71388eb71 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -200,7 +200,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon); CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets); CHECK_PARSE_BOOL(VerilogBreakBetweenInstancePorts); - + CHECK_PARSE_NESTED_BOOL(AlignConsecutiveShortCaseStatements, Enabled); CHECK_PARSE_NESTED_BOOL(AlignConsecutiveShortCaseStatements, AcrossEmptyLines); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 9338739334d3d73..91243d5caeb5b4d 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24227,13 +24227,43 @@ TEST_F(FormatTest, IgnorePPDefinitions) { verifyNoChange("#define A b", Style); verifyNoChange("#define A ( args )", Style); verifyNoChange("#define A ( args ) = func ( args )", Style); - verifyNoChange("#define TEXT \\\nLine number one . \\\nNumber two .", - Style); + verifyNoChange("#define A x:", Style); verifyNoChange("#define A a. b", Style); - - // TODO - // verifyNoChange("/* comment */ #define A ( args )", Style); + + // Surrounded with formatted code + verifyFormat("int a;\n" + "#define A a\n" + "int a;", + "int a ;\n" + "#define A a\n" + "int a ;", + Style); + + // Columns are not broken when a limit is set + Style.ColumnLimit = 10; + verifyNoChange("#define A a a a a", Style); + Style.ColumnLimit = 0; + + // Multiline definition + verifyNoChange("#define A \\\n" + "Line one with spaces . \\\n" + " Line two.", + Style); + verifyNoChange("#define A \\\n" + "a a \\\n" + "a \\\na", + Style); + Style.AlignEscapedNewlines = FormatStyle::ENAS_Left; + verifyNoChange("#define A \\\n" + "a a \\\n" + "a \\\na", + Style); + Style.AlignEscapedNewlines = FormatStyle::ENAS_Right; + verifyNoChange("#define A \\\n" + "a a \\\n" + "a \\\na", + Style); } TEST_F(FormatTest, VeryLongNamespaceCommentSplit) { >From 1640ab6481abeb306bcc1b62dd21a72b4a77046c Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Mon, 13 Nov 2023 10:58:12 +0100 Subject: [PATCH 04/15] Updated documentation files --- clang/docs/ClangFormatStyleOptions.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 37d76d5c2dd58c0..e1d6d5756822e9c 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -3232,6 +3232,11 @@ the configuration (without a prefix: ``Auto``). For example: `KJ_IF_MAYBE <https://github.com/capnproto/capnproto/blob/master/kjdoc/tour.md#maybes>`_ +.. _IgnorePPDefinitions: + +**IgnorePPDefinitions** (``Boolean``) :versionbadge:`clang-format 18` :ref:`¶ <IgnorePPDefinitions>` + Ignore formatting in preprocessor definitions. + .. _IncludeBlocks: **IncludeBlocks** (``IncludeBlocksStyle``) :versionbadge:`clang-format 6` :ref:`¶ <IncludeBlocks>` >From 4d2be7958751264cfe59ababb2949e89f98cbafb Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Mon, 13 Nov 2023 11:06:17 +0100 Subject: [PATCH 05/15] Reformatted changes --- clang/include/clang/Format/Format.h | 2 +- clang/unittests/Format/FormatTest.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index fce68d70f4dd690..18ab712af4cff06 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -2512,7 +2512,7 @@ struct FormatStyle { /// <https://github.com/capnproto/capnproto/blob/master/kjdoc/tour.md#maybes>`_ /// \version 13 std::vector<std::string> IfMacros; - + /// Ignore formatting in preprocessor definitions. /// \version 18 bool IgnorePPDefinitions; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 91243d5caeb5b4d..f0cc20fc06a597e 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24230,7 +24230,7 @@ TEST_F(FormatTest, IgnorePPDefinitions) { verifyNoChange("#define A x:", Style); verifyNoChange("#define A a. b", Style); - + // Surrounded with formatted code verifyFormat("int a;\n" "#define A a\n" @@ -24239,17 +24239,17 @@ TEST_F(FormatTest, IgnorePPDefinitions) { "#define A a\n" "int a ;", Style); - + // Columns are not broken when a limit is set Style.ColumnLimit = 10; verifyNoChange("#define A a a a a", Style); Style.ColumnLimit = 0; - + // Multiline definition verifyNoChange("#define A \\\n" "Line one with spaces . \\\n" " Line two.", - Style); + Style); verifyNoChange("#define A \\\n" "a a \\\n" "a \\\na", @@ -24262,7 +24262,7 @@ TEST_F(FormatTest, IgnorePPDefinitions) { Style.AlignEscapedNewlines = FormatStyle::ENAS_Right; verifyNoChange("#define A \\\n" "a a \\\n" - "a \\\na", + "a \\\na", Style); } >From dc12a24229a9feb90f5fcd87e5efa9136a4385ac Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Mon, 13 Nov 2023 16:43:10 +0100 Subject: [PATCH 06/15] Fixed code order --- clang/include/clang/Format/Format.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 18ab712af4cff06..c48c8c94559ca77 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -4788,13 +4788,13 @@ struct FormatStyle { R.ExperimentalAutoDetectBinPacking && FixNamespaceComments == R.FixNamespaceComments && ForEachMacros == R.ForEachMacros && + IgnorePPDefinitions == R.IgnorePPDefinitions && IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks && IncludeStyle.IncludeCategories == R.IncludeStyle.IncludeCategories && IncludeStyle.IncludeIsMainRegex == R.IncludeStyle.IncludeIsMainRegex && IncludeStyle.IncludeIsMainSourceRegex == R.IncludeStyle.IncludeIsMainSourceRegex && - IgnorePPDefinitions == R.IgnorePPDefinitions && IndentAccessModifiers == R.IndentAccessModifiers && IndentCaseBlocks == R.IndentCaseBlocks && IndentCaseLabels == R.IndentCaseLabels && >From 9debd8b86b928560b519d644a59fa0a7fa8bb230 Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Mon, 13 Nov 2023 16:44:50 +0100 Subject: [PATCH 07/15] Added more tests to cover PP indentation and fixed readability in other tests --- clang/unittests/Format/FormatTest.cpp | 44 +++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index f0cc20fc06a597e..48b069598c34b82 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24231,7 +24231,7 @@ TEST_F(FormatTest, IgnorePPDefinitions) { verifyNoChange("#define A x:", Style); verifyNoChange("#define A a. b", Style); - // Surrounded with formatted code + // Surrounded with formatted code. verifyFormat("int a;\n" "#define A a\n" "int a;", @@ -24240,29 +24240,61 @@ TEST_F(FormatTest, IgnorePPDefinitions) { "int a ;", Style); - // Columns are not broken when a limit is set + // Columns are not broken when a limit is set. Style.ColumnLimit = 10; verifyNoChange("#define A a a a a", Style); Style.ColumnLimit = 0; - // Multiline definition + // Multiline definition. verifyNoChange("#define A \\\n" "Line one with spaces . \\\n" " Line two.", Style); verifyNoChange("#define A \\\n" "a a \\\n" - "a \\\na", + "a \\\n" + "a", Style); Style.AlignEscapedNewlines = FormatStyle::ENAS_Left; verifyNoChange("#define A \\\n" "a a \\\n" - "a \\\na", + "a \\\n" + "a", Style); Style.AlignEscapedNewlines = FormatStyle::ENAS_Right; verifyNoChange("#define A \\\n" "a a \\\n" - "a \\\na", + "a \\\n" + "a", + Style); + + // Adjust indendations but don't change the definition. + Style.IndentPPDirectives = FormatStyle::PPDIS_None; + verifyNoChange("#if A\n" + "#define A a\n" + "#endif", + Style); + verifyNoChange("#if A\n" + "#define A a\\\n" + " a a\n" + "#endif", + Style); + verifyFormat("#if A\n" + "#define A a\n" + "#endif", + "#if A\n" + " #define A a\n" + "#endif", + Style); + Style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash; + verifyNoChange("#if A\n" + "# define A a\n" + "#endif", + Style); + Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash; + verifyNoChange("#if A\n" + " #define A a\n" + "#endif", Style); } >From 1faffeb0134ef2d6d0fcf0bbf493ece1cc68d22d Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Tue, 14 Nov 2023 11:10:04 +0100 Subject: [PATCH 08/15] Fixed the option affecting other PP directives --- clang/lib/Format/UnwrappedLineFormatter.cpp | 5 +++- clang/unittests/Format/FormatTest.cpp | 28 +++++++++++++++------ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index bbf6383ff7673f6..7bfb25089ac06ee 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1355,8 +1355,11 @@ unsigned UnwrappedLineFormatter::format( bool FixIndentation = (FixBadIndentation || ContinueFormatting) && Indent != TheLine.First->OriginalColumn; bool ShouldFormat = TheLine.Affected || FixIndentation; - if (Style.IgnorePPDefinitions && TheLine.Type == LT_PreprocessorDirective) + if (Style.IgnorePPDefinitions && TheLine.Type == LT_PreprocessorDirective && + TheLine.getFirstNonComment()->Next->is(tok::pp_define)) { ShouldFormat = false; + } + // We cannot format this line; if the reason is that the line had a // parsing error, remember that. if (ShouldFormat && TheLine.Type == LT_Invalid && Status) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 48b069598c34b82..f3f17d4bd3ccf7e 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24272,10 +24272,11 @@ TEST_F(FormatTest, IgnorePPDefinitions) { Style.IndentPPDirectives = FormatStyle::PPDIS_None; verifyNoChange("#if A\n" "#define A a\n" - "#endif", + "#endif\n", Style); - verifyNoChange("#if A\n" - "#define A a\\\n" + verifyNoChange("#define UNITY 1\n" + "#if A\n" + "# define A a\\\n" " a a\n" "#endif", Style); @@ -24287,15 +24288,28 @@ TEST_F(FormatTest, IgnorePPDefinitions) { "#endif", Style); Style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash; - verifyNoChange("#if A\n" - "# define A a\n" - "#endif", - Style); + verifyFormat("#if A\n" + "# define A a\n" + "#endif", + "#if A\n" + " # define A a\n" + "#endif", + Style); Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash; verifyNoChange("#if A\n" " #define A a\n" "#endif", Style); + + Style.IndentPPDirectives = FormatStyle::PPDIS_None; + // IgnorePPDefinitions should not affect other PP directives + verifyFormat("#if !defined(A)\n" + "# define A a\n" + "#endif", + "#if ! defined ( A )\n" + " # define A a\n" + "#endif", + Style); } TEST_F(FormatTest, VeryLongNamespaceCommentSplit) { >From afdcba3084da008cc206ce67b64c60fffe77bf6b Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Tue, 14 Nov 2023 12:52:54 +0100 Subject: [PATCH 09/15] Added test cases for trailing comments --- clang/unittests/Format/FormatTest.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index f3f17d4bd3ccf7e..4de4701cd352e80 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24243,6 +24243,20 @@ TEST_F(FormatTest, IgnorePPDefinitions) { // Columns are not broken when a limit is set. Style.ColumnLimit = 10; verifyNoChange("#define A a a a a", Style); + + Style.ColumnLimit = 15; + verifyNoChange("#define A //a very long comment\n", Style); + // in the following examples, since second line will not be formtted, it won't + // take into considertaion the alignment from the first line. The third line + // will follow the second line's alignment. + verifyFormat("int aaaaaa; // a\n" + "#define A // a\n" + "int a; // a\n", + "int aaaaaa; // a\n" + "#define A // a\n" + "int a; // a\n", + Style); + Style.ColumnLimit = 0; // Multiline definition. >From 3d2a56bfa8e2eb3f5a3b8ee05135cec664dc76ee Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Sat, 18 Nov 2023 13:47:53 +0100 Subject: [PATCH 10/15] Refactored condition to ignore PP directives other than #define --- clang/lib/Format/UnwrappedLineFormatter.cpp | 12 +++++++++--- clang/unittests/Format/FormatTest.cpp | 18 ++++++++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 7bfb25089ac06ee..0a49e1cc282372e 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1308,6 +1308,13 @@ class OptimizingLineFormatter : public LineFormatter { } // anonymous namespace +static bool lineContainsPPDefinition(const AnnotatedLine &Line) { + auto *Tok = Line.getFirstNonComment(); + if (!Tok || !Tok->is(tok::hash) || !Tok->Next) + return false; + return Tok->Next->is(tok::pp_define); +} + unsigned UnwrappedLineFormatter::format( const SmallVectorImpl<AnnotatedLine *> &Lines, bool DryRun, int AdditionalIndent, bool FixBadIndentation, unsigned FirstStartColumn, @@ -1355,10 +1362,9 @@ unsigned UnwrappedLineFormatter::format( bool FixIndentation = (FixBadIndentation || ContinueFormatting) && Indent != TheLine.First->OriginalColumn; bool ShouldFormat = TheLine.Affected || FixIndentation; - if (Style.IgnorePPDefinitions && TheLine.Type == LT_PreprocessorDirective && - TheLine.getFirstNonComment()->Next->is(tok::pp_define)) { + + if (Style.IgnorePPDefinitions && lineContainsPPDefinition(TheLine)) ShouldFormat = false; - } // We cannot format this line; if the reason is that the line had a // parsing error, remember that. diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 4de4701cd352e80..d6eb5378b813cc5 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24245,16 +24245,16 @@ TEST_F(FormatTest, IgnorePPDefinitions) { verifyNoChange("#define A a a a a", Style); Style.ColumnLimit = 15; - verifyNoChange("#define A //a very long comment\n", Style); + verifyNoChange("#define A //a very long comment", Style); // in the following examples, since second line will not be formtted, it won't // take into considertaion the alignment from the first line. The third line // will follow the second line's alignment. verifyFormat("int aaaaaa; // a\n" "#define A // a\n" - "int a; // a\n", + "int a; // a", "int aaaaaa; // a\n" "#define A // a\n" - "int a; // a\n", + "int a; // a", Style); Style.ColumnLimit = 0; @@ -24286,7 +24286,7 @@ TEST_F(FormatTest, IgnorePPDefinitions) { Style.IndentPPDirectives = FormatStyle::PPDIS_None; verifyNoChange("#if A\n" "#define A a\n" - "#endif\n", + "#endif", Style); verifyNoChange("#define UNITY 1\n" "#if A\n" @@ -24324,6 +24324,16 @@ TEST_F(FormatTest, IgnorePPDefinitions) { " # define A a\n" "#endif", Style); + + // With comments. + verifyNoChange("/* */ # define A a // a a", Style); + verifyFormat("int a; // a\n" + "#define A // a\n" + "int aaa; // a", + "int a; // a\n" + "#define A // a\n" + "int aaa; // a", + Style); } TEST_F(FormatTest, VeryLongNamespaceCommentSplit) { >From 7d73cc8d002eaa8dbabc834deea639e88ed7adf6 Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Wed, 22 Nov 2023 13:36:51 +0100 Subject: [PATCH 11/15] Fixed code style --- clang/lib/Format/UnwrappedLineFormatter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 0a49e1cc282372e..4ff483bde022648 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1308,7 +1308,7 @@ class OptimizingLineFormatter : public LineFormatter { } // anonymous namespace -static bool lineContainsPPDefinition(const AnnotatedLine &Line) { +static bool LineContainsPPDefinition(const AnnotatedLine &Line) { auto *Tok = Line.getFirstNonComment(); if (!Tok || !Tok->is(tok::hash) || !Tok->Next) return false; @@ -1363,7 +1363,7 @@ unsigned UnwrappedLineFormatter::format( Indent != TheLine.First->OriginalColumn; bool ShouldFormat = TheLine.Affected || FixIndentation; - if (Style.IgnorePPDefinitions && lineContainsPPDefinition(TheLine)) + if (Style.IgnorePPDefinitions && LineContainsPPDefinition(TheLine)) ShouldFormat = false; // We cannot format this line; if the reason is that the line had a >From 4ab55bd1c1e3de1871d0e5a0db9086a5af90ea9e Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Thu, 23 Nov 2023 15:45:35 +0100 Subject: [PATCH 12/15] Renamed the option name to SkipMacroDefinition based on feedback --- clang/docs/ClangFormatStyleOptions.rst | 10 +++++----- clang/docs/ReleaseNotes.rst | 2 +- clang/include/clang/Format/Format.h | 10 +++++----- clang/lib/Format/Format.cpp | 4 ++-- clang/lib/Format/UnwrappedLineFormatter.cpp | 2 +- clang/lib/Format/UnwrappedLineParser.cpp | 2 +- clang/unittests/Format/ConfigParseTest.cpp | 2 +- clang/unittests/Format/FormatTest.cpp | 8 ++++---- 8 files changed, 20 insertions(+), 20 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index e1d6d5756822e9c..2abf6bccafdff98 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -3232,11 +3232,6 @@ the configuration (without a prefix: ``Auto``). For example: `KJ_IF_MAYBE <https://github.com/capnproto/capnproto/blob/master/kjdoc/tour.md#maybes>`_ -.. _IgnorePPDefinitions: - -**IgnorePPDefinitions** (``Boolean``) :versionbadge:`clang-format 18` :ref:`¶ <IgnorePPDefinitions>` - Ignore formatting in preprocessor definitions. - .. _IncludeBlocks: **IncludeBlocks** (``IncludeBlocksStyle``) :versionbadge:`clang-format 6` :ref:`¶ <IncludeBlocks>` @@ -4905,6 +4900,11 @@ the configuration (without a prefix: ``Auto``). int bar; int bar; } // namespace b } // namespace b +.. _SkipMacroDefinition: + +**SkipMacroDefinition** (``Boolean``) :versionbadge:`clang-format 18` :ref:`¶ <SkipMacroDefinition>` + Do not format macro definitions. + .. _SortIncludes: **SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8` :ref:`¶ <SortIncludes>` diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e2e8ee8d76d2e10..a1cc6bc4c85a463 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -960,8 +960,8 @@ clang-format ------------ - Add ``AllowBreakBeforeNoexceptSpecifier`` option. - Add ``AllowShortCompoundRequirementOnASingleLine`` option. -- Change ``BreakAfterAttributes`` from ``Never`` to ``Leave`` in LLVM style. - Add ``BreakAdjacentStringLiterals`` option. +- Change ``BreakAfterAttributes`` from ``Never`` to ``Leave`` in LLVM style. libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index c48c8c94559ca77..4b2a772052c1d17 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -2513,10 +2513,6 @@ struct FormatStyle { /// \version 13 std::vector<std::string> IfMacros; - /// Ignore formatting in preprocessor definitions. - /// \version 18 - bool IgnorePPDefinitions; - /// Specify whether access modifiers should have their own indentation level. /// /// When ``false``, access modifiers are indented (or outdented) relative to @@ -3888,6 +3884,10 @@ struct FormatStyle { /// \version 13 unsigned ShortNamespaceLines; + /// Do not format macro definitions. + /// \version 18 + bool SkipMacroDefinition; + /// Include sorting options. enum SortIncludesOptions : int8_t { /// Includes are never sorted. @@ -4788,7 +4788,6 @@ struct FormatStyle { R.ExperimentalAutoDetectBinPacking && FixNamespaceComments == R.FixNamespaceComments && ForEachMacros == R.ForEachMacros && - IgnorePPDefinitions == R.IgnorePPDefinitions && IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks && IncludeStyle.IncludeCategories == R.IncludeStyle.IncludeCategories && IncludeStyle.IncludeIsMainRegex == @@ -4850,6 +4849,7 @@ struct FormatStyle { RequiresExpressionIndentation == R.RequiresExpressionIndentation && SeparateDefinitionBlocks == R.SeparateDefinitionBlocks && ShortNamespaceLines == R.ShortNamespaceLines && + SkipMacroDefinition == R.SkipMacroDefinition && SortIncludes == R.SortIncludes && SortJavaStaticImport == R.SortJavaStaticImport && SpaceAfterCStyleCast == R.SpaceAfterCStyleCast && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index a6e451595527e73..b5d023e83f96d27 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1002,7 +1002,6 @@ template <> struct MappingTraits<FormatStyle> { IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments); IO.mapOptional("ForEachMacros", Style.ForEachMacros); IO.mapOptional("IfMacros", Style.IfMacros); - IO.mapOptional("IgnorePPDefinitions", Style.IgnorePPDefinitions); IO.mapOptional("IncludeBlocks", Style.IncludeStyle.IncludeBlocks); IO.mapOptional("IncludeCategories", Style.IncludeStyle.IncludeCategories); IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex); @@ -1082,6 +1081,7 @@ template <> struct MappingTraits<FormatStyle> { Style.RequiresExpressionIndentation); IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks); IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines); + IO.mapOptional("SkipMacroDefinition", Style.SkipMacroDefinition); IO.mapOptional("SortIncludes", Style.SortIncludes); IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport); IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations); @@ -1508,7 +1508,6 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.ForEachMacros.push_back("Q_FOREACH"); LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH"); LLVMStyle.IfMacros.push_back("KJ_IF_MAYBE"); - LLVMStyle.IgnorePPDefinitions = false; LLVMStyle.IncludeStyle.IncludeCategories = { {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0, false}, {"^(<|\"(gtest|gmock|isl|json)/)", 3, 0, false}, @@ -1557,6 +1556,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_OuterScope; LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave; LLVMStyle.ShortNamespaceLines = 1; + LLVMStyle.SkipMacroDefinition = false; LLVMStyle.SortIncludes = FormatStyle::SI_CaseSensitive; LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before; LLVMStyle.SortUsingDeclarations = FormatStyle::SUD_LexicographicNumeric; diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 4ff483bde022648..1fde54d407e100d 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1363,7 +1363,7 @@ unsigned UnwrappedLineFormatter::format( Indent != TheLine.First->OriginalColumn; bool ShouldFormat = TheLine.Affected || FixIndentation; - if (Style.IgnorePPDefinitions && LineContainsPPDefinition(TheLine)) + if (Style.SkipMacroDefinition && LineContainsPPDefinition(TheLine)) ShouldFormat = false; // We cannot format this line; if the reason is that the line had a diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index bd4e15feb0d289e..13006993e80eec6 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -1138,7 +1138,7 @@ void UnwrappedLineParser::parsePPDefine() { return; } - if (Style.IgnorePPDefinitions) { + if (Style.SkipMacroDefinition) { do { nextToken(); } while (!eof()); diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 7a93ec71388eb71..f1fd478a4c3d1e4 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -167,7 +167,6 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(DerivePointerAlignment); CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding"); CHECK_PARSE_BOOL(DisableFormat); - CHECK_PARSE_BOOL(IgnorePPDefinitions); CHECK_PARSE_BOOL(IndentAccessModifiers); CHECK_PARSE_BOOL(IndentCaseLabels); CHECK_PARSE_BOOL(IndentCaseBlocks); @@ -185,6 +184,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(ReflowComments); CHECK_PARSE_BOOL(RemoveBracesLLVM); CHECK_PARSE_BOOL(RemoveSemicolon); + CHECK_PARSE_BOOL(SkipMacroDefinition); CHECK_PARSE_BOOL(SpacesInSquareBrackets); CHECK_PARSE_BOOL(SpaceInEmptyBlock); CHECK_PARSE_BOOL(SpacesInContainerLiterals); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index d6eb5378b813cc5..876bf19cbf61bb0 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24219,9 +24219,9 @@ TEST_F(FormatTest, WhitespaceSensitiveMacros) { verifyNoChange("FOO(String-ized&Messy+But: :Still=Intentional);", Style); } -TEST_F(FormatTest, IgnorePPDefinitions) { - FormatStyle Style = getLLVMStyle(); - Style.IgnorePPDefinitions = true; +TEST_F(FormatTest, SkipMacroDefinition) { + auto Style = getLLVMStyle(); + Style.SkipMacroDefinition = true; verifyNoChange("#define A", Style); verifyNoChange("#define A b", Style); @@ -24316,7 +24316,7 @@ TEST_F(FormatTest, IgnorePPDefinitions) { Style); Style.IndentPPDirectives = FormatStyle::PPDIS_None; - // IgnorePPDefinitions should not affect other PP directives + // SkipMacroDefinition should not affect other PP directives verifyFormat("#if !defined(A)\n" "# define A a\n" "#endif", >From a0388addc03de16661d41588673360debc8233d9 Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Thu, 23 Nov 2023 16:14:11 +0100 Subject: [PATCH 13/15] Simplified condition --- clang/lib/Format/UnwrappedLineFormatter.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 1fde54d407e100d..0db2b738c11002c 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1308,13 +1308,6 @@ class OptimizingLineFormatter : public LineFormatter { } // anonymous namespace -static bool LineContainsPPDefinition(const AnnotatedLine &Line) { - auto *Tok = Line.getFirstNonComment(); - if (!Tok || !Tok->is(tok::hash) || !Tok->Next) - return false; - return Tok->Next->is(tok::pp_define); -} - unsigned UnwrappedLineFormatter::format( const SmallVectorImpl<AnnotatedLine *> &Lines, bool DryRun, int AdditionalIndent, bool FixBadIndentation, unsigned FirstStartColumn, @@ -1363,7 +1356,7 @@ unsigned UnwrappedLineFormatter::format( Indent != TheLine.First->OriginalColumn; bool ShouldFormat = TheLine.Affected || FixIndentation; - if (Style.SkipMacroDefinition && LineContainsPPDefinition(TheLine)) + if (Style.SkipMacroDefinition && TheLine.startsWith(tok::hash, tok::pp_define)) ShouldFormat = false; // We cannot format this line; if the reason is that the line had a >From edc929ba16701e4ca5fe17f1e745f3c348f06a4f Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Tue, 28 Nov 2023 16:07:25 +0100 Subject: [PATCH 14/15] changed the logic to only ignore the body of a PP define --- clang/lib/Format/ContinuationIndenter.cpp | 15 ++++- clang/lib/Format/ContinuationIndenter.h | 8 ++- clang/lib/Format/UnwrappedLineFormatter.cpp | 3 - clang/lib/Format/UnwrappedLineParser.cpp | 18 +++--- clang/unittests/Format/FormatTest.cpp | 69 +++++++++++++-------- 5 files changed, 71 insertions(+), 42 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 8fffdccd35c059b..12b3fccecc44306 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -739,8 +739,18 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, } if (!DryRun) { - Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, Spaces, - State.Column + Spaces + PPColumnCorrection); + if (Style.SkipMacroDefinition && CurrentState.IsPPDefineBody && + !Current.is(tok::comment)) { + Whitespaces.addUntouchableToken(Current, State.Line->InPPDirective); + } else { + Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, Spaces, + State.Column + Spaces + PPColumnCorrection); + } + } + + if (Style.SkipMacroDefinition && Previous.is(tok::pp_define)) { + CurrentState.NoLineBreak = true; + CurrentState.IsPPDefineBody = true; } // If "BreakBeforeInheritanceComma" mode, don't break within the inheritance @@ -1888,6 +1898,7 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State, NewState.NestedBlockIndent = NestedBlockIndent; NewState.BreakBeforeParameter = BreakBeforeParameter; NewState.HasMultipleNestedBlocks = (Current.BlockParameterCount > 1); + NewState.IsPPDefineBody = CurrentState.IsPPDefineBody; if (Style.BraceWrapping.BeforeLambdaBody && Current.Next && Current.is(tok::l_paren)) { diff --git a/clang/lib/Format/ContinuationIndenter.h b/clang/lib/Format/ContinuationIndenter.h index 057b85bd32d5051..d1a611bfa3aafe2 100644 --- a/clang/lib/Format/ContinuationIndenter.h +++ b/clang/lib/Format/ContinuationIndenter.h @@ -212,7 +212,8 @@ struct ParenState { ObjCSelectorNameFound(false), HasMultipleNestedBlocks(false), NestedBlockInlined(false), IsInsideObjCArrayLiteral(false), IsCSharpGenericTypeConstraint(false), IsChainedConditional(false), - IsWrappedConditional(false), UnindentOperator(false) {} + IsPPDefineBody(false), IsWrappedConditional(false), + UnindentOperator(false) {} /// \brief The token opening this parenthesis level, or nullptr if this level /// is opened by fake parenthesis. @@ -349,6 +350,9 @@ struct ParenState { /// a chained conditional expression (e.g. else-if) bool IsChainedConditional : 1; + /// \brief true if in pre-processor define body + bool IsPPDefineBody : 1; + /// \brief true if there conditionnal was wrapped on the first operator (the /// question mark) bool IsWrappedConditional : 1; @@ -402,6 +406,8 @@ struct ParenState { return IsCSharpGenericTypeConstraint; if (IsChainedConditional != Other.IsChainedConditional) return IsChainedConditional; + if (IsPPDefineBody != Other.IsPPDefineBody) + return IsPPDefineBody; if (IsWrappedConditional != Other.IsWrappedConditional) return IsWrappedConditional; if (UnindentOperator != Other.UnindentOperator) diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 0db2b738c11002c..2b66bb18edf9b38 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1356,9 +1356,6 @@ unsigned UnwrappedLineFormatter::format( Indent != TheLine.First->OriginalColumn; bool ShouldFormat = TheLine.Affected || FixIndentation; - if (Style.SkipMacroDefinition && TheLine.startsWith(tok::hash, tok::pp_define)) - ShouldFormat = false; - // We cannot format this line; if the reason is that the line had a // parsing error, remember that. if (ShouldFormat && TheLine.Type == LT_Invalid && Status) { diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 13006993e80eec6..01b01fbfce1234c 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -1138,14 +1138,6 @@ void UnwrappedLineParser::parsePPDefine() { return; } - if (Style.SkipMacroDefinition) { - do { - nextToken(); - } while (!eof()); - addUnwrappedLine(); - return; - } - if (IncludeGuard == IG_IfNdefed && IncludeGuardToken->TokenText == FormatTok->TokenText) { IncludeGuard = IG_Defined; @@ -1165,7 +1157,15 @@ void UnwrappedLineParser::parsePPDefine() { // guard processing above, and changes preprocessing nesting. FormatTok->Tok.setKind(tok::identifier); FormatTok->Tok.setIdentifierInfo(Keywords.kw_internal_ident_after_define); - nextToken(); + + if (Style.SkipMacroDefinition) { + do { + nextToken(); + } while (!eof()); + } else { + nextToken(); + } + if (FormatTok->Tok.getKind() == tok::l_paren && !FormatTok->hasWhitespaceBefore()) { parseParens(); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 876bf19cbf61bb0..abeb5609025ef12 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24223,17 +24223,24 @@ TEST_F(FormatTest, SkipMacroDefinition) { auto Style = getLLVMStyle(); Style.SkipMacroDefinition = true; - verifyNoChange("#define A", Style); + verifyFormat("#define A", "#define A", Style); + verifyFormat("#define A a aa", "#define A a aa", Style); verifyNoChange("#define A b", Style); verifyNoChange("#define A ( args )", Style); verifyNoChange("#define A ( args ) = func ( args )", Style); + verifyNoChange("#define A ( args ) { int a = 1 ; }", Style); + verifyNoChange("#define A ( args ) \\\n" + " {\\\n" + " int a = 1 ;\\\n" + "}", + Style); verifyNoChange("#define A x:", Style); verifyNoChange("#define A a. b", Style); // Surrounded with formatted code. verifyFormat("int a;\n" - "#define A a\n" + "#define A a\n" "int a;", "int a ;\n" "#define A a\n" @@ -24242,21 +24249,15 @@ TEST_F(FormatTest, SkipMacroDefinition) { // Columns are not broken when a limit is set. Style.ColumnLimit = 10; + verifyFormat("#define A a a a a", " # define A a a a a ", Style); verifyNoChange("#define A a a a a", Style); Style.ColumnLimit = 15; - verifyNoChange("#define A //a very long comment", Style); - // in the following examples, since second line will not be formtted, it won't - // take into considertaion the alignment from the first line. The third line - // will follow the second line's alignment. - verifyFormat("int aaaaaa; // a\n" - "#define A // a\n" - "int a; // a", - "int aaaaaa; // a\n" - "#define A // a\n" - "int a; // a", - Style); - + verifyFormat("#define A // a\n" + " // very\n" + " // long\n" + " // comment", + "#define A //a very long comment", Style); Style.ColumnLimit = 0; // Multiline definition. @@ -24288,12 +24289,6 @@ TEST_F(FormatTest, SkipMacroDefinition) { "#define A a\n" "#endif", Style); - verifyNoChange("#define UNITY 1\n" - "#if A\n" - "# define A a\\\n" - " a a\n" - "#endif", - Style); verifyFormat("#if A\n" "#define A a\n" "#endif", @@ -24302,11 +24297,15 @@ TEST_F(FormatTest, SkipMacroDefinition) { "#endif", Style); Style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash; + verifyNoChange("#if A\n" + "# define A a\n" + "#endif", + Style); verifyFormat("#if A\n" "# define A a\n" "#endif", "#if A\n" - " # define A a\n" + " #define A a\n" "#endif", Style); Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash; @@ -24314,26 +24313,42 @@ TEST_F(FormatTest, SkipMacroDefinition) { " #define A a\n" "#endif", Style); + verifyFormat("#if A\n" + " #define A a\n" + "#endif", + "#if A\n" + " # define A a\n" + "#endif", + Style); Style.IndentPPDirectives = FormatStyle::PPDIS_None; // SkipMacroDefinition should not affect other PP directives verifyFormat("#if !defined(A)\n" - "# define A a\n" + "#define A a\n" "#endif", "#if ! defined ( A )\n" - " # define A a\n" + " #define A a\n" "#endif", Style); // With comments. - verifyNoChange("/* */ # define A a // a a", Style); - verifyFormat("int a; // a\n" - "#define A // a\n" - "int aaa; // a", + verifyFormat("/* */ #define A a // a a", "/* */ # define A a // a a", + Style); + verifyNoChange("/* */ #define A a // a a", Style); + + verifyFormat("int a; // a\n" + "#define A // a\n" + "int aaa; // a", "int a; // a\n" "#define A // a\n" "int aaa; // a", Style); + + // multiline macro definitions + verifyNoChange("#define A a\\\n" + " A a \\\n " + " A a", + Style); } TEST_F(FormatTest, VeryLongNamespaceCommentSplit) { >From 75ae623406b1f7cac1d0224ecdd3ac6c33debfe1 Mon Sep 17 00:00:00 2001 From: Tomek Paszek <to...@unity3d.com> Date: Tue, 28 Nov 2023 16:13:57 +0100 Subject: [PATCH 15/15] renamed the setting to SkipMacroDefinitionBody --- clang/docs/ClangFormatStyleOptions.rst | 6 +++--- clang/docs/ReleaseNotes.rst | 1 + clang/include/clang/Format/Format.h | 6 +++--- clang/lib/Format/ContinuationIndenter.cpp | 4 ++-- clang/lib/Format/Format.cpp | 4 ++-- clang/lib/Format/UnwrappedLineFormatter.cpp | 1 - clang/lib/Format/UnwrappedLineParser.cpp | 2 +- clang/unittests/Format/ConfigParseTest.cpp | 2 +- clang/unittests/Format/FormatTest.cpp | 6 +++--- 9 files changed, 16 insertions(+), 16 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 2abf6bccafdff98..262318a8b833a3a 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -4900,10 +4900,10 @@ the configuration (without a prefix: ``Auto``). int bar; int bar; } // namespace b } // namespace b -.. _SkipMacroDefinition: +.. _SkipMacroDefinitionBody: -**SkipMacroDefinition** (``Boolean``) :versionbadge:`clang-format 18` :ref:`¶ <SkipMacroDefinition>` - Do not format macro definitions. +**SkipMacroDefinitionBody** (``Boolean``) :versionbadge:`clang-format 18` :ref:`¶ <SkipMacroDefinitionBody>` + Do not format macro definition body. .. _SortIncludes: diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a1cc6bc4c85a463..124ee3d7ea21bd6 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -961,6 +961,7 @@ clang-format - Add ``AllowBreakBeforeNoexceptSpecifier`` option. - Add ``AllowShortCompoundRequirementOnASingleLine`` option. - Add ``BreakAdjacentStringLiterals`` option. +- Add ``SkipMacroDefinitionBody`` option. - Change ``BreakAfterAttributes`` from ``Never`` to ``Leave`` in LLVM style. libclang diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 4b2a772052c1d17..247e336656f5e61 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -3884,9 +3884,9 @@ struct FormatStyle { /// \version 13 unsigned ShortNamespaceLines; - /// Do not format macro definitions. + /// Do not format macro definition body. /// \version 18 - bool SkipMacroDefinition; + bool SkipMacroDefinitionBody; /// Include sorting options. enum SortIncludesOptions : int8_t { @@ -4849,7 +4849,7 @@ struct FormatStyle { RequiresExpressionIndentation == R.RequiresExpressionIndentation && SeparateDefinitionBlocks == R.SeparateDefinitionBlocks && ShortNamespaceLines == R.ShortNamespaceLines && - SkipMacroDefinition == R.SkipMacroDefinition && + SkipMacroDefinitionBody == R.SkipMacroDefinitionBody && SortIncludes == R.SortIncludes && SortJavaStaticImport == R.SortJavaStaticImport && SpaceAfterCStyleCast == R.SpaceAfterCStyleCast && diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 12b3fccecc44306..33b9d7f15cb3cb9 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -739,7 +739,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, } if (!DryRun) { - if (Style.SkipMacroDefinition && CurrentState.IsPPDefineBody && + if (Style.SkipMacroDefinitionBody && CurrentState.IsPPDefineBody && !Current.is(tok::comment)) { Whitespaces.addUntouchableToken(Current, State.Line->InPPDirective); } else { @@ -748,7 +748,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, } } - if (Style.SkipMacroDefinition && Previous.is(tok::pp_define)) { + if (Style.SkipMacroDefinitionBody && Previous.is(tok::pp_define)) { CurrentState.NoLineBreak = true; CurrentState.IsPPDefineBody = true; } diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index b5d023e83f96d27..bb7411d9e70381b 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1081,7 +1081,7 @@ template <> struct MappingTraits<FormatStyle> { Style.RequiresExpressionIndentation); IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks); IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines); - IO.mapOptional("SkipMacroDefinition", Style.SkipMacroDefinition); + IO.mapOptional("SkipMacroDefinitionBody", Style.SkipMacroDefinitionBody); IO.mapOptional("SortIncludes", Style.SortIncludes); IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport); IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations); @@ -1556,7 +1556,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_OuterScope; LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave; LLVMStyle.ShortNamespaceLines = 1; - LLVMStyle.SkipMacroDefinition = false; + LLVMStyle.SkipMacroDefinitionBody = false; LLVMStyle.SortIncludes = FormatStyle::SI_CaseSensitive; LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before; LLVMStyle.SortUsingDeclarations = FormatStyle::SUD_LexicographicNumeric; diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 2b66bb18edf9b38..280485d9a90d1bf 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1355,7 +1355,6 @@ unsigned UnwrappedLineFormatter::format( bool FixIndentation = (FixBadIndentation || ContinueFormatting) && Indent != TheLine.First->OriginalColumn; bool ShouldFormat = TheLine.Affected || FixIndentation; - // We cannot format this line; if the reason is that the line had a // parsing error, remember that. if (ShouldFormat && TheLine.Type == LT_Invalid && Status) { diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 01b01fbfce1234c..970c31d0d4cde92 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -1158,7 +1158,7 @@ void UnwrappedLineParser::parsePPDefine() { FormatTok->Tok.setKind(tok::identifier); FormatTok->Tok.setIdentifierInfo(Keywords.kw_internal_ident_after_define); - if (Style.SkipMacroDefinition) { + if (Style.SkipMacroDefinitionBody) { do { nextToken(); } while (!eof()); diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index f1fd478a4c3d1e4..52f84525159a188 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -184,7 +184,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(ReflowComments); CHECK_PARSE_BOOL(RemoveBracesLLVM); CHECK_PARSE_BOOL(RemoveSemicolon); - CHECK_PARSE_BOOL(SkipMacroDefinition); + CHECK_PARSE_BOOL(SkipMacroDefinitionBody); CHECK_PARSE_BOOL(SpacesInSquareBrackets); CHECK_PARSE_BOOL(SpaceInEmptyBlock); CHECK_PARSE_BOOL(SpacesInContainerLiterals); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index abeb5609025ef12..0da133d56baf255 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24219,9 +24219,9 @@ TEST_F(FormatTest, WhitespaceSensitiveMacros) { verifyNoChange("FOO(String-ized&Messy+But: :Still=Intentional);", Style); } -TEST_F(FormatTest, SkipMacroDefinition) { +TEST_F(FormatTest, SkipMacroDefinitionBody) { auto Style = getLLVMStyle(); - Style.SkipMacroDefinition = true; + Style.SkipMacroDefinitionBody = true; verifyFormat("#define A", "#define A", Style); verifyFormat("#define A a aa", "#define A a aa", Style); @@ -24322,7 +24322,7 @@ TEST_F(FormatTest, SkipMacroDefinition) { Style); Style.IndentPPDirectives = FormatStyle::PPDIS_None; - // SkipMacroDefinition should not affect other PP directives + // SkipMacroDefinitionBody should not affect other PP directives verifyFormat("#if !defined(A)\n" "#define A a\n" "#endif", _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits