[clang] 6ca5281 - [clang-format][PR47290] Add ShortNamespaceLines format option
Björn Schäpers via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 1 12:28:51 PST 2021
Author: Krystian Kuzniarek
Date: 2021-03-01T21:28:14+01:00
New Revision: 6ca52815fb3cb32343b9a008fdf9f593da39f203
URL: https://github.com/llvm/llvm-project/commit/6ca52815fb3cb32343b9a008fdf9f593da39f203
DIFF: https://github.com/llvm/llvm-project/commit/6ca52815fb3cb32343b9a008fdf9f593da39f203.diff
LOG: [clang-format][PR47290] Add ShortNamespaceLines format option
clang-format documentation states that having enabled
FixNamespaceComments one may expect below code:
c++
namespace a {
foo();
}
to be turned into:
c++
namespace a {
foo();
} // namespace a
In reality, no "// namespace a" was added. The problem was too high
value of kShortNamespaceMaxLines, which is used while deciding whether
a namespace is long enough to be formatted.
As with 9163fe2, clang-format idempotence is preserved.
Differential Revision: https://reviews.llvm.org/D87587
Added:
Modified:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/NamespaceEndCommentsFixer.cpp
clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
Removed:
################################################################################
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 1dbf11987cd8..a0af61faa143 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -2210,14 +2210,16 @@ the configuration (without a prefix: ``Auto``).
not use this in config files, etc. Use at your own risk.
**FixNamespaceComments** (``bool``)
- If ``true``, clang-format adds missing namespace end comments and
- fixes invalid existing ones.
+ If ``true``, clang-format adds missing namespace end comments for
+ short namespaces and fixes invalid existing ones. Short ones are
+ controlled by "ShortNamespaceLines".
.. code-block:: c++
true: false:
namespace a { vs. namespace a {
foo(); foo();
+ bar(); bar();
} // namespace a }
**ForEachMacros** (``std::vector<std::string>``)
@@ -2367,7 +2369,7 @@ 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 indented 2 levels below the record,
+ consequence, record members are always indented 2 levels below the record,
regardless of the access modifier presence. Value of the
``AccessModifierOffset`` is ignored.
@@ -3040,43 +3042,72 @@ the configuration (without a prefix: ``Auto``).
/* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
* information */
+**ShortNamespaceLines** (``unsigned``)
+ The maximal number of unwrapped lines that a short namespace spans.
+ Defaults to 1.
+
+ This determines the maximum length of short namespaces by counting
+ unwrapped lines (i.e. containing neither opening nor closing
+ namespace brace) and makes "FixNamespaceComments" omit adding
+ end comments for those.
+
+ .. code-block:: c++
+
+ ShortNamespaceLines: 1 vs. ShortNamespaceLines: 0
+ namespace a { namespace a {
+ int foo; int foo;
+ } } // namespace a
+
+ ShortNamespaceLines: 1 vs. ShortNamespaceLines: 0
+ namespace b { namespace b {
+ int foo; int foo;
+ int bar; int bar;
+ } // namespace b } // namespace b
+
**SortIncludes** (``SortIncludesOptions``)
Controls if and how clang-format will sort ``#includes``.
+ If ``Never``, includes are never sorted.
+ If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
+ insensitive fashion.
+ If ``CaseSensitive``, includes are sorted in an alphabetical or case
+ sensitive fashion.
- Possible Values:
+ Possible values:
- * ``SI_Never`` (in configuration ``Never``)
+ * ``SI_Never`` (in configuration: ``Never``)
Includes are never sorted.
.. code-block:: c++
- #include "B/A.h"
- #include "A/B.h"
- #include "a/b.h"
- #include "A/b.h"
- #include "B/a.h"
+ #include "B/A.h"
+ #include "A/B.h"
+ #include "a/b.h"
+ #include "A/b.h"
+ #include "B/a.h"
- * ``SI_CaseInsensitive`` (in configuration ``CaseInsensitive``)
+ * ``SI_CaseInsensitive`` (in configuration: ``CaseInsensitive``)
Includes are sorted in an ASCIIbetical or case insensitive fashion.
.. code-block:: c++
- #include "A/B.h"
- #include "A/b.h"
- #include "B/A.h"
- #include "B/a.h"
- #include "a/b.h"
+ #include "A/B.h"
+ #include "A/b.h"
+ #include "B/A.h"
+ #include "B/a.h"
+ #include "a/b.h"
- * ``SI_CaseSensitive`` (in configuration ``CaseSensitive``)
+ * ``SI_CaseSensitive`` (in configuration: ``CaseSensitive``)
Includes are sorted in an alphabetical or case sensitive fashion.
.. code-block:: c++
- #include "A/B.h"
- #include "A/b.h"
- #include "a/b.h"
- #include "B/A.h"
- #include "B/a.h"
+ #include "A/B.h"
+ #include "A/b.h"
+ #include "a/b.h"
+ #include "B/A.h"
+ #include "B/a.h"
+
+
**SortJavaStaticImport** (``SortJavaStaticImportOptions``)
When sorting Java imports, by default static imports are placed before
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9db9a0edc391..89592606c88c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -203,6 +203,9 @@ clang-format
- Option ``IndentAccessModifiers`` has been added to be able to give access
modifiers their own indentation level inside records.
+- Option ``ShortNamespaceLines`` has been added to give better control
+ over ``FixNamespaceComments`` when determining a namespace length.
+
libclang
--------
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index d60c56058a85..2bb2e670cb14 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1964,12 +1964,14 @@ struct FormatStyle {
/// not use this in config files, etc. Use at your own risk.
bool ExperimentalAutoDetectBinPacking;
- /// If ``true``, clang-format adds missing namespace end comments and
- /// fixes invalid existing ones.
+ /// If ``true``, clang-format adds missing namespace end comments for
+ /// short namespaces and fixes invalid existing ones. Short ones are
+ /// controlled by "ShortNamespaceLines".
/// \code
/// true: false:
/// namespace a { vs. namespace a {
/// foo(); foo();
+ /// bar(); bar();
/// } // namespace a }
/// \endcode
bool FixNamespaceComments;
@@ -2644,6 +2646,27 @@ struct FormatStyle {
bool ReflowComments;
// clang-format on
+ /// The maximal number of unwrapped lines that a short namespace spans.
+ /// Defaults to 1.
+ ///
+ /// This determines the maximum length of short namespaces by counting
+ /// unwrapped lines (i.e. containing neither opening nor closing
+ /// namespace brace) and makes "FixNamespaceComments" omit adding
+ /// end comments for those.
+ /// \code
+ /// ShortNamespaceLines: 1 vs. ShortNamespaceLines: 0
+ /// namespace a { namespace a {
+ /// int foo; int foo;
+ /// } } // namespace a
+ ///
+ /// ShortNamespaceLines: 1 vs. ShortNamespaceLines: 0
+ /// namespace b { namespace b {
+ /// int foo; int foo;
+ /// int bar; int bar;
+ /// } // namespace b } // namespace b
+ /// \endcode
+ unsigned ShortNamespaceLines;
+
/// Include sorting options.
enum SortIncludesOptions : unsigned char {
/// Includes are never sorted.
@@ -3224,6 +3247,7 @@ struct FormatStyle {
R.PenaltyBreakTemplateDeclaration &&
PointerAlignment == R.PointerAlignment &&
RawStringFormats == R.RawStringFormats &&
+ ShortNamespaceLines == R.ShortNamespaceLines &&
SortIncludes == R.SortIncludes &&
SortJavaStaticImport == R.SortJavaStaticImport &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index f2fc098c2b1a..fd8080fdd2fa 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -642,6 +642,7 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("PointerAlignment", Style.PointerAlignment);
IO.mapOptional("RawStringFormats", Style.RawStringFormats);
IO.mapOptional("ReflowComments", Style.ReflowComments);
+ IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
IO.mapOptional("SortIncludes", Style.SortIncludes);
IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
@@ -1006,6 +1007,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.ObjCSpaceAfterProperty = false;
LLVMStyle.ObjCSpaceBeforeProtocolList = true;
LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
+ LLVMStyle.ShortNamespaceLines = 1;
LLVMStyle.SpacesBeforeTrailingComments = 1;
LLVMStyle.Standard = FormatStyle::LS_Latest;
LLVMStyle.UseCRLF = false;
diff --git a/clang/lib/Format/NamespaceEndCommentsFixer.cpp b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
index c5bb6e9eab5d..def551f863cd 100644
--- a/clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -22,10 +22,6 @@ namespace clang {
namespace format {
namespace {
-// The maximal number of unwrapped lines that a short namespace spans.
-// Short namespaces don't need an end comment.
-static const int kShortNamespaceMaxLines = 1;
-
// Computes the name of a namespace given the namespace token.
// Returns "" for anonymous namespace.
std::string computeName(const FormatToken *NamespaceTok) {
@@ -283,7 +279,7 @@ std::pair<tooling::Replacements, unsigned> NamespaceEndCommentsFixer::analyze(
computeEndCommentText(NamespaceName, AddNewline, NamespaceTok,
Style.SpacesInLineCommentPrefix.Minimum);
if (!hasEndComment(EndCommentPrevTok)) {
- bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
+ bool isShort = I - StartLineIndex <= Style.ShortNamespaceLines + 1;
if (!isShort)
addEndComment(EndCommentPrevTok, EndCommentText, SourceMgr, &Fixes);
} else if (!validEndComment(EndCommentPrevTok, NamespaceName,
diff --git a/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp b/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
index 3340e02f4a1c..9b0301930d21 100644
--- a/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ b/clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -816,7 +816,7 @@ TEST_F(NamespaceEndCommentsFixerTest,
"}\n"));
}
-TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) {
+TEST_F(NamespaceEndCommentsFixerTest, AddsEndCommentForNamespacesAroundMacros) {
// Conditional blocks around are fine
EXPECT_EQ("namespace A {\n"
"#if 1\n"
@@ -1116,6 +1116,75 @@ TEST_F(NamespaceEndCommentsFixerTest, IgnoreUnbalanced) {
"}\n"
"}\n"));
}
+
+using ShortNamespaceLinesTest = NamespaceEndCommentsFixerTest;
+
+TEST_F(ShortNamespaceLinesTest, ZeroUnwrappedLines) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 0u;
+
+ EXPECT_EQ("namespace OneLinerNamespace {}\n",
+ fixNamespaceEndComments("namespace OneLinerNamespace {}\n", Style));
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "}\n",
+ Style));
+ EXPECT_EQ("namespace LongNamespace {\n"
+ "int i;\n"
+ "}// namespace LongNamespace\n",
+ fixNamespaceEndComments("namespace LongNamespace {\n"
+ "int i;\n"
+ "}\n",
+ Style));
+}
+
+TEST_F(ShortNamespaceLinesTest, OneUnwrappedLine) {
+ constexpr auto DefaultUnwrappedLines = 1u;
+ auto const Style = getLLVMStyle();
+
+ EXPECT_EQ(DefaultUnwrappedLines, Style.ShortNamespaceLines);
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "int i;\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "int i;\n"
+ "}\n"));
+ EXPECT_EQ("namespace LongNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}// namespace LongNamespace\n",
+ fixNamespaceEndComments("namespace LongNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n"));
+}
+
+TEST_F(ShortNamespaceLinesTest, MultipleUnwrappedLine) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 2u;
+
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n",
+ Style));
+ EXPECT_EQ("namespace LongNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "int k;\n"
+ "}// namespace LongNamespace\n",
+ fixNamespaceEndComments("namespace LongNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "int k;\n"
+ "}\n",
+ Style));
+}
} // end namespace
} // end namespace format
} // end namespace clang
More information about the cfe-commits
mailing list