caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[clang-format] Add IndentPragma style to eliminate common clang-format off scenario

Browse Source 
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
This commit is contained in:
mydeveloperday
2020-12-10 11:17:33 +00:00
parent 7b2d62fd7f
commit e9e6e3b34a
8 changed files with 209 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
clang/docs/ClangFormatStyleOptions.rst
View File

@@ -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

24
clang/include/clang/Format/Format.h
View File

@@ -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 &&

6
clang/lib/Format/ContinuationIndenter.cpp
View File

@@ -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

2
clang/lib/Format/Format.cpp
View File

@@ -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;

19
clang/lib/Format/UnwrappedLineFormatter.cpp
View File

@@ -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,

15
clang/lib/Format/UnwrappedLineParser.cpp
View File

@@ -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 ||

2
clang/lib/Format/UnwrappedLineParser.h
View File

@@ -88,7 +88,7 @@ private:
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();

124
clang/unittests/Format/FormatTest.cpp
View File

@@ -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