https://github.com/noambouillet created https://github.com/llvm/llvm-project/pull/227704
## Summary With `IndentAccessModifiers: true`, clang-format currently indents record members two levels even when the record has no explicit access label. Add `IndentImplicitAccessModifiers`, defaulting to `true`, to preserve that output. Setting it to `false` indents members before the first explicit `public:`, `protected:`, or `private:` by one level. Members following the first label retain the existing two-level indentation. Each record tracks its own access labels, so a label in a nested record does not change its parent. Qt access labels follow the same rule. The new behavior is limited to C-family parsing; Java formatting remains unchanged. ```yaml IndentAccessModifiers: true IndentImplicitAccessModifiers: false ``` This makes the layout in #61631 possible without changing existing configurations. It also addresses the related requests in #54333 and #182566. ## Tests - Built `clang-format` and `FormatTests` from LLVM main. - Ran all 1,295 `FormatTests`; all passed. - Confirmed the new setting formats the #61631 reproduction exactly as requested and a second pass makes no changes. ## 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. >From f55190042b1ba98c2946eb94e468cc4899718a38 Mon Sep 17 00:00:00 2001 From: Noam Bouillet <[email protected]> Date: Wed, 30 Sep 2026 14:07:44 +0200 Subject: [PATCH] feat(format): make implicit access indent optional Preserve existing output by default while allowing records without an explicit access label to use one indentation level. Refs #61631 --- clang/docs/ClangFormatStyleOptions.md | 17 +++++-- clang/docs/ReleaseNotes.md | 4 ++ clang/include/clang/Format/Format.h | 16 +++++-- clang/lib/Format/Format.cpp | 3 ++ clang/lib/Format/UnwrappedLineParser.cpp | 47 ++++++++++++++----- clang/lib/Format/UnwrappedLineParser.h | 6 ++- clang/unittests/Format/ConfigParseTest.cpp | 1 + clang/unittests/Format/FormatTest.cpp | 53 ++++++++++++++++++++++ clang/unittests/Format/FormatTestJava.cpp | 12 +++++ 9 files changed, 140 insertions(+), 19 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.md b/clang/docs/ClangFormatStyleOptions.md index 81984ff185e53..d831bcac07c9c 100644 --- a/clang/docs/ClangFormatStyleOptions.md +++ b/clang/docs/ClangFormatStyleOptions.md @@ -4752,9 +4752,10 @@ the configuration (without a prefix: `Auto`). 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. + consequence, record members are by default indented 2 levels below the + record, regardless of the access modifier presence. Value of the + `AccessModifierOffset` is ignored. `IndentImplicitAccessModifiers` can + change the indentation before the first explicit access modifier. ```c++ false: true: @@ -4952,6 +4953,16 @@ the configuration (without a prefix: `Auto`). +(indentimplicitaccessmodifiers)= + +**IndentImplicitAccessModifiers** (`Boolean`) {versionbadge}`clang-format 24` {ref}`¶ <IndentImplicitAccessModifiers>` + +: When `IndentAccessModifiers` is `true`, indent members before the first + explicit access modifier by two levels. Set this option to `false` to + indent those members by one level. Members after an explicit access + modifier still use two levels. This option has no effect if + `IndentAccessModifiers` is false. + (indentppdirectives)= **IndentPPDirectives** (`PPDirectiveIndentStyle`) {versionbadge}`clang-format 6` {ref}`¶ <IndentPPDirectives>` diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index c778703e8cc6f..a2090bb98df30 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -944,6 +944,10 @@ features cannot lower the translation-unit ABI level; ### clang-format +- Add `IndentImplicitAccessModifiers` to allow members before the first + explicit access modifier to use one indentation level when + `IndentAccessModifiers` is enabled. The default preserves existing formatting. + - Add `SpacesInBlockComments` option to control spacing after `/*` and before `*/` in ordinary block comments. - Add `AfterRequiresExpression` sub-option of `BraceWrapping` to wrap the diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 6d4fa6e8ee2a7..40ae52a0148e8 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -3187,9 +3187,10 @@ struct FormatStyle { /// 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. + /// consequence, record members are by default indented 2 levels below the + /// record, regardless of the access modifier presence. Value of the + /// `AccessModifierOffset` is ignored. `IndentImplicitAccessModifiers` can + /// change the indentation before the first explicit access modifier. /// \code /// false: true: /// class C { vs. class C { @@ -3208,6 +3209,14 @@ struct FormatStyle { /// \version 13 bool IndentAccessModifiers; + /// When `IndentAccessModifiers` is `true`, indent members before the first + /// explicit access modifier by two levels. Set this option to `false` to + /// indent those members by one level. Members after an explicit access + /// modifier still use two levels. This option has no effect if + /// `IndentAccessModifiers` is false. + /// \version 24 + bool IndentImplicitAccessModifiers; + /// Indent case label blocks one level from the case label. /// /// When `false`, the block following the case label uses the same @@ -6242,6 +6251,7 @@ struct FormatStyle { R.IncludeStyle.IncludeIsMainSourceRegex && IncludeStyle.MainIncludeChar == R.IncludeStyle.MainIncludeChar && IndentAccessModifiers == R.IndentAccessModifiers && + IndentImplicitAccessModifiers == R.IndentImplicitAccessModifiers && IndentCaseBlocks == R.IndentCaseBlocks && IndentCaseLabels == R.IndentCaseLabels && IndentExportBlock == R.IndentExportBlock && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 4c78c1dbe9f80..2f6d70d2bf4e2 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1399,6 +1399,8 @@ template <> struct MappingTraits<FormatStyle> { IO.mapOptional("IncludeIsMainSourceRegex", Style.IncludeStyle.IncludeIsMainSourceRegex); IO.mapOptional("IndentAccessModifiers", Style.IndentAccessModifiers); + IO.mapOptional("IndentImplicitAccessModifiers", + Style.IndentImplicitAccessModifiers); IO.mapOptional("IndentCaseBlocks", Style.IndentCaseBlocks); IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels); IO.mapOptional("IndentExportBlock", Style.IndentExportBlock); @@ -1977,6 +1979,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$"; LLVMStyle.IncludeStyle.MainIncludeChar = tooling::IncludeStyle::MICD_Quote; LLVMStyle.IndentAccessModifiers = false; + LLVMStyle.IndentImplicitAccessModifiers = true; LLVMStyle.IndentCaseBlocks = false; LLVMStyle.IndentCaseLabels = false; LLVMStyle.IndentExportBlock = true; diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 4825e825af1fa..0e0825112b513 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] { + const bool IsQtAccessLabel = + SeenExplicitAccessModifier && !*SeenExplicitAccessModifier && + FormatTok->isOneOf(Keywords.kw_signals, Keywords.kw_qsignals, + Keywords.kw_slots, Keywords.kw_qslots) && + Tokens->peekNextToken(/*SkipComment=*/true)->is(tok::colon); + if (SeenExplicitAccessModifier && !*SeenExplicitAccessModifier && + (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; @@ -879,7 +894,8 @@ FormatToken *UnwrappedLineParser::parseBlock(bool MustBeDeclaration, size_t PPEndHash = computePPHash(); // Munch the closing brace. - nextToken(/*LevelDifference=*/-AddLevels); + nextToken(/*LevelDifference=*/ + -static_cast<int>(AddLevels + SeenExplicitAccessModifier)); // When this is a function block and there is an unnecessary semicolon // afterwards then mark it as optional (so the RemoveSemi pass can get rid of @@ -4298,8 +4314,17 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr, bool IsJavaRecord) { addUnwrappedLine(); } - unsigned AddLevels = Style.IndentAccessModifiers ? 2u : 1u; - parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/false); + const bool IndentAfterExplicitAccessModifier = + Style.isCpp() && Style.IndentAccessModifiers && + !Style.IndentImplicitAccessModifiers; + unsigned AddLevels = + Style.IndentAccessModifiers && !IndentAfterExplicitAccessModifier + ? 2u + : 1u; + 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 5b93c8f346d75..2e1755ad3b2c6 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 86511edb9d40d..ccbc346901460 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -190,6 +190,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding"); CHECK_PARSE_BOOL(DisableFormat); CHECK_PARSE_BOOL(IndentAccessModifiers); + CHECK_PARSE_BOOL(IndentImplicitAccessModifiers); CHECK_PARSE_BOOL(IndentCaseBlocks); CHECK_PARSE_BOOL(IndentCaseLabels); CHECK_PARSE_BOOL(IndentExportBlock); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index bb630da34d7d9..656a352ecb005 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -24920,6 +24920,59 @@ TEST_F(FormatTest, IndentAccessModifiers) { Style); } +TEST_F(FormatTest, IndentImplicitAccessModifiers) { + FormatStyle Style = getLLVMStyle(); + Style.IndentAccessModifiers = true; + Style.IndentImplicitAccessModifiers = false; + 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); + + 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/FormatTestJava.cpp b/clang/unittests/Format/FormatTestJava.cpp index a11fce963d820..86e4b2fc361b2 100644 --- a/clang/unittests/Format/FormatTestJava.cpp +++ b/clang/unittests/Format/FormatTestJava.cpp @@ -28,6 +28,18 @@ class FormatTestJava : public test::FormatTestBase { } }; +TEST_F(FormatTestJava, IndentImplicitAccessModifiersDoesNotAffectJava) { + FormatStyle Style = getDefaultStyle(); + Style.IndentWidth = 4; + Style.IndentAccessModifiers = true; + Style.IndentImplicitAccessModifiers = false; + 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
