[llvm-branch-commits] [clang] e9e6e3b - [clang-format] Add IndentPragma style to eliminate common clang-format off scenario

via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Dec 10 03:22:15 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 llvm-branch-commits mailing list