Skip to content

Commit 2a42c75

Browse files
Budovimkurdej
Budovi
authored and
mkurdej
committed
[clang-format] [PR19056] Add support for access modifiers indentation
Adds support for coding styles that make a separate indentation level for access modifiers, such as Code::Blocks or QtCreator. The new option, `IndentAccessModifiers`, if enabled, forces the content inside classes, structs and unions (“records”) to be indented twice while removing a level for access modifiers. The value of `AccessModifierOffset` is disregarded in this case, aiming towards an ease of use. ====== The PR (https://bugs.llvm.org/show_bug.cgi?id=19056) had an implementation attempt by @mydeveloperday already (https://reviews.llvm.org/D60225) but I've decided to start from scratch. They differ in functionality, chosen approaches, and even the option name. The code tries to re-use the existing functionality to achieve this behavior, limiting possibility of breaking something else. Reviewed By: MyDeveloperDay, curdeius, HazardyKnusperkeks Differential Revision: https://reviews.llvm.org/D94661
1 parent 0b05908 commit 2a42c75

File tree

8 files changed

+139
-21
lines changed

8 files changed

+139
-21
lines changed

‎clang/docs/ClangFormatStyleOptions.rst

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2360,6 +2360,33 @@ the configuration (without a prefix: ``Auto``).
23602360
``ClassImpl.hpp`` would not have the main include file put on top
23612361
before any other include.
23622362

2363+
**IndentAccessModifiers** (``bool``)
2364+
Specify whether access modifiers should have their own indentation level.
2365+
2366+
When ``false``, access modifiers are indented (or outdented) relative to
2367+
the record members, respecting the ``AccessModifierOffset``. Record
2368+
members are indented one level below the record.
2369+
When ``true``, access modifiers get their own indentation level. As a
2370+
consequence, record members are indented 2 levels below the record,
2371+
regardless of the access modifier presence. Value of the
2372+
``AccessModifierOffset`` is ignored.
2373+
2374+
.. code-block:: c++
2375+
2376+
false: true:
2377+
class C { vs. class C {
2378+
class D { class D {
2379+
void bar(); void bar();
2380+
protected: protected:
2381+
D(); D();
2382+
}; };
2383+
public: public:
2384+
C(); C();
2385+
}; };
2386+
void foo() { void foo() {
2387+
return 1; return 1;
2388+
} }
2389+
23632390
**IndentCaseBlocks** (``bool``)
23642391
Indent case label blocks one level from the case label.
23652392

‎clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,9 @@ clang-format
196196
- ``BasedOnStyle: InheritParentConfig`` allows to use the ``.clang-format`` of
197197
the parent directories to overwrite only parts of it.
198198

199+
- Option ``IndentAccessModifiers`` has been added to be able to give access
200+
modifiers their own indentation level inside records.
201+
199202
libclang
200203
--------
201204

‎clang/include/clang/Format/Format.h

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2047,6 +2047,32 @@ struct FormatStyle {
20472047

20482048
tooling::IncludeStyle IncludeStyle;
20492049

2050+
/// Specify whether access modifiers should have their own indentation level.
2051+
///
2052+
/// When ``false``, access modifiers are indented (or outdented) relative to
2053+
/// the record members, respecting the ``AccessModifierOffset``. Record
2054+
/// members are indented one level below the record.
2055+
/// When ``true``, access modifiers get their own indentation level. As a
2056+
/// consequence, record members are always indented 2 levels below the record,
2057+
/// regardless of the access modifier presence. Value of the
2058+
/// ``AccessModifierOffset`` is ignored.
2059+
/// \code
2060+
/// false: true:
2061+
/// class C { vs. class C {
2062+
/// class D { class D {
2063+
/// void bar(); void bar();
2064+
/// protected: protected:
2065+
/// D(); D();
2066+
/// }; };
2067+
/// public: public:
2068+
/// C(); C();
2069+
/// }; };
2070+
/// void foo() { void foo() {
2071+
/// return 1; return 1;
2072+
/// } }
2073+
/// \endcode
2074+
bool IndentAccessModifiers;
2075+
20502076
/// Indent case labels one level from the switch statement.
20512077
///
20522078
/// When ``false``, use the same indentation level as for the switch
@@ -3161,6 +3187,7 @@ struct FormatStyle {
31613187
R.IncludeStyle.IncludeIsMainRegex &&
31623188
IncludeStyle.IncludeIsMainSourceRegex ==
31633189
R.IncludeStyle.IncludeIsMainSourceRegex &&
3190+
IndentAccessModifiers == R.IndentAccessModifiers &&
31643191
IndentCaseLabels == R.IndentCaseLabels &&
31653192
IndentCaseBlocks == R.IndentCaseBlocks &&
31663193
IndentGotoLabels == R.IndentGotoLabels &&

‎clang/lib/Format/Format.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,7 @@ template <> struct MappingTraits<FormatStyle> {
597597
IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex);
598598
IO.mapOptional("IncludeIsMainSourceRegex",
599599
Style.IncludeStyle.IncludeIsMainSourceRegex);
600+
IO.mapOptional("IndentAccessModifiers", Style.IndentAccessModifiers);
600601
IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
601602
IO.mapOptional("IndentCaseBlocks", Style.IndentCaseBlocks);
602603
IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
@@ -984,6 +985,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
984985
{".*", 1, 0, false}};
985986
LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
986987
LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
988+
LLVMStyle.IndentAccessModifiers = false;
987989
LLVMStyle.IndentCaseLabels = false;
988990
LLVMStyle.IndentCaseBlocks = false;
989991
LLVMStyle.IndentGotoLabels = true;

‎clang/lib/Format/UnwrappedLineFormatter.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,13 @@ class LevelIndentTracker {
101101
if (RootToken.isAccessSpecifier(false) ||
102102
RootToken.isObjCAccessSpecifier() ||
103103
(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
104-
RootToken.Next && RootToken.Next->is(tok::colon)))
105-
return Style.AccessModifierOffset;
104+
RootToken.Next && RootToken.Next->is(tok::colon))) {
105+
// The AccessModifierOffset may be overriden by IndentAccessModifiers,
106+
// in which case we take a negative value of the IndentWidth to simulate
107+
// the upper indent level.
108+
return Style.IndentAccessModifiers ? -Style.IndentWidth
109+
: Style.AccessModifierOffset;
110+
}
106111
return 0;
107112
}
108113

