llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Devansh Varshney (देवांश वार्ष्णेय) (varshneydevansh) <details> <summary>Changes</summary> ## The basics <!-- TODO: Verify the following, checking each box with an 'x' between the brackets: [x] --> - [x] I branched from the main - [x] My pull request is against main - [x] My code follows the llvm Style Guide ## The details ### Resolves <!-- TODO: What Github issue does this resolve? Please include a link. --> - Fixes #<!-- -->24492 ### Proposed Changes Add a clang format style options for the `goto` to get the proper desired output. <!-- TODO: Describe what this Pull Request does. Include screenshots if applicable. --> #### Behavior Before Change Our handling of `goto` labels isn't ideal, as often we are not getting the desired output and missing out the proper indentation for the got label. <!--TODO: Image, gif or explanation of behavior before this pull request. --> #### Behavior After Change The user will have three options to determine how they want their `goto` labels to be formatted with the 3 available options- ```cc enum GotoLabelIndentation { GLI_None, // Do not indent goto labels. GLI_Indent, // Indent goto labels at the same level as the surrounding code. GLI_HalfIndent, // Indent goto labels at a half indent level. }; ``` <!--TODO: Image, gif or explanation of behavior after this pull request. --> ### Test Coverage I haven't tested this, as soon as I got confirmation that these are the required changes that will update the status of the Test Coverage. <!-- TODO: Please create unit tests, and explain here how they cover your changes, or tell us how you tested it manually. If your changes include browser-specific behavior, include information about the browser and device that you used for testing. --> ### Documentation Documentation needs to be done for this as it's a feature request. Once this will pass all the test cases I will add the required document for this. <!-- TODO: Does any documentation need to be created or updated because of this PR? - If so please explain. --> ### Additional Information <!-- Anything else we should know? --> --- Full diff: https://github.com/llvm/llvm-project/pull/65140.diff 5 Files Affected: - (modified) clang/docs/ClangFormatStyleOptions.rst (+11-11) - (modified) clang/include/clang/Format/Format.h (+19-11) - (modified) clang/lib/Format/Format.cpp (+9-1) - (modified) clang/lib/Format/UnwrappedLineParser.cpp (+1-1) - (modified) clang/unittests/Format/ConfigParseTest.cpp (+12-1) ``````````diff diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 7847f6aa5fb1c9..d76af9f2fcb272 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -3311,22 +3311,22 @@ the configuration (without a prefix: ``Auto``). .. _IndentGotoLabels: -**IndentGotoLabels** (``Boolean``) :versionbadge:`clang-format 10` :ref:`¶ <IndentGotoLabels>` +**IndentGotoLabels** (``enum``) :versionbadge:`clang-format 10` :ref:`¶ <IndentGotoLabels>` Indent goto labels. - When ``false``, goto labels are flushed left. + When ``GLI_None``, goto labels are flushed left. .. code-block:: c++ - true: false: - int f() { vs. int f() { - if (foo()) { if (foo()) { - label1: label1: - bar(); bar(); - } } - label2: label2: - return 1; return 1; - } } + GLI_Indent: GLI_None: + int f() { vs. int f() { + if (foo()) { if (foo()) { + label1: label1: + bar(); bar(); + } } + label2: label2: + return 1; return 1; + } } .. _IndentPPDirectives: diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index b641b313e32d98..4bc67d1908460c 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -2421,20 +2421,28 @@ struct FormatStyle { /// Indent goto labels. /// - /// When ``false``, goto labels are flushed left. /// \code - /// true: false: - /// int f() { vs. int f() { - /// if (foo()) { if (foo()) { - /// label1: label1: - /// bar(); bar(); - /// } } - /// label2: label2: - /// return 1; return 1; - /// } } + /// GLI_Indent: GLI_None: + /// namespace NS { vs. namespace NS { + /// class C { class C { + /// void foo() { void foo() { + /// while (x) { while (x) { + /// if (y) { if (y) { + /// label: label: + /// } } + /// } } + /// } } + /// } } + /// } } /// \endcode /// \version 10 - bool IndentGotoLabels; + + enum GotoLabelIndentation { + GLI_None, // Do not indent goto labels. + GLI_Indent, // Indent goto labels at the same level as the surrounding code. + }; + + GotoLabelIndentation IndentGotoLabels; /// Indents extern blocks enum IndentExternBlockStyle : int8_t { diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 339af30e38a7a8..99f7ce4bff0d3d 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1199,6 +1199,14 @@ template <> struct DocumentListTraits<std::vector<FormatStyle>> { return Seq[Index]; } }; + +template <> struct ScalarEnumerationTraits<FormatStyle::GotoLabelIndentation> { + static void enumeration(IO &IO, FormatStyle::GotoLabelIndentation &Value) { + IO.enumCase(Value, "false", FormatStyle::GLI_None); + IO.enumCase(Value, "true", FormatStyle::GLI_Indent); + } +}; + } // namespace yaml } // namespace llvm @@ -1478,7 +1486,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.IndentCaseLabels = false; LLVMStyle.IndentCaseBlocks = false; LLVMStyle.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock; - LLVMStyle.IndentGotoLabels = true; + LLVMStyle.IndentGotoLabels = FormatStyle::GLI_Indent; LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None; LLVMStyle.IndentRequiresClause = true; LLVMStyle.IndentWidth = 2; diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 5a82b200055a87..4192bfbefd03bd 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -1647,7 +1647,7 @@ void UnwrappedLineParser::parseStructuralElement( nextToken(); Line->Tokens.begin()->Tok->MustBreakBefore = true; FormatTok->setFinalizedType(TT_GotoLabelColon); - parseLabel(!Style.IndentGotoLabels); + parseLabel(Style.IndentGotoLabels != FormatStyle::GLI_None); if (HasLabel) *HasLabel = true; return; diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index b3b6755cadee4b..a0f138506d3d2f 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -141,6 +141,16 @@ TEST(ConfigParseTest, GetsCorrectBasedOnStyle) { EXPECT_EQ(0, parseConfiguration(#STRUCT ":\n " TEXT, &Style).value()); \ EXPECT_EQ(VALUE, Style.STRUCT.FIELD) << "Unexpected value after parsing!" +#define CHECK_PARSE_ENUM_FIELD(FIELD, CONFIG_NAME, ENUM_VAL) +Style.FIELD = FormatStyle::GotoLabelIndentation::GLI_None; +EXPECT_EQ(0, parseConfiguration(CONFIG_NAME ": true", &Style).value()); +EXPECT_EQ(ENUM_VAL, Style.FIELD); +EXPECT_EQ(0, parseConfiguration(CONFIG_NAME ": false", &Style).value()); +EXPECT_EQ(FormatStyle::GotoLabelIndentation::GLI_None, Style.FIELD) + +#define CHECK_PARSE_ENUM(FIELD, ENUM_VAL) \ + CHECK_PARSE_ENUM_FIELD(FIELD, #FIELD, ENUM_VAL) + TEST(ConfigParseTest, ParsesConfigurationBools) { FormatStyle Style = {}; Style.Language = FormatStyle::LK_Cpp; @@ -161,7 +171,8 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(IndentAccessModifiers); CHECK_PARSE_BOOL(IndentCaseLabels); CHECK_PARSE_BOOL(IndentCaseBlocks); - CHECK_PARSE_BOOL(IndentGotoLabels); + CHECK_PARSE_ENUM(IndentGotoLabels, + FormatStyle::GotoLabelIndentation::GLI_Indent); CHECK_PARSE_BOOL_FIELD(IndentRequiresClause, "IndentRequires"); CHECK_PARSE_BOOL(IndentRequiresClause); CHECK_PARSE_BOOL(IndentWrappedFunctionNames); `````````` </details> https://github.com/llvm/llvm-project/pull/65140 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits