[clang] [clang-format] modified goto bool to enum (PR #65140)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 15 00:40:53 PDT 2024
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
More information about the cfe-commits
mailing list