‎clang/lib/Format/UnwrappedLineParser.cpp

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ size_t UnwrappedLineParser::computePPHash() const {
579579
return h;
580580
}
581581

582-
void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
582+
void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
583583
bool MunchSemi) {
584584
assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
585585
"'{' or macro block token expected");
@@ -589,7 +589,7 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
589589
size_t PPStartHash = computePPHash();
590590

591591
unsigned InitialLevel = Line->Level;
592-
nextToken(/*LevelDifference=*/AddLevel ? 1 : 0);
592+
nextToken(/*LevelDifference=*/AddLevels);
593593

594594
if (MacroBlock && FormatTok->is(tok::l_paren))
595595
parseParens();
@@ -604,8 +604,7 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
604604

605605
ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
606606
MustBeDeclaration);
607-
if (AddLevel)
608-
++Line->Level;
607+
Line->Level += AddLevels;
609608
parseLevel(/*HasOpeningBrace=*/true);
610609

611610
if (eof())
@@ -621,7 +620,7 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
621620
size_t PPEndHash = computePPHash();
622621

623622
// Munch the closing brace.
624-
nextToken(/*LevelDifference=*/AddLevel ? -1 : 0);
623+
nextToken(/*LevelDifference=*/-AddLevels);
625624

626625
if (MacroBlock && FormatTok->is(tok::l_paren))
627626
parseParens();
@@ -1125,12 +1124,12 @@ void UnwrappedLineParser::parseStructuralElement() {
11251124
if (Style.BraceWrapping.AfterExternBlock) {
11261125
addUnwrappedLine();
11271126
}
1128-
parseBlock(/*MustBeDeclaration=*/true,
1129-
/*AddLevel=*/Style.BraceWrapping.AfterExternBlock);
1127+
unsigned AddLevels = Style.BraceWrapping.AfterExternBlock ? 1u : 0u;
1128+
parseBlock(/*MustBeDeclaration=*/true, AddLevels);
11301129
} else {
1131-
parseBlock(/*MustBeDeclaration=*/true,
1132-
/*AddLevel=*/Style.IndentExternBlock ==
1133-
FormatStyle::IEBS_Indent);
1130+
unsigned AddLevels =
1131+
Style.IndentExternBlock == FormatStyle::IEBS_Indent ? 1u : 0u;
1132+
parseBlock(/*MustBeDeclaration=*/true, AddLevels);
11341133
}
11351134
addUnwrappedLine();
11361135
return;
@@ -1159,7 +1158,7 @@ void UnwrappedLineParser::parseStructuralElement() {
11591158
return;
11601159
}
11611160
if (FormatTok->is(TT_MacroBlockBegin)) {
1162-
parseBlock(/*MustBeDeclaration=*/false, /*AddLevel=*/true,
1161+
parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
11631162
/*MunchSemi=*/false);
11641163
return;
11651164
}
@@ -2128,10 +2127,13 @@ void UnwrappedLineParser::parseNamespace() {
21282127
if (ShouldBreakBeforeBrace(Style, InitialToken))
21292128
addUnwrappedLine();
21302129

2131-
bool AddLevel = Style.NamespaceIndentation == FormatStyle::NI_All ||
2132-
(Style.NamespaceIndentation == FormatStyle::NI_Inner &&
2133-
DeclarationScopeStack.size() > 1);
2134-
parseBlock(/*MustBeDeclaration=*/true, AddLevel);
2130+
unsigned AddLevels =
2131+
Style.NamespaceIndentation == FormatStyle::NI_All ||
2132+
(Style.NamespaceIndentation == FormatStyle::NI_Inner &&
2133+
DeclarationScopeStack.size() > 1)
2134+
? 1u
2135+
: 0u;
2136+
parseBlock(/*MustBeDeclaration=*/true, AddLevels);
21352137
// Munch the semicolon after a namespace. This is more common than one would
21362138
// think. Putting the semicolon into its own line is very ugly.
21372139
if (FormatTok->Tok.is(tok::semi))
@@ -2577,7 +2579,7 @@ void UnwrappedLineParser::parseJavaEnumBody() {
25772579
while (FormatTok) {
25782580
if (FormatTok->is(tok::l_brace)) {
25792581
// Parse the constant's class body.
2580-
parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
2582+
parseBlock(/*MustBeDeclaration=*/true, /*AddLevels=*/1u,
25812583
/*MunchSemi=*/false);
25822584
} else if (FormatTok->is(tok::l_paren)) {
25832585
parseParens();
@@ -2679,8 +2681,8 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
26792681
if (ShouldBreakBeforeBrace(Style, InitialToken))
26802682
addUnwrappedLine();
26812683

2682-
parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
2683-
/*MunchSemi=*/false);
2684+
unsigned AddLevels = Style.IndentAccessModifiers ? 2u : 1u;
2685+
parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/false);
26842686
}
26852687
}
26862688
// There is no addUnwrappedLine() here so that we fall through to parsing a

