MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, JakeMerdichAMD, krasimir.
MyDeveloperDay added projects: clang-format, clang.
MyDeveloperDay requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.
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)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D92753
Files:
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
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -67,12 +67,13 @@
void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
+ const FormatStyle &Style = getLLVMStyle(),
+ bool messUp = true) {
ScopedTrace t(File, Line, ::testing::Message() << Code.str());
EXPECT_EQ(Expected.str(), format(Expected, Style))
<< "Expected code is not stable";
EXPECT_EQ(Expected.str(), format(Code, Style));
- if (Style.Language == FormatStyle::LK_Cpp) {
+ if (Style.Language == FormatStyle::LK_Cpp && messUp) {
// Objective-C++ is a superset of C++, so everything checked for C++
// needs to be checked for Objective-C++ as well.
FormatStyle ObjCStyle = Style;
@@ -82,8 +83,10 @@
}
void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle()) {
- _verifyFormat(File, Line, Code, test::messUp(Code), Style);
+ const FormatStyle &Style = getLLVMStyle(),
+ bool messUp = true) {
+ _verifyFormat(File, Line, Code, messUp ? test::messUp(Code) : Code, Style,
+ messUp);
}
void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef Code,
@@ -14116,6 +14119,7 @@
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);
@@ -17562,6 +17566,119 @@
"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,
+ /*messUp=*/false);
+ verifyFormat("void foo() {\n"
+ "#pragma omp simd\n"
+ " for (int i = 0; i < 10; i++) {\n"
+ " }\n"
+ "}",
+ Style,
+ /*messUp=*/false);
+ 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,
+ /*messUp=*/false);
+
+ 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,
+ /*messUp=*/false);
+
+ Style.IndentPragmas = true;
+ verifyFormat("#pragma once", Style);
+ verifyFormat("#pragma omp simd\n"
+ "for (int i = 0; i < 10; i++) {\n"
+ "}",
+ Style,
+ /*messUp=*/false);
+ verifyFormat("void foo() {\n"
+ " #pragma omp simd\n"
+ " for (int i = 0; i < 10; i++) {\n"
+ " }\n"
+ "}",
+ Style,
+ /*messUp=*/false);
+ 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,
+ /*messUp=*/false);
+
+ 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,
+ /*messUp=*/false);
+
+ 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,
+ /*messUp=*/false);
+
+ Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+ 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,
+ /*messUp=*/false);
+}
+
} // namespace
} // namespace format
} // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.h
===================================================================
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -88,7 +88,7 @@
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();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -714,7 +714,7 @@
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 @@
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 @@
PPBranchLevel > 0)
Line->Level += PPBranchLevel;
flushComments(isOnNewLine(*FormatTok));
- parsePPDirective();
+ parsePPDirective(Line->Level);
}
while (FormatTok->getType() == TT_ConflictStart ||
FormatTok->getType() == TT_ConflictEnd ||
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1237,10 +1237,21 @@
}
// 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,
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -557,6 +557,7 @@
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 @@
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.IndentCaseBlocks = false;
LLVMStyle.IndentGotoLabels = true;
+ LLVMStyle.IndentPragmas = false;
LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
LLVMStyle.IndentRequires = false;
LLVMStyle.IndentWrappedFunctionNames = false;
Index: clang/lib/Format/ContinuationIndenter.cpp
===================================================================
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -589,6 +589,13 @@
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
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1528,6 +1528,30 @@
/// \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: vs false:
+ ///
+ /// #pragma once #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 +2518,7 @@
IndentCaseLabels == R.IndentCaseLabels &&
IndentCaseBlocks == R.IndentCaseBlocks &&
IndentGotoLabels == R.IndentGotoLabels &&
+ IndentPragmas == R.IndentPragmas &&
IndentPPDirectives == R.IndentPPDirectives &&
IndentExternBlock == R.IndentExternBlock &&
IndentRequires == R.IndentRequires && IndentWidth == R.IndentWidth &&
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1917,6 +1917,31 @@
+**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: vs false:
+
+ #pragma once #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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
