Budovi updated this revision to Diff 320818.
Budovi added a comment.
Updated the tests after checking the issues were not caused by the patch and
they are reported / fixed.
Rebased the changes to the newest version.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94661/new/
https://reviews.llvm.org/D94661
Files:
clang/docs/ClangFormatStyleOptions.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,52 @@
"}",
format(Source, Style));
}
+
+TEST_F(FormatTest, IndentAccessModifiers) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AllowShortEnumsOnASingleLine = false;
+ Style.IndentAccessModifiers = true;
+ verifyFormat("class C {\n"
+ " int i;\n"
+ "};\n",
+ Style);
+ 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);
+ verifyFormat("union C {\n"
+ " int i;\n"
+ " unsigned u;\n"
+ "};\n",
+ Style);
+ verifyFormat("enum class E\n"
+ "{\n"
+ " A,\n"
+ " B\n"
+ "};\n",
+ Style);
+ verifyFormat("void foo() {\n"
+ " class C {\n"
+ " int i;\n"
+ " };\n"
+ " return;\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,31 @@
tooling::IncludeStyle IncludeStyle;
+ /// Makes an indentation level for record (``class``, ``struct``, ``union``)
+ /// access modifiers.
+ ///
+ /// When ``false``, access modifiers are indented relative to the member
+ /// indentation, respecting the ``AccessModifierOffset``.
+ /// When ``true``, access modifiers get their own indentation level. Record
+ /// members are always indented 2 levels below the record, regardless of the
+ /// access modifier presence. Value of ``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 +3181,7 @@
R.IncludeStyle.IncludeIsMainRegex &&
IncludeStyle.IncludeIsMainSourceRegex ==
R.IncludeStyle.IncludeIsMainSourceRegex &&
+ IndentAccessModifiers == R.IndentAccessModifiers &&
IndentCaseLabels == R.IndentCaseLabels &&
IndentCaseBlocks == R.IndentCaseBlocks &&
IndentGotoLabels == R.IndentGotoLabels &&
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2351,6 +2351,32 @@
``ClassImpl.hpp`` would not have the main include file put on top
before any other include.
+**IndentAccessModifiers** (``bool``)
+ Makes an indentation level for record (``class``, ``struct``, ``union``)
+ access modifiers.
+
+ When ``false``, access modifiers are indented relative to the member
+ indentation, respecting the ``AccessModifierOffset``.
+ When ``true``, access modifiers get their own indentation level. Record
+ members are always indented 2 levels below the record, regardless of the
+ access modifier presence. Value of ``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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
