llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-format
Author: None (Ezlanding1)
<details>
<summary>Changes</summary>
Updates the goto label indentation styles based on the feature request.
Resolves #<!-- -->24492.
### Changes:
>From the discussion in the issue, it seems like there are four preferred
>styles of indentation for goto labels:
- `LeftAlign` - Aligns goto labels to the left (matches the previous `false`
behavior)
- `NoIndent` - Aligns goto labels with the previous indent level (matches the
previous `true` behavior)
- `Indent` - Aligns goto labels with the current indent level
- `HalfIndent` - Aligns goto labels halfway between the current and previous
indent level
Let me know if there are any more styles or changes that should be added.
### Note:
There are certain design decisions that may need feedback:
1. `HalfIndent` does not respect PPDirective override
As before, the `NoIndent` style is overridden by a preprocessor directive, and
will behave like `Indent` if `Line->InPPDirective` is true. The `HalfIndent`
style does not respect this rule, and will always half-indent the label
regardless of the context. Let me know if I should change this behavior to
match `NoIndent`, it's just a simple check for `Line->InPPDirective` before
returning the indent offset.
2. `HalfIndent` label calculation discrepancies
For all the other styles, the label is found via the `parseLabel` function. For
`HalfIndent`, however, since it is between indent levels, an indent offset is
calculated via `RootToken.Next->is(TT_GotoLabelColon)`. This means that, in
the case of a token before the label on the same line, such as something like:
```
void foo() {
/* comment */ label:
}
```
the label will not be half-indented correctly (and will rather be completely
indented because the check will not trigger to half the last level). Should we
handle this? It is fairly uncommon, but I'd be happy to fix it if needed. I'll
add a `bool HalfIndent` to the `UnwrappedLine`, which will be set to true in
`parseLabel` and checked in `getIndentOffset`
### Unit Testing
Unit testing will be added soon. However, feedback at this stage would be
greatly appreciated to confirm my approach before tests are written
---
Full diff: https://github.com/llvm/llvm-project/pull/180109.diff
6 Files Affected:
- (modified) clang/docs/ClangFormatStyleOptions.rst (+60-13)
- (modified) clang/include/clang/Format/Format.h (+56-16)
- (modified) clang/lib/Format/Format.cpp (+13-1)
- (modified) clang/lib/Format/UnwrappedLineFormatter.cpp (+7)
- (modified) clang/lib/Format/UnwrappedLineParser.cpp (+15-5)
- (modified) clang/lib/Format/UnwrappedLineParser.h (+2-1)
``````````diff
diff --git a/clang/docs/ClangFormatStyleOptions.rst
b/clang/docs/ClangFormatStyleOptions.rst
index 5ba117c231ad5..74f9da27918b3 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -4535,22 +4535,69 @@ the configuration (without a prefix: ``Auto``).
.. _IndentGotoLabels:
-**IndentGotoLabels** (``Boolean``) :versionbadge:`clang-format 10` :ref:`¶
<IndentGotoLabels>`
- Indent goto labels.
+**IndentGotoLabels** (``IndentGotoLabelStyle``) :versionbadge:`clang-format
23` :ref:`¶ <IndentGotoLabels>`
+ The goto label indenting style to use.
- When ``false``, goto labels are flushed left.
+ Possible values:
+
+ * ``IGLS_LeftAlign`` (in configuration: ``LeftAlign``)
+ Left align goto labels.
+
+ .. code-block:: c++
+
+ int f() {
+ if (foo()) {
+ label1:
+ bar();
+ }
+ label2:
+ return 1;
+ }
+
+ * ``IGLS_NoIndent`` (in configuration: ``NoIndent``)
+ Do not indent goto labels.
+
+ .. code-block:: c++
+
+ int f() {
+ if (foo()) {
+ label1:
+ bar();
+ }
+ label2:
+ return 1;
+ }
+
+ * ``IGLS_Indent`` (in configuration: ``Indent``)
+ Indent goto labels.
+
+ .. code-block:: c++
+
+ int f() {
+ if (foo()) {
+ label1:
+ bar();
+ }
+ label2:
+ return 1;
+ }
+
+ * ``IGLS_HalfIndent`` (in configuration: ``HalfIndent``)
+ Indent goto labels to half the indentation of the surrounding code.
+ If the indentation width is not an even number, it will round down.
+
+ .. code-block:: c++
+
+ int f() {
+ if (foo()) {
+ label1:
+ bar();
+ }
+ label2:
+ return 1;
+ }
- .. code-block:: c++
- true: false:
- 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 43bea4b80cb8a..03f55ba269873 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3078,22 +3078,62 @@ struct FormatStyle {
/// \version 11
IndentExternBlockStyle IndentExternBlock;
- /// 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;
- /// } }
- /// \endcode
- /// \version 10
- bool IndentGotoLabels;
+ /// Options for indenting goto labels.
+ enum IndentGotoLabelStyle : int8_t {
+ /// Left align goto labels.
+ /// \code
+ /// int f() {
+ /// if (foo()) {
+ /// label1:
+ /// bar();
+ /// }
+ /// label2:
+ /// return 1;
+ /// }
+ /// \endcode
+ IGLS_LeftAlign,
+ /// Do not indent goto labels.
+ /// \code
+ /// int f() {
+ /// if (foo()) {
+ /// label1:
+ /// bar();
+ /// }
+ /// label2:
+ /// return 1;
+ /// }
+ /// \endcode
+ IGLS_NoIndent,
+ /// Indent goto labels.
+ /// \code
+ /// int f() {
+ /// if (foo()) {
+ /// label1:
+ /// bar();
+ /// }
+ /// label2:
+ /// return 1;
+ /// }
+ /// \endcode
+ IGLS_Indent,
+ /// Indent goto labels to half the indentation of the surrounding code.
+ /// If the indentation width is not an even number, it will round down.
+ /// \code
+ /// int f() {
+ /// if (foo()) {
+ /// label1:
+ /// bar();
+ /// }
+ /// label2:
+ /// return 1;
+ /// }
+ /// \endcode
+ IGLS_HalfIndent,
+ };
+
+ /// The goto label indenting style to use.
+ /// \version 23
+ IndentGotoLabelStyle IndentGotoLabels;
/// Options for indenting preprocessor directives.
enum PPDirectiveIndentStyle : int8_t {
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 1e68de531791f..0abfd534631cb 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1465,6 +1465,18 @@ template <> struct
DocumentListTraits<std::vector<FormatStyle>> {
return Seq[Index];
}
};
+
+template <> struct ScalarEnumerationTraits<FormatStyle::IndentGotoLabelStyle> {
+ static void enumeration(IO &IO, FormatStyle::IndentGotoLabelStyle &Value) {
+ IO.enumCase(Value, "LeftAlign", FormatStyle::IGLS_LeftAlign);
+ IO.enumCase(Value, "NoIndent", FormatStyle::IGLS_NoIndent);
+ IO.enumCase(Value, "Indent", FormatStyle::IGLS_Indent);
+ IO.enumCase(Value, "HalfIndent", FormatStyle::IGLS_HalfIndent);
+ IO.enumCase(Value, "false", FormatStyle::IGLS_LeftAlign);
+ IO.enumCase(Value, "true", FormatStyle::IGLS_NoIndent);
+ }
+};
+
} // namespace yaml
} // namespace llvm
@@ -1759,7 +1771,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind
Language) {
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.IndentExportBlock = true;
LLVMStyle.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
- LLVMStyle.IndentGotoLabels = true;
+ LLVMStyle.IndentGotoLabels = FormatStyle::IGLS_NoIndent;
LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
LLVMStyle.IndentRequiresClause = true;
LLVMStyle.IndentWidth = 2;
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 8589aa83f6c55..552038d1e0753 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -120,7 +120,14 @@ class LevelIndentTracker {
int getIndentOffset(const AnnotatedLine &Line) {
if (Style.isJava() || Style.isJavaScript() || Style.isCSharp())
return 0;
+
const auto &RootToken = *Line.First;
+
+ if (Style.IndentGotoLabels == FormatStyle::IGLS_HalfIndent &&
+ RootToken.Next != NULL && RootToken.Next->is(TT_GotoLabelColon)) {
+ return -static_cast<int>(Style.IndentWidth / 2);
+ }
+
if (Line.Type == LT_AccessModifier ||
RootToken.isAccessSpecifier(/*ColonRequired=*/false) ||
RootToken.isObjCAccessSpecifier() ||
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp
b/clang/lib/Format/UnwrappedLineParser.cpp
index f57ef1328eac7..14e72ffdd8aa7 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1710,7 +1710,7 @@ void UnwrappedLineParser::parseStructuralElement(
if (!Line->InMacroBody || CurrentLines->size() > 1)
Line->Tokens.begin()->Tok->MustBreakBefore = true;
FormatTok->setFinalizedType(TT_GotoLabelColon);
- parseLabel(!Style.IndentGotoLabels);
+ parseLabel(Style.IndentGotoLabels);
if (HasLabel)
*HasLabel = true;
return;
@@ -3354,14 +3354,24 @@ void UnwrappedLineParser::parseDoWhile() {
parseStructuralElement();
}
-void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
+void UnwrappedLineParser::parseLabel(
+ FormatStyle::IndentGotoLabelStyle IndentGotoLabels) {
nextToken();
unsigned OldLineLevel = Line->Level;
- if (LeftAlignLabel)
+ switch (IndentGotoLabels) {
+ case FormatStyle::IGLS_LeftAlign:
Line->Level = 0;
- else if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
- --Line->Level;
+ break;
+ case FormatStyle::IGLS_NoIndent:
+ if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
+ --Line->Level;
+ break;
+ case FormatStyle::IGLS_HalfIndent:
+ case FormatStyle::IGLS_Indent:
+ default:
+ break;
+ }
if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() &&
FormatTok->is(tok::l_brace)) {
diff --git a/clang/lib/Format/UnwrappedLineParser.h
b/clang/lib/Format/UnwrappedLineParser.h
index 86022d9b316c6..48119e8802b57 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -159,7 +159,8 @@ class UnwrappedLineParser {
void parseLoopBody(bool KeepBraces, bool WrapRightBrace);
void parseForOrWhileLoop(bool HasParens = true);
void parseDoWhile();
- void parseLabel(bool LeftAlignLabel = false);
+ void parseLabel(FormatStyle::IndentGotoLabelStyle IndentGotoLabels =
+ FormatStyle::IGLS_NoIndent);
void parseCaseLabel();
void parseSwitch(bool IsExpr);
void parseNamespace();
``````````
</details>
https://github.com/llvm/llvm-project/pull/180109
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
