Budovi updated this revision to Diff 321724.
Budovi added a comment.
Changes:
- Added a unit test with a different `IndentWidth`
- Removed a unit test that contained a class nested in function; didn't add
much IMHO
- Added brief comments about potentially non-obvious behaviour to some tests
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94661/new/
https://reviews.llvm.org/D94661
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
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/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15453,6 +15453,7 @@
CHECK_PARSE_BOOL(DerivePointerAlignment);
CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
CHECK_PARSE_BOOL(DisableFormat);
+ CHECK_PARSE_BOOL(IndentAccessModifiers);
CHECK_PARSE_BOOL(IndentCaseLabels);
CHECK_PARSE_BOOL(IndentCaseBlocks);
CHECK_PARSE_BOOL(IndentGotoLabels);
@@ -19050,6 +19051,57 @@
"}",
format(Source, Style));
}
+
+TEST_F(FormatTest, IndentAccessModifiers) {
+ FormatStyle Style = getLLVMStyle();
+ Style.IndentAccessModifiers = true;
+ // Members are *two* levels below the record;
+ // Style.IndentWidth == 2, thus yielding a 4 spaces wide indentation
+ verifyFormat("class C {\n"
+ " int i;\n"
+ "};\n",
+ Style);
+ verifyFormat("union C {\n"
+ " int i;\n"
+ " unsigned u;\n"
+ "};\n",
+ Style);
+ // Access modifiers should be indented one level below the record
+ verifyFormat("class C {\n"
+ " public:\n"
+ " int i;\n"
+ "};\n",
+ Style);
+ verifyFormat("struct S {\n"
+ " private:\n"
+ " class C {\n"
+ " int j;\n"
+ "\n"
+ " public:\n"
+ " C();\n"
+ " };\n"
+ "\n"
+ " public:\n"
+ " int i;\n"
+ "};\n",
+ Style);
+ // Enumerations are not records and should be unaffected
+ Style.AllowShortEnumsOnASingleLine = false;
+ verifyFormat("enum class E\n"
+ "{\n"
+ " A,\n"
+ " B\n"
+ "};\n",
+ Style);
+ // Test with a different indentation width;
+ // also proves that the result is Style.AccessModifierOffset agonistic
+ Style.IndentWidth = 3;
+ verifyFormat("class C {\n"
+ " public:\n"
+ " int i;\n"
+ "};\n",
+ Style);
+}
} // namespace
} // namespace format
} // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.h
===================================================================
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -85,7 +85,7 @@
void reset();
void parseFile();
void parseLevel(bool HasOpeningBrace);
- void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
+ void parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1u,
bool MunchSemi = true);
void parseChildBlock();
void parsePPDirective();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -579,7 +579,7 @@
return h;
}
-void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
+void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
bool MunchSemi) {
assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
"'{' or macro block token expected");
@@ -589,7 +589,7 @@
size_t PPStartHash = computePPHash();
unsigned InitialLevel = Line->Level;
- nextToken(/*LevelDifference=*/AddLevel ? 1 : 0);
+ nextToken(/*LevelDifference=*/AddLevels);
if (MacroBlock && FormatTok->is(tok::l_paren))
parseParens();
@@ -604,8 +604,7 @@
ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
MustBeDeclaration);
- if (AddLevel)
- ++Line->Level;
+ Line->Level += AddLevels;
parseLevel(/*HasOpeningBrace=*/true);
if (eof())
@@ -621,7 +620,7 @@
size_t PPEndHash = computePPHash();
// Munch the closing brace.
- nextToken(/*LevelDifference=*/AddLevel ? -1 : 0);
+ nextToken(/*LevelDifference=*/-AddLevels);
if (MacroBlock && FormatTok->is(tok::l_paren))
parseParens();
@@ -1125,12 +1124,12 @@
if (Style.BraceWrapping.AfterExternBlock) {
addUnwrappedLine();
}
- parseBlock(/*MustBeDeclaration=*/true,
- /*AddLevel=*/Style.BraceWrapping.AfterExternBlock);
+ unsigned AddLevels = Style.BraceWrapping.AfterExternBlock ? 1u : 0u;
+ parseBlock(/*MustBeDeclaration=*/true, AddLevels);
} else {
- parseBlock(/*MustBeDeclaration=*/true,
- /*AddLevel=*/Style.IndentExternBlock ==
- FormatStyle::IEBS_Indent);
+ unsigned AddLevels =
+ Style.IndentExternBlock == FormatStyle::IEBS_Indent ? 1u : 0u;
+ parseBlock(/*MustBeDeclaration=*/true, AddLevels);
}
addUnwrappedLine();
return;
@@ -1159,7 +1158,7 @@
return;
}
if (FormatTok->is(TT_MacroBlockBegin)) {
- parseBlock(/*MustBeDeclaration=*/false, /*AddLevel=*/true,
+ parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
/*MunchSemi=*/false);
return;
}
@@ -2128,10 +2127,13 @@
if (ShouldBreakBeforeBrace(Style, InitialToken))
addUnwrappedLine();
- bool AddLevel = Style.NamespaceIndentation == FormatStyle::NI_All ||
- (Style.NamespaceIndentation == FormatStyle::NI_Inner &&
- DeclarationScopeStack.size() > 1);
- parseBlock(/*MustBeDeclaration=*/true, AddLevel);
+ unsigned AddLevels =
+ Style.NamespaceIndentation == FormatStyle::NI_All ||
+ (Style.NamespaceIndentation == FormatStyle::NI_Inner &&
+ DeclarationScopeStack.size() > 1)
+ ? 1u
+ : 0u;
+ parseBlock(/*MustBeDeclaration=*/true, AddLevels);
// Munch the semicolon after a namespace. This is more common than one would
// think. Putting the semicolon into its own line is very ugly.
if (FormatTok->Tok.is(tok::semi))
@@ -2577,7 +2579,7 @@
while (FormatTok) {
if (FormatTok->is(tok::l_brace)) {
// Parse the constant's class body.
- parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
+ parseBlock(/*MustBeDeclaration=*/true, /*AddLevels=*/1u,
/*MunchSemi=*/false);
} else if (FormatTok->is(tok::l_paren)) {
parseParens();
@@ -2679,8 +2681,8 @@
if (ShouldBreakBeforeBrace(Style, InitialToken))
addUnwrappedLine();
- parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
- /*MunchSemi=*/false);
+ unsigned AddLevels = Style.IndentAccessModifiers ? 2u : 1u;
+ parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/false);
}
}
// There is no addUnwrappedLine() here so that we fall through to parsing a
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -101,8 +101,13 @@
if (RootToken.isAccessSpecifier(false) ||
RootToken.isObjCAccessSpecifier() ||
(RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon)))
- return Style.AccessModifierOffset;
+ RootToken.Next && RootToken.Next->is(tok::colon))) {
+ // The AccessModifierOffset may be overriden 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 0;
}
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -597,6 +597,7 @@
IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex);
IO.mapOptional("IncludeIsMainSourceRegex",
Style.IncludeStyle.IncludeIsMainSourceRegex);
+ IO.mapOptional("IndentAccessModifiers", Style.IndentAccessModifiers);
IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
IO.mapOptional("IndentCaseBlocks", Style.IndentCaseBlocks);
IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
@@ -983,6 +984,7 @@
{".*", 1, 0, false}};
LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
+ LLVMStyle.IndentAccessModifiers = false;
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.IndentCaseBlocks = false;
LLVMStyle.IndentGotoLabels = true;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2042,6 +2042,32 @@
tooling::IncludeStyle IncludeStyle;
+ /// 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
+ bool IndentAccessModifiers;
+
/// Indent case labels one level from the switch statement.
///
/// When ``false``, use the same indentation level as for the switch
@@ -3156,6 +3182,7 @@
R.IncludeStyle.IncludeIsMainRegex &&
IncludeStyle.IncludeIsMainSourceRegex ==
R.IncludeStyle.IncludeIsMainSourceRegex &&
+ IndentAccessModifiers == R.IndentAccessModifiers &&
IndentCaseLabels == R.IndentCaseLabels &&
IndentCaseBlocks == R.IndentCaseBlocks &&
IndentGotoLabels == R.IndentGotoLabels &&
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -189,6 +189,9 @@
#include "B/A.h"
#include "B/a.h"
+- Option ``IndentAccessModifiers`` has been added to be able to give access
+ modifiers their own indentation level inside records.
+
libclang
--------
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2351,6 +2351,33 @@
``ClassImpl.hpp`` would not have the main include file put on top
before any other include.
+**IndentAccessModifiers** (``bool``)
+ 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 indented 2 levels below the record,
+ regardless of the access modifier presence. Value of the
+ ``AccessModifierOffset`` is ignored.
+
+ .. code-block:: 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** (``bool``)
Indent case label blocks one level from the case label.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
