[clang] e9e6e3b - [clang-format] Add IndentPragma style to eliminate common clang-format off scenario
via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 10 03:18:11 PST 2020
Author: mydeveloperday
Date: 2020-12-10T11:17:33Z
New Revision: e9e6e3b34a8e0857a274df51aac27e88c1de402b
URL: https://github.com/llvm/llvm-project/commit/e9e6e3b34a8e0857a274df51aac27e88c1de402b
DIFF: https://github.com/llvm/llvm-project/commit/e9e6e3b34a8e0857a274df51aac27e88c1de402b.diff
LOG: [clang-format] Add IndentPragma style to eliminate common clang-format off scenario
A quick search of github.com, shows one common scenario for excessive use of //clang-format off/on is the indentation of #pragma's, especially around the areas of loop optimization or OpenMP
This revision aims to help that by introducing an `IndentPragmas` style, the aim of which is to keep the pragma at the current level of scope
```
for (int i = 0; i < 5; i++) {
// clang-format off
#pragma HLS UNROLL
// clang-format on
for (int j = 0; j < 5; j++) {
// clang-format off
#pragma HLS UNROLL
// clang-format on
....
```
can become
```
for (int i = 0; i < 5; i++) {
#pragma HLS UNROLL
for (int j = 0; j < 5; j++) {
#pragma HLS UNROLL
....
```
This revision also support working alongside the `IndentPPDirective` of `BeforeHash` and `AfterHash` (see unit tests for examples)
Reviewed By: curdeius
Differential Revision: https://reviews.llvm.org/D92753
Added:
Modified:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/ContinuationIndenter.cpp
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 239c9177bec2..f63ed168f099 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1917,6 +1917,30 @@ the configuration (without a prefix: ``Auto``).
+**IndentPragmas** (``bool``)
+ Indent pragmas
+
+ When ``false``, pragmas are flushed left or follow IndentPPDirectives.
+ When ``true``, pragmas are indented to the current scope level.
+
+ .. code-block:: c++
+
+ false: true:
+ #pragma once vs #pragma once
+ void foo() { void foo() {
+ #pragma omp simd #pragma omp simd
+ for (int i=0;i<10;i++) { for (int i=0;i<10;i++) {
+ #pragma omp simd #pragma omp simd
+ for (int i=0;i<10;i++) { for (int i=0;i<10;i++) {
+ } }
+ #if 1 #if 1
+ #pragma omp simd #pragma omp simd
+ for (int i=0;i<10;i++) { for (int i=0;i<10;i++) {
+ } }
+ #endif #endif
+ } }
+ } }
+
**IndentRequires** (``bool``)
Indent the requires clause in a template
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 6e304630fbdc..d8ed27687b63 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1528,6 +1528,29 @@ struct FormatStyle {
/// \endcode
bool IndentGotoLabels;
+ /// Indent pragmas
+ ///
+ /// When ``false``, pragmas are flushed left or follow IndentPPDirectives.
+ /// When ``true``, pragmas are indented to the current scope level.
+ /// \code
+ /// false: true:
+ /// #pragma once vs #pragma once
+ /// void foo() { void foo() {
+ /// #pragma omp simd #pragma omp simd
+ /// for (int i=0;i<10;i++) { for (int i=0;i<10;i++) {
+ /// #pragma omp simd #pragma omp simd
+ /// for (int i=0;i<10;i++) { for (int i=0;i<10;i++) {
+ /// } }
+ /// #if 1 #if 1
+ /// #pragma omp simd #pragma omp simd
+ /// for (int i=0;i<10;i++) { for (int i=0;i<10;i++) {
+ /// } }
+ /// #endif #endif
+ /// } }
+ /// } }
+ /// \endcode
+ bool IndentPragmas;
+
/// Options for indenting preprocessor directives.
enum PPDirectiveIndentStyle {
/// Does not indent any directives.
@@ -2494,6 +2517,7 @@ struct FormatStyle {
IndentCaseLabels == R.IndentCaseLabels &&
IndentCaseBlocks == R.IndentCaseBlocks &&
IndentGotoLabels == R.IndentGotoLabels &&
+ IndentPragmas == R.IndentPragmas &&
IndentPPDirectives == R.IndentPPDirectives &&
IndentExternBlock == R.IndentExternBlock &&
IndentRequires == R.IndentRequires && IndentWidth == R.IndentWidth &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 9db42b6c4a70..9e15c87509ac 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -589,6 +589,12 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
State.Line->Type == LT_ImportStatement)) {
Spaces += State.FirstIndent;
+ bool isPragmaLine =
+ State.Line->First->startsSequence(tok::hash, tok::pp_pragma);
+ // If indenting pragmas remove the extra space for the #.
+ if (Style.IndentPragmas && isPragmaLine)
+ Spaces--;
+
// For preprocessor indent with tabs, State.Column will be 1 because of the
// hash. This causes second-level indents onward to have an extra space
// after the tabs. We avoid this misalignment by subtracting 1 from the
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 4263f7b47e7c..23eee19b1640 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -557,6 +557,7 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
IO.mapOptional("IndentCaseBlocks", Style.IndentCaseBlocks);
IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
+ IO.mapOptional("IndentPragmas", Style.IndentPragmas);
IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
IO.mapOptional("IndentExternBlock", Style.IndentExternBlock);
IO.mapOptional("IndentRequires", Style.IndentRequires);
@@ -924,6 +925,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.IndentCaseBlocks = false;
LLVMStyle.IndentGotoLabels = true;
+ LLVMStyle.IndentPragmas = false;
LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
LLVMStyle.IndentRequires = false;
LLVMStyle.IndentWrappedFunctionNames = false;
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index d8f718306a45..e39ee26950bc 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1237,10 +1237,21 @@ void UnwrappedLineFormatter::formatFirstToken(
}
// Preprocessor directives get indented before the hash only if specified
- if (Style.IndentPPDirectives != FormatStyle::PPDIS_BeforeHash &&
- (Line.Type == LT_PreprocessorDirective ||
- Line.Type == LT_ImportStatement))
- Indent = 0;
+ if (Line.Type == LT_PreprocessorDirective ||
+ Line.Type == LT_ImportStatement) {
+ switch (Style.IndentPPDirectives) {
+ case FormatStyle::PPDIS_AfterHash:
+ Indent = 0;
+ break;
+ case FormatStyle::PPDIS_None:
+ case FormatStyle::PPDIS_BeforeHash: {
+ // If we want to indent pragmas.
+ bool isPragmaLine = RootToken.startsSequence(tok::hash, tok::pp_pragma);
+ if (!Style.IndentPragmas && isPragmaLine)
+ Indent = 0;
+ } break;
+ }
+ }
Whitespaces->replaceWhitespace(RootToken, Newlines, Indent, Indent,
/*IsAligned=*/false,
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 3803119a2096..ddb3a247429a 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -714,7 +714,7 @@ void UnwrappedLineParser::parseChildBlock() {
nextToken();
}
-void UnwrappedLineParser::parsePPDirective() {
+void UnwrappedLineParser::parsePPDirective(unsigned Level) {
assert(FormatTok->Tok.is(tok::hash) && "'#' expected");
ScopedMacroState MacroState(*Line, Tokens, FormatTok);
@@ -745,6 +745,17 @@ void UnwrappedLineParser::parsePPDirective() {
case tok::pp_endif:
parsePPEndIf();
break;
+ case tok::pp_pragma: {
+ bool IndentPPDirectives =
+ Style.IndentPPDirectives != FormatStyle::PPDIS_None;
+ unsigned CurrentLevel = Line->Level;
+ Line->Level =
+ Style.IndentPragmas
+ ? (IndentPPDirectives ? (Level - (PPBranchLevel + 1)) : Level)
+ : CurrentLevel;
+ parsePPUnknown();
+ Line->Level = CurrentLevel;
+ } break;
default:
parsePPUnknown();
break;
@@ -3157,7 +3168,7 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
PPBranchLevel > 0)
Line->Level += PPBranchLevel;
flushComments(isOnNewLine(*FormatTok));
- parsePPDirective();
+ parsePPDirective(Line->Level);
}
while (FormatTok->getType() == TT_ConflictStart ||
FormatTok->getType() == TT_ConflictEnd ||
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 02b328cb72de..30604c037986 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -88,7 +88,7 @@ class UnwrappedLineParser {
void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
bool MunchSemi = true);
void parseChildBlock();
- void parsePPDirective();
+ void parsePPDirective(unsigned Level);
void parsePPDefine();
void parsePPIf(bool IfDef);
void parsePPElIf();
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index d3ea5c2b5880..1c78b6e2cb1e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -14116,6 +14116,7 @@ TEST_F(FormatTest, ParsesConfigurationBools) {
CHECK_PARSE_BOOL(IndentCaseLabels);
CHECK_PARSE_BOOL(IndentCaseBlocks);
CHECK_PARSE_BOOL(IndentGotoLabels);
+ CHECK_PARSE_BOOL(IndentPragmas);
CHECK_PARSE_BOOL(IndentRequires);
CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
@@ -17571,6 +17572,129 @@ TEST_F(FormatTest, ConceptsAndRequires) {
"struct constant;",
Style);
}
+
+TEST_F(FormatTest, IndentPragmas) {
+ FormatStyle Style = getLLVMStyle();
+ Style.IndentPPDirectives = FormatStyle::PPDIS_None;
+
+ Style.IndentPragmas = false;
+ verifyFormat("#pragma once", Style);
+ verifyFormat("#pragma omp simd\n"
+ "for (int i = 0; i < 10; i++) {\n"
+ "}",
+ Style);
+ verifyFormat("void foo() {\n"
+ "#pragma omp simd\n"
+ " for (int i = 0; i < 10; i++) {\n"
+ " }\n"
+ "}",
+ Style);
+ verifyFormat("void foo() {\n"
+ "// outer loop\n"
+ "#pragma omp simd\n"
+ " for (int k = 0; k < 10; k++) {\n"
+ "// inner loop\n"
+ "#pragma omp simd\n"
+ " for (int j = 0; j < 10; j++) {\n"
+ " }\n"
+ " }\n"
+ "}",
+ Style);
+
+ verifyFormat("void foo() {\n"
+ "// outer loop\n"
+ "#if 1\n"
+ "#pragma omp simd\n"
+ " for (int k = 0; k < 10; k++) {\n"
+ "// inner loop\n"
+ "#pragma omp simd\n"
+ " for (int j = 0; j < 10; j++) {\n"
+ " }\n"
+ " }\n"
+ "#endif\n"
+ "}",
+ Style);
+
+ Style.IndentPragmas = true;
+ verifyFormat("#pragma once", Style);
+ verifyFormat("#pragma omp simd\n"
+ "for (int i = 0; i < 10; i++) {\n"
+ "}",
+ Style);
+ verifyFormat("void foo() {\n"
+ " #pragma omp simd\n"
+ " for (int i = 0; i < 10; i++) {\n"
+ " }\n"
+ "}",
+ Style);
+ verifyFormat("void foo() {\n"
+ " #pragma omp simd\n"
+ " for (int i = 0; i < 10; i++) {\n"
+ " #pragma omp simd\n"
+ " for (int j = 0; j < 10; j++) {\n"
+ " }\n"
+ " }\n"
+ "}",
+ Style);
+
+ verifyFormat("void foo() {\n"
+ " #pragma omp simd\n"
+ " for (...) {\n"
+ " #pragma omp simd\n"
+ " for (...) {\n"
+ " }\n"
+ " }\n"
+ "}",
+ Style);
+
+ Style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+
+ verifyFormat("void foo() {\n"
+ "# pragma omp simd\n"
+ " for (int i = 0; i < 10; i++) {\n"
+ "# pragma omp simd\n"
+ " for (int j = 0; j < 10; j++) {\n"
+ " }\n"
+ " }\n"
+ "}",
+ Style);
+
+ verifyFormat("void foo() {\n"
+ "#if 1\n"
+ "# pragma omp simd\n"
+ " for (int k = 0; k < 10; k++) {\n"
+ "# pragma omp simd\n"
+ " for (int j = 0; j < 10; j++) {\n"
+ " }\n"
+ " }\n"
+ "#endif\n"
+ "}",
+ Style);
+
+ Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+ EXPECT_EQ("void foo() {\n"
+ "#if 1\n"
+ " #pragma omp simd\n"
+ " for (int k = 0; k < 10; k++) {\n"
+ " #pragma omp simd\n"
+ " for (int j = 0; j < 10; j++) {\n"
+ " }\n"
+ " }\n"
+ "#endif\n"
+ "}",
+ format("void foo() {\n"
+ "#if 1\n"
+ " #pragma omp simd\n"
+ " for (int k = 0; k < 10; k++) {\n"
+ " #pragma omp simd\n"
+ " for (int j = 0; j < 10; j++) {\n"
+ " }\n"
+ " }\n"
+ "#endif\n"
+ "}",
+ Style));
+}
+
} // namespace
} // namespace format
} // namespace clang
More information about the cfe-commits
mailing list