‎clang/lib/Format/UnwrappedLineParser.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class UnwrappedLineParser {
8585
void reset();
8686
void parseFile();
8787
void parseLevel(bool HasOpeningBrace);
88-
void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
88+
void parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1u,
8989
bool MunchSemi = true);
9090
void parseChildBlock();
9191
void parsePPDirective();

‎clang/unittests/Format/FormatTest.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15453,6 +15453,7 @@ TEST_F(FormatTest, ParsesConfigurationBools) {
1545315453
CHECK_PARSE_BOOL(DerivePointerAlignment);
1545415454
CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
1545515455
CHECK_PARSE_BOOL(DisableFormat);
15456+
CHECK_PARSE_BOOL(IndentAccessModifiers);
1545615457
CHECK_PARSE_BOOL(IndentCaseLabels);
1545715458
CHECK_PARSE_BOOL(IndentCaseBlocks);
1545815459
CHECK_PARSE_BOOL(IndentGotoLabels);
@@ -19155,6 +19156,57 @@ TEST_F(FormatTest, StatementAttributeLikeMacros) {
1915519156
"}",
1915619157
format(Source, Style));
1915719158
}
19159+
19160+
TEST_F(FormatTest, IndentAccessModifiers) {
19161+
FormatStyle Style = getLLVMStyle();
19162+
Style.IndentAccessModifiers = true;
19163+
// Members are *two* levels below the record;
19164+
// Style.IndentWidth == 2, thus yielding a 4 spaces wide indentation.
19165+
verifyFormat("class C {\n"
19166+
" int i;\n"
19167+
"};\n",
19168+
Style);
19169+
verifyFormat("union C {\n"
19170+
" int i;\n"
19171+
" unsigned u;\n"
19172+
"};\n",
19173+
Style);
19174+
// Access modifiers should be indented one level below the record.
19175+
verifyFormat("class C {\n"
19176+
" public:\n"
19177+
" int i;\n"
19178+
"};\n",
19179+
Style);
19180+
verifyFormat("struct S {\n"
19181+
" private:\n"
19182+
" class C {\n"
19183+
" int j;\n"
19184+
"\n"
19185+
" public:\n"
19186+
" C();\n"
19187+
" };\n"
19188+
"\n"
19189+
" public:\n"
19190+
" int i;\n"
19191+
"};\n",
19192+
Style);
19193+
// Enumerations are not records and should be unaffected.
19194+
Style.AllowShortEnumsOnASingleLine = false;
19195+
verifyFormat("enum class E\n"
19196+
"{\n"
19197+
" A,\n"
19198+
" B\n"
19199+
"};\n",
19200+
Style);
19201+
// Test with a different indentation width;
19202+
// also proves that the result is Style.AccessModifierOffset agnostic.
19203+
Style.IndentWidth = 3;
19204+
verifyFormat("class C {\n"
19205+
" public:\n"
19206+
" int i;\n"
19207+
"};\n",
19208+
Style);
19209+
}
1915819210
} // namespace
1915919211
} // namespace format
1916019212
} // namespace clang

0 commit comments

Comments
 (0)