[clang] 2a42c75 - [clang-format] [PR19056] Add support for access modifiers indentation
Marek Kurdej via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 26 00:17:16 PST 2021
Author: Jakub Budiský
Date: 2021-02-26T09:17:07+01:00
New Revision: 2a42c759ae7bb09dd448d188138f310d014fcab6
URL: https://github.com/llvm/llvm-project/commit/2a42c759ae7bb09dd448d188138f310d014fcab6
DIFF: https://github.com/llvm/llvm-project/commit/2a42c759ae7bb09dd448d188138f310d014fcab6.diff
LOG: [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
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 7ae8ea913099..1dbf11987cd8 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -2360,6 +2360,33 @@ the configuration (without a prefix: ``Auto``).
``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.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 26b380407c0b..6a0ebd9ab3ee 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -196,6 +196,9 @@ clang-format
- ``BasedOnStyle: InheritParentConfig`` allows to use the ``.clang-format`` of
the parent directories to overwrite only parts of it.
+- Option ``IndentAccessModifiers`` has been added to be able to give access
+ modifiers their own indentation level inside records.
+
libclang
--------
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 1a669ebf07f5..d60c56058a85 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2047,6 +2047,32 @@ struct FormatStyle {
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
@@ -3161,6 +3187,7 @@ struct FormatStyle {
R.IncludeStyle.IncludeIsMainRegex &&
IncludeStyle.IncludeIsMainSourceRegex ==
R.IncludeStyle.IncludeIsMainSourceRegex &&
+ IndentAccessModifiers == R.IndentAccessModifiers &&
IndentCaseLabels == R.IndentCaseLabels &&
IndentCaseBlocks == R.IndentCaseBlocks &&
IndentGotoLabels == R.IndentGotoLabels &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index a8808b389f84..f2fc098c2b1a 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -597,6 +597,7 @@ template <> struct MappingTraits<FormatStyle> {
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);
@@ -984,6 +985,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
{".*", 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;
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 5dd0ccdfa6fd..0cdf20f21315 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -101,8 +101,13 @@ class LevelIndentTracker {
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;
}
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index f689a6361a3a..1e70da852b2d 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -579,7 +579,7 @@ size_t UnwrappedLineParser::computePPHash() const {
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 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
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 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
MustBeDeclaration);
- if (AddLevel)
- ++Line->Level;
+ Line->Level += AddLevels;
parseLevel(/*HasOpeningBrace=*/true);
if (eof())
@@ -621,7 +620,7 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
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 @@ void UnwrappedLineParser::parseStructuralElement() {
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 @@ void UnwrappedLineParser::parseStructuralElement() {
return;
}
if (FormatTok->is(TT_MacroBlockBegin)) {
- parseBlock(/*MustBeDeclaration=*/false, /*AddLevel=*/true,
+ parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
/*MunchSemi=*/false);
return;
}
@@ -2128,10 +2127,13 @@ void UnwrappedLineParser::parseNamespace() {
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 @@ void UnwrappedLineParser::parseJavaEnumBody() {
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 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
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
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 02b328cb72de..fde9e07dfb04 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -85,7 +85,7 @@ class UnwrappedLineParser {
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();
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index ccda0a87bef3..b4b31fa70c10 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -15453,6 +15453,7 @@ TEST_F(FormatTest, ParsesConfigurationBools) {
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);
@@ -19155,6 +19156,57 @@ TEST_F(FormatTest, StatementAttributeLikeMacros) {
"}",
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
diff erent indentation width;
+ // also proves that the result is Style.AccessModifierOffset agnostic.
+ Style.IndentWidth = 3;
+ verifyFormat("class C {\n"
+ " public:\n"
+ " int i;\n"
+ "};\n",
+ Style);
+}
} // namespace
} // namespace format
} // namespace clang
More information about the cfe-commits
mailing list