Author: n0ma Date: 2026-10-03T13:13:54Z New Revision: cd8d3f00c51c2b5a0ebaad76d1a148ed0208953f
URL: https://github.com/llvm/llvm-project/commit/cd8d3f00c51c2b5a0ebaad76d1a148ed0208953f DIFF: https://github.com/llvm/llvm-project/commit/cd8d3f00c51c2b5a0ebaad76d1a148ed0208953f.diff LOG: [clang-format] Allow one indent level before explicit access labels (#227704) ## Summary With `IndentAccessModifiers: true`, clang-format indents record members two levels even when the record has no explicit access label. Extend `IndentAccessModifiers` with a third mode to allow one level until the first explicit access label: - `Never` (`false`): indent members one level and use `AccessModifierOffset` for access labels. - `Always` (`true`): indent members two levels, even without an explicit access label. - `AfterFirstAccessModifier`: indent members one level before the first explicit access label and two levels from that label onward. ```yaml IndentAccessModifiers: AfterFirstAccessModifier ``` It recognizes C++ and Qt access labels. When a record has no explicit label, its members stay at one level. The new behavior applies to C, C++, and Objective-C; other languages treat the new mode like `Always`. Existing configuration values `true` and `false` retain their behavior, and style defaults are unchanged. C++ callers assigning a boolean directly to `FormatStyle::IndentAccessModifiers` must now use the corresponding enum value. This makes the layout in #61631 possible without changing existing configurations. It also addresses the related requests in #54333 and #182566. ## Design point for review If a record has members before its first explicit access label, those members use one level, even when an access label appears later. Reviewers may prefer a different option name or rule for that case. ## Development note AI assisted with the fix as I'm not used to the codebase. I reviewed the resulting patch and tests before submitting this PR. Added: Modified: clang/docs/ClangFormatStyleOptions.md clang/docs/ReleaseNotes.md clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp clang/lib/Format/UnwrappedLineFormatter.cpp clang/lib/Format/UnwrappedLineParser.cpp clang/lib/Format/UnwrappedLineParser.h clang/unittests/Format/ConfigParseTest.cpp clang/unittests/Format/FormatTest.cpp clang/unittests/Format/FormatTestCSharp.cpp clang/unittests/Format/FormatTestJava.cpp Removed: ################################################################################ diff --git a/clang/docs/ClangFormatStyleOptions.md b/clang/docs/ClangFormatStyleOptions.md index cd3c7b60391a0d..af81eed1c6f138 100644 --- a/clang/docs/ClangFormatStyleOptions.md +++ b/clang/docs/ClangFormatStyleOptions.md @@ -4744,33 +4744,54 @@ the configuration (without a prefix: `Auto`). (indentaccessmodifiers)= -**IndentAccessModifiers** (`Boolean`) {versionbadge}`clang-format 13` {ref}`¶ <IndentAccessModifiers>` +**IndentAccessModifiers** (`IndentAccessModifierStyle`) {versionbadge}`clang-format 13` {ref}`¶ <IndentAccessModifiers>` -: Specify whether access modifiers should have their own indentation level. +: Specify how access modifiers and record members are indented. + + Possible values: + + - `IAMS_Never` (in configuration: `Never`) + Use `AccessModifierOffset` for access modifiers and indent members one + level below the record. + + ```c++ + struct S { + int before; + + public: + int after; + }; + ``` + + - `IAMS_Always` (in configuration: `Always`) + Give access modifiers their own indentation level and indent all + members two levels below the record. Value of the `AccessModifierOffset` + is ignored. + + ```c++ + struct S { + int before; + + public: + int after; + }; + ``` + + - `IAMS_AfterFirstAccessModifier` (in configuration: `AfterFirstAccessModifier`) + In C, C++, and Objective-C, indent members one level until the first + explicit access modifier, then two levels. Other languages use the + `Always` behavior. Value of the `AccessModifierOffset` is ignored. + + ```c++ + struct S { + int before; + + public: + int after; + }; + ``` - When `false`, access modifiers are indented (or outdented) relative to - the record members, respecting the `AccessModifierOffset`. Record - members are indented one level below the record. - When `true`, access modifiers get their own indentation level. As a - consequence, record members are always indented 2 levels below the record, - regardless of the access modifier presence. Value of the - `AccessModifierOffset` is ignored. - ```c++ - false: true: - class C { vs. class C { - class D { class D { - void bar(); void bar(); - protected: protected: - D(); D(); - }; }; - public: public: - C(); C(); - }; }; - void foo() { void foo() { - return 1; return 1; - } } - ``` (indentcaseblocks)= diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 6fcca9db4b7650..2dca8fe3deed12 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -1001,6 +1001,9 @@ features cannot lower the translation-unit ABI level; - `QualifierOrder` now supports `typedef`, `consteval`, `constinit`, `thread_local`, `extern`, `mutable`, `signed`, `unsigned`, `long`, `short`, and `explicit` declaration specifiers. +- Extend `IndentAccessModifiers` with `AfterFirstAccessModifier` to indent + members before the first explicit access modifier by one level. Existing + configuration values `true` and `false` remain supported. ### libclang diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 3f3f02469fccd4..9cc887d613595c 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -3181,32 +3181,48 @@ struct FormatStyle { /// \version 13 std::vector<std::string> IfMacros; - /// Specify whether access modifiers should have their own indentation level. - /// - /// When `false`, access modifiers are indented (or outdented) relative to - /// the record members, respecting the `AccessModifierOffset`. Record - /// members are indented one level below the record. - /// When `true`, access modifiers get their own indentation level. As a - /// consequence, record members are always indented 2 levels below the record, - /// regardless of the access modifier presence. Value of the - /// `AccessModifierOffset` is ignored. - /// \code - /// false: true: - /// class C { vs. class C { - /// class D { class D { - /// void bar(); void bar(); - /// protected: protected: - /// D(); D(); - /// }; }; - /// public: public: - /// C(); C(); - /// }; }; - /// void foo() { void foo() { - /// return 1; return 1; - /// } } - /// \endcode + /// Modes for indenting access modifiers and record members. + enum IndentAccessModifierStyle : int8_t { + /// Use `AccessModifierOffset` for access modifiers and indent members one + /// level below the record. + /// \code + /// struct S { + /// int before; + /// + /// public: + /// int after; + /// }; + /// \endcode + IAMS_Never, + /// Give access modifiers their own indentation level and indent all + /// members two levels below the record. Value of the `AccessModifierOffset` + /// is ignored. + /// \code + /// struct S { + /// int before; + /// + /// public: + /// int after; + /// }; + /// \endcode + IAMS_Always, + /// In C, C++, and Objective-C, indent members one level until the first + /// explicit access modifier, then two levels. Other languages use the + /// `Always` behavior. Value of the `AccessModifierOffset` is ignored. + /// \code + /// struct S { + /// int before; + /// + /// public: + /// int after; + /// }; + /// \endcode + IAMS_AfterFirstAccessModifier, + }; + + /// Specify how access modifiers and record members are indented. /// \version 13 - bool IndentAccessModifiers; + IndentAccessModifierStyle IndentAccessModifiers; /// Indent case label blocks one level from the case label. /// diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index c49144f3291de1..c4baf8002c274d 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -477,6 +477,21 @@ struct ScalarEnumerationTraits<FormatStyle::EnumTrailingCommaStyle> { } }; +template <> +struct ScalarEnumerationTraits<FormatStyle::IndentAccessModifierStyle> { + static void enumeration(IO &IO, + FormatStyle::IndentAccessModifierStyle &Value) { + IO.enumCase(Value, "Never", FormatStyle::IAMS_Never); + IO.enumCase(Value, "Always", FormatStyle::IAMS_Always); + IO.enumCase(Value, "AfterFirstAccessModifier", + FormatStyle::IAMS_AfterFirstAccessModifier); + + // For backward compatibility. + IO.enumCase(Value, "false", FormatStyle::IAMS_Never); + IO.enumCase(Value, "true", FormatStyle::IAMS_Always); + } +}; + template <> struct ScalarEnumerationTraits<FormatStyle::IndentExternBlockStyle> { static void enumeration(IO &IO, FormatStyle::IndentExternBlockStyle &Value) { @@ -1977,7 +1992,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { {".*", 1, 0, false}}; LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$"; LLVMStyle.IncludeStyle.MainIncludeChar = tooling::IncludeStyle::MICD_Quote; - LLVMStyle.IndentAccessModifiers = false; + LLVMStyle.IndentAccessModifiers = FormatStyle::IAMS_Never; LLVMStyle.IndentCaseBlocks = false; LLVMStyle.IndentCaseLabels = false; LLVMStyle.IndentExportBlock = true; diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 11a6d359527ae4..3e922eb2ac9b90 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -136,8 +136,9 @@ class LevelIndentTracker { // The AccessModifierOffset may be overridden by IndentAccessModifiers, // in which case we take a negative value of the IndentWidth to simulate // the upper indent level. - return Style.IndentAccessModifiers ? -Style.IndentWidth - : Style.AccessModifierOffset; + return Style.IndentAccessModifiers != FormatStyle::IAMS_Never + ? -Style.IndentWidth + : Style.AccessModifierOffset; } return 0; } diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 4825e825af1fa6..901335950b9fac 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -352,7 +352,8 @@ bool UnwrappedLineParser::precededByCommentOrPPDirective() const { /// (A simple block has a single statement.) bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace, IfStmtKind *IfKind, - FormatToken **IfLeftBrace) { + FormatToken **IfLeftBrace, + bool *SeenExplicitAccessModifier) { const bool InRequiresExpression = OpeningBrace && OpeningBrace->is(TT_RequiresExpressionLBrace); const bool IsPrecededByCommentOrPPDirective = @@ -377,7 +378,18 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace, Kind = tok::r_brace; auto ParseDefault = [this, OpeningBrace, IfKind, &IfLBrace, &HasDoWhile, - &HasLabel, &StatementCount] { + &HasLabel, &StatementCount, + SeenExplicitAccessModifier] { + if (SeenExplicitAccessModifier && !*SeenExplicitAccessModifier) { + const bool IsQtAccessLabel = + FormatTok->isOneOf(Keywords.kw_signals, Keywords.kw_qsignals, + Keywords.kw_slots, Keywords.kw_qslots) && + Tokens->peekNextToken(/*SkipComment=*/true)->is(tok::colon); + if (FormatTok->isAccessSpecifierKeyword() || IsQtAccessLabel) { + ++Line->Level; + *SeenExplicitAccessModifier = true; + } + } parseStructuralElement(OpeningBrace, IfKind, &IfLBrace, HasDoWhile ? nullptr : &HasDoWhile, HasLabel ? nullptr : &HasLabel); @@ -744,11 +756,10 @@ bool UnwrappedLineParser::mightFitOnOneLine( return Line.Level * Style.IndentWidth + Length <= ColumnLimit; } -FormatToken *UnwrappedLineParser::parseBlock(bool MustBeDeclaration, - unsigned AddLevels, bool MunchSemi, - bool KeepBraces, - IfStmtKind *IfKind, - bool UnindentWhitesmithsBraces) { +FormatToken *UnwrappedLineParser::parseBlock( + bool MustBeDeclaration, unsigned AddLevels, bool MunchSemi, bool KeepBraces, + IfStmtKind *IfKind, bool UnindentWhitesmithsBraces, + bool IndentAfterExplicitAccessModifier) { auto HandleVerilogBlockLabel = [this]() { // ":" name if (Style.isVerilog() && FormatTok->is(tok::colon)) { @@ -820,7 +831,11 @@ FormatToken *UnwrappedLineParser::parseBlock(bool MustBeDeclaration, Line->Level += AddLevels - (IsWhitesmiths ? 1 : 0); FormatToken *IfLBrace = nullptr; - const bool SimpleBlock = parseLevel(Tok, IfKind, &IfLBrace); + bool SeenExplicitAccessModifier = false; + const bool SimpleBlock = + parseLevel(Tok, IfKind, &IfLBrace, + IndentAfterExplicitAccessModifier ? &SeenExplicitAccessModifier + : nullptr); if (eof()) return IfLBrace; @@ -878,6 +893,8 @@ FormatToken *UnwrappedLineParser::parseBlock(bool MustBeDeclaration, size_t PPEndHash = computePPHash(); + if (SeenExplicitAccessModifier) + ++AddLevels; // Munch the closing brace. nextToken(/*LevelDifference=*/-AddLevels); @@ -4298,8 +4315,26 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr, bool IsJavaRecord) { addUnwrappedLine(); } - unsigned AddLevels = Style.IndentAccessModifiers ? 2u : 1u; - parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/false); + bool IndentAfterExplicitAccessModifier = false; + unsigned AddLevels = 1u; + switch (Style.IndentAccessModifiers) { + case FormatStyle::IAMS_Never: + break; + case FormatStyle::IAMS_AfterFirstAccessModifier: + if (Style.isCpp()) { + IndentAfterExplicitAccessModifier = true; + break; + } + // Other languages use the same indentation as IAMS_Always. + [[fallthrough]]; + case FormatStyle::IAMS_Always: + AddLevels = 2u; + break; + } + parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/false, + /*KeepBraces=*/true, /*IfKind=*/nullptr, + /*UnindentWhitesmithsBraces=*/false, + IndentAfterExplicitAccessModifier); } setPreviousRBraceType(ClosingBraceType); } diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h index 5b93c8f346d75a..2e1755ad3b2c68 100644 --- a/clang/lib/Format/UnwrappedLineParser.h +++ b/clang/lib/Format/UnwrappedLineParser.h @@ -126,13 +126,15 @@ class UnwrappedLineParser { bool precededByCommentOrPPDirective() const; bool parseLevel(const FormatToken *OpeningBrace = nullptr, IfStmtKind *IfKind = nullptr, - FormatToken **IfLeftBrace = nullptr); + FormatToken **IfLeftBrace = nullptr, + bool *SeenExplicitAccessModifier = nullptr); bool mightFitOnOneLine(UnwrappedLine &Line, const FormatToken *OpeningBrace = nullptr) const; FormatToken *parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u, bool MunchSemi = true, bool KeepBraces = true, IfStmtKind *IfKind = nullptr, - bool UnindentWhitesmithsBraces = false); + bool UnindentWhitesmithsBraces = false, + bool IndentAfterExplicitAccessModifier = false); void parseChildBlock(); void parsePPDirective(); void parsePPDefine(); diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 32839712e297af..ab160db1e9f13c 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -189,7 +189,6 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL(DerivePointerAlignment); CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding"); CHECK_PARSE_BOOL(DisableFormat); - CHECK_PARSE_BOOL(IndentAccessModifiers); CHECK_PARSE_BOOL(IndentCaseBlocks); CHECK_PARSE_BOOL(IndentCaseLabels); CHECK_PARSE_BOOL(IndentExportBlock); @@ -1030,6 +1029,19 @@ TEST(ConfigParseTest, ParsesConfiguration) { AllowShortIfStatementsOnASingleLine, FormatStyle::SIS_WithoutElse); + Style.IndentAccessModifiers = FormatStyle::IAMS_Always; + CHECK_PARSE("IndentAccessModifiers: Never", IndentAccessModifiers, + FormatStyle::IAMS_Never); + CHECK_PARSE("IndentAccessModifiers: Always", IndentAccessModifiers, + FormatStyle::IAMS_Always); + CHECK_PARSE("IndentAccessModifiers: AfterFirstAccessModifier", + IndentAccessModifiers, + FormatStyle::IAMS_AfterFirstAccessModifier); + CHECK_PARSE("IndentAccessModifiers: false", IndentAccessModifiers, + FormatStyle::IAMS_Never); + CHECK_PARSE("IndentAccessModifiers: true", IndentAccessModifiers, + FormatStyle::IAMS_Always); + Style.IndentExternBlock = FormatStyle::IEBS_NoIndent; CHECK_PARSE("IndentExternBlock: AfterExternBlock", IndentExternBlock, FormatStyle::IEBS_AfterExternBlock); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 0a5950c25c60ef..e9cf68156e63e3 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24837,7 +24837,7 @@ TEST_F(FormatTest, StatementAttributeLikeMacros) { TEST_F(FormatTest, IndentAccessModifiers) { FormatStyle Style = getLLVMStyle(); - Style.IndentAccessModifiers = true; + Style.IndentAccessModifiers = FormatStyle::IAMS_Always; // Members are *two* levels below the record; // Style.IndentWidth == 2, thus yielding a 4 spaces wide indentation. verifyFormat("class C {\n" @@ -24918,6 +24918,106 @@ TEST_F(FormatTest, IndentAccessModifiers) { Style); } +TEST_F(FormatTest, IndentAccessModifiersAfterFirst) { + FormatStyle Style = getLLVMStyle(); + verifyFormat("struct S {\n" + " int before;\n" + "\n" + "public:\n" + " int after;\n" + "};", + Style); + + Style.IndentAccessModifiers = FormatStyle::IAMS_Always; + verifyFormat("struct S {\n" + " int before;\n" + "\n" + " public:\n" + " int after;\n" + "};", + Style); + Style.IndentAccessModifiers = FormatStyle::IAMS_AfterFirstAccessModifier; + verifyFormat("struct S {\n" + " int member;\n" + "};", + Style); + verifyFormat("struct S {\n" + " int before;\n" + "\n" + " public:\n" + " int after;\n" + "};", + Style); + verifyFormat("struct S {\n" + " int before;\n" + "\n" + " public:\n" + " int after;\n" + "\n" + " private:\n" + " int last;\n" + "};", + Style); +} + +TEST_F(FormatTest, IndentAccessModifiersAfterFirstAllman) { + FormatStyle Style = getLLVMStyle(); + Style.IndentAccessModifiers = FormatStyle::IAMS_AfterFirstAccessModifier; + Style.IndentWidth = 4; + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Never; + Style.BreakBeforeBraces = FormatStyle::BS_Allman; + + verifyFormat("class Outer\n" + "{\n" + " public:\n" + " struct Inner\n" + " {\n" + " bool first;\n" + " bool second;\n" + " };\n" + "};", + Style); + verifyFormat("struct S\n" + "{\n" + " int before;\n" + " private:\n" + " int after;\n" + "};", + Style); + verifyFormat("union U\n" + "{\n" + " int first;\n" + " class Inner\n" + " {\n" + " public:\n" + " int member;\n" + " };\n" + " int last;\n" + "};", + Style); + verifyFormat("class QtObject\n" + "{\n" + " signals:\n" + " void changed();\n" + "};", + Style); +} + +TEST_F(FormatTest, IndentAccessModifiersAfterFirstWhitesmiths) { + FormatStyle Style = getLLVMStyle(); + Style.IndentAccessModifiers = FormatStyle::IAMS_AfterFirstAccessModifier; + Style.IndentWidth = 4; + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Never; + Style.BreakBeforeBraces = FormatStyle::BS_Whitesmiths; + verifyFormat("struct S\n" + " {\n" + " int before;\n" + " public:\n" + " int after;\n" + " };", + Style); +} + TEST_F(FormatTest, LimitlessStringsAndComments) { auto Style = getLLVMStyleWithColumns(0); constexpr StringRef Code( diff --git a/clang/unittests/Format/FormatTestCSharp.cpp b/clang/unittests/Format/FormatTestCSharp.cpp index 805fe6f9bd3c2f..be98501db166f2 100644 --- a/clang/unittests/Format/FormatTestCSharp.cpp +++ b/clang/unittests/Format/FormatTestCSharp.cpp @@ -124,6 +124,17 @@ TEST_F(FormatTestCSharp, AccessModifiers) { "}"); } +TEST_F(FormatTestCSharp, IndentAccessModifiersAfterFirstUsesAlways) { + FormatStyle Style = getDefaultStyle(); + Style.IndentAccessModifiers = FormatStyle::IAMS_AfterFirstAccessModifier; + verifyFormat("class C\n" + "{\n" + " int Before;\n" + " public int After;\n" + "}", + Style); +} + TEST_F(FormatTestCSharp, NoStringLiteralBreaks) { // Breaking of interpolated strings is not implemented. auto Style = getDefaultStyle(); diff --git a/clang/unittests/Format/FormatTestJava.cpp b/clang/unittests/Format/FormatTestJava.cpp index a11fce963d8208..53168ef6a19e15 100644 --- a/clang/unittests/Format/FormatTestJava.cpp +++ b/clang/unittests/Format/FormatTestJava.cpp @@ -28,6 +28,17 @@ class FormatTestJava : public test::FormatTestBase { } }; +TEST_F(FormatTestJava, IndentAccessModifiersAfterFirstDoesNotAffectJava) { + FormatStyle Style = getDefaultStyle(); + Style.IndentWidth = 4; + Style.IndentAccessModifiers = FormatStyle::IAMS_AfterFirstAccessModifier; + verifyFormat("class C {\n" + " int before;\n" + " public int after;\n" + "}", + Style); +} + TEST_F(FormatTestJava, NoAlternativeOperatorNames) { verifyFormat("someObject.and();"); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
