HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, klimek, krasimir.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
HazardyKnusperkeks requested review of this revision.

I'm not sure about the name.

And I don't know about changing the Prefix from `StringRef` to `std::string`, 
but I suppose nearly all prefixes should fit into the small buffer, so no heap 
allocation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92257

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===================================================================
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3269,6 +3269,183 @@
                    JSStyle20));
 }
 
+TEST_F(FormatTestComments, SpaceAtLineCommentBegin) {
+  FormatStyle Style = getLLVMStyle();
+  StringRef NoTextInComment = " // \n"
+                              "\n"
+                              "void foo() {// \n"
+                              "// \n"
+                              "}";
+
+  EXPECT_EQ("//\n"
+            "\n"
+            "void foo() { //\n"
+            "  //\n"
+            "}",
+            format(NoTextInComment, Style));
+
+  Style.SpacesInLineComments.Minimum = 0;
+  EXPECT_EQ("//\n"
+            "\n"
+            "void foo() { //\n"
+            "  //\n"
+            "}",
+            format(NoTextInComment, Style));
+
+  Style = getLLVMStyle();
+  StringRef Code = "//Free comment without space\n"
+                   "//   Free comment with 3 spaces\n"
+                   "///Free Doxygen without space\n"
+                   "///   Free Doxygen with 3 spaces\n"
+                   "namespace Foo {\n"
+                   "bool bar(bool b) {\n"
+                   "  bool ret1 = true; ///<Doxygenstyle without space\n"
+                   "  bool ret2 = true; ///<   Doxygenstyle with 3 spaces\n"
+                   "  if (b) {\n"
+                   "    //Foo\n"
+                   "    //   In function comment\n"
+                   "    ret2 = false;\n"
+                   "  } // End of if\n"
+                   "  return ret1 && ret2;\n"
+                   "}\n"
+                   "}\n"
+                   "\n"
+                   "namespace Bar {\n"
+                   "int foo();\n"
+                   "} //  namespace Bar\n"
+                   "//@Nothing added because of the non ascii char\n"
+                   "//@      Nothing removed because of the non ascii char\n";
+
+  EXPECT_EQ("// Free comment without space\n"
+            "//   Free comment with 3 spaces\n"
+            "/// Free Doxygen without space\n"
+            "///   Free Doxygen with 3 spaces\n"
+            "namespace Foo {\n"
+            "bool bar(bool b) {\n"
+            "  bool ret1 = true; ///< Doxygenstyle without space\n"
+            "  bool ret2 = true; ///<   Doxygenstyle with 3 spaces\n"
+            "  if (b) {\n"
+            "    // Foo\n"
+            "    //   In function comment\n"
+            "    ret2 = false;\n"
+            "  } // End of if\n"
+            "  return ret1 && ret2;\n"
+            "}\n"
+            "} // namespace Foo\n"
+            "\n"
+            "namespace Bar {\n"
+            "int foo();\n"
+            "} //  namespace Bar\n"
+            "//@Nothing added because of the non ascii char\n"
+            "//@      Nothing removed because of the non ascii char\n",
+            format(Code, Style));
+
+  Style.SpacesInLineComments = {0, 0};
+  EXPECT_EQ("//Free comment without space\n"
+            "//Free comment with 3 spaces\n"
+            "///Free Doxygen without space\n"
+            "///Free Doxygen with 3 spaces\n"
+            "namespace Foo {\n"
+            "bool bar(bool b) {\n"
+            "  bool ret1 = true; ///<Doxygenstyle without space\n"
+            "  bool ret2 = true; ///<Doxygenstyle with 3 spaces\n"
+            "  if (b) {\n"
+            "    //Foo\n"
+            "    //In function comment\n"
+            "    ret2 = false;\n"
+            "  } //End of if\n"
+            "  return ret1 && ret2;\n"
+            "}\n"
+            "} //namespace Foo\n"
+            "\n"
+            "namespace Bar {\n"
+            "int foo();\n"
+            "} //namespace Bar\n"
+            "//@Nothing added because of the non ascii char\n"
+            "//@      Nothing removed because of the non ascii char\n",
+            format(Code, Style));
+
+  Style.SpacesInLineComments = {2, -1u};
+  EXPECT_EQ("//  Free comment without space\n"
+            "//   Free comment with 3 spaces\n"
+            "///  Free Doxygen without space\n"
+            "///   Free Doxygen with 3 spaces\n"
+            "namespace Foo {\n"
+            "bool bar(bool b) {\n"
+            "  bool ret1 = true; ///<  Doxygenstyle without space\n"
+            "  bool ret2 = true; ///<   Doxygenstyle with 3 spaces\n"
+            "  if (b) {\n"
+            "    //  Foo\n"
+            "    //   In function comment\n"
+            "    ret2 = false;\n"
+            "  } //  End of if\n"
+            "  return ret1 && ret2;\n"
+            "}\n"
+            "} //  namespace Foo\n"
+            "\n"
+            "namespace Bar {\n"
+            "int foo();\n"
+            "} //  namespace Bar\n"
+            "//@Nothing added because of the non ascii char\n"
+            "//@      Nothing removed because of the non ascii char\n",
+            format(Code, Style));
+
+  Style = getLLVMStyleWithColumns(20);
+  StringRef WrapCode = "//Lorem ipsum dolor sit amet\n"
+                       "\n"
+                       "//  Lorem   ipsum   dolor   sit   amet\n"
+                       "\n"
+                       "void f() {//Hello World\n"
+                       "}";
+
+  EXPECT_EQ("// Lorem ipsum dolor\n"
+            "// sit amet\n"
+            "\n"
+            "//  Lorem   ipsum\n"
+            "//  dolor   sit amet\n" // Why are here the spaces dropped?
+            "\n"
+            "void f() { // Hello\n"
+            "           // World\n"
+            "}",
+            format(WrapCode, Style));
+
+  Style.SpacesInLineComments = {0, 0};
+  EXPECT_EQ("//Lorem ipsum dolor\n"
+            "//sit amet\n"
+            "\n"
+            "//Lorem   ipsum\n"
+            "//dolor   sit amet\n"
+            "\n"
+            "void f() { //Hello\n"
+            "           //World\n"
+            "}",
+            format(WrapCode, Style));
+
+  Style.SpacesInLineComments = {1, 1};
+  EXPECT_EQ("// Lorem ipsum dolor\n"
+            "// sit amet\n"
+            "\n"
+            "// Lorem   ipsum\n"
+            "// dolor   sit amet\n"
+            "\n"
+            "void f() { // Hello\n"
+            "           // World\n"
+            "}",
+            format(WrapCode, Style));
+
+  Style.SpacesInLineComments = {3, 3};
+  EXPECT_EQ("//   Lorem ipsum\n"
+            "//   dolor sit amet\n"
+            "\n"
+            "//   Lorem   ipsum\n"
+            "//   dolor   sit amet\n"
+            "\n"
+            "void f() { //   Hello\n"
+            "           //   World\n"
+            "}",
+            format(WrapCode, Style));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14571,6 +14571,26 @@
               "      - 'CPPEVAL'\n"
               "    CanonicalDelimiter: 'cc'",
               RawStringFormats, ExpectedRawStringFormats);
+
+  CHECK_PARSE("SpacesInLineComments:\n"
+              "  Minimum: 0\n"
+              "  Maximum: 0",
+              SpacesInLineComments.Minimum, 0u);
+  EXPECT_EQ(Style.SpacesInLineComments.Maximum, 0u);
+  Style.SpacesInLineComments.Minimum = 1;
+  CHECK_PARSE("SpacesInLineComments:\n"
+              "  Minimum: 2",
+              SpacesInLineComments.Minimum, 0u);
+  CHECK_PARSE("SpacesInLineComments:\n"
+              "  Maximum: -1",
+              SpacesInLineComments.Maximum, -1u);
+  CHECK_PARSE("SpacesInLineComments:\n"
+              "  Minimum: 2",
+              SpacesInLineComments.Minimum, 2u);
+  CHECK_PARSE("SpacesInLineComments:\n"
+              "  Maximum: 1",
+              SpacesInLineComments.Maximum, 1u);
+  EXPECT_EQ(Style.SpacesInLineComments.Minimum, 1u);
 }
 
 TEST_F(FormatTest, ParsesConfigurationWithLanguages) {
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===================================================================
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -66,8 +66,10 @@
 }
 
 std::string computeEndCommentText(StringRef NamespaceName, bool AddNewline,
-                                  const FormatToken *NamespaceTok) {
-  std::string text = "// ";
+                                  const FormatToken *NamespaceTok,
+                                  unsigned SpacesToAdd) {
+  std::string text = "//";
+  text.append(SpacesToAdd, ' ');
   text += NamespaceTok->TokenText;
   if (NamespaceTok->is(TT_NamespaceMacro))
     text += "(";
@@ -278,7 +280,8 @@
                       EndCommentNextTok->NewlinesBefore == 0 &&
                       EndCommentNextTok->isNot(tok::eof);
     const std::string EndCommentText =
-        computeEndCommentText(NamespaceName, AddNewline, NamespaceTok);
+        computeEndCommentText(NamespaceName, AddNewline, NamespaceTok,
+                              Style.SpacesInLineComments.Minimum);
     if (!hasEndComment(EndCommentPrevTok)) {
       bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
       if (!isShort)
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -624,6 +624,7 @@
                    Style.SpacesInContainerLiterals);
     IO.mapOptional("SpacesInCStyleCastParentheses",
                    Style.SpacesInCStyleCastParentheses);
+    IO.mapOptional("SpacesInLineComments", Style.SpacesInLineComments);
     IO.mapOptional("SpacesInParentheses", Style.SpacesInParentheses);
     IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
     IO.mapOptional("SpaceBeforeSquareBrackets",
@@ -673,6 +674,20 @@
   }
 };
 
+template <> struct MappingTraits<FormatStyle::SpacesInLineComment> {
+  static void mapping(IO &IO, FormatStyle::SpacesInLineComment &Space) {
+    // Transform the maximum to signed, to parse "-1" correctly
+    int signedMaximum = static_cast<int>(Space.Maximum);
+    IO.mapOptional("Minimum", Space.Minimum);
+    IO.mapOptional("Maximum", signedMaximum);
+    Space.Maximum = static_cast<unsigned>(signedMaximum);
+
+    if (Space.Maximum != -1u) {
+      Space.Minimum = std::min(Space.Minimum, Space.Maximum);
+    }
+  }
+};
+
 // Allows to read vector<FormatStyle> while keeping default values.
 // IO.getContext() should contain a pointer to the FormatStyle structure, that
 // will be used to get default values for missing keys.
@@ -945,6 +960,7 @@
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
+  LLVMStyle.SpacesInLineComments = {/*Minimum=*/1, /*Maximum=*/-1u};
   LLVMStyle.SpaceAfterCStyleCast = false;
   LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceAfterTemplateKeyword = true;
Index: clang/lib/Format/BreakableToken.h
===================================================================
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -471,8 +471,9 @@
   // formatting. It can be different than the original prefix when the original
   // line starts like this:
   // //content
-  // Then the original prefix is "//", but the prefix is "// ".
-  SmallVector<StringRef, 16> Prefix;
+  // Then the original prefix is "//", but the prefix could be "// ", if adding
+  // spaces is desired.
+  SmallVector<std::string, 16> Prefix;
 
   SmallVector<unsigned, 16> OriginalContentColumn;
 
Index: clang/lib/Format/BreakableToken.cpp
===================================================================
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -779,22 +779,25 @@
           getLineCommentIndentPrefix(Lines[i].ltrim(Blanks), Style);
       assert((TokenText.startswith("//") || TokenText.startswith("#")) &&
              "unsupported line comment prefix, '//' and '#' are supported");
-      OriginalPrefix[i] = Prefix[i] = IndentPrefix;
-      if (Lines[i].size() > Prefix[i].size() &&
-          isAlphanumeric(Lines[i][Prefix[i].size()])) {
-        if (Prefix[i] == "//")
-          Prefix[i] = "// ";
-        else if (Prefix[i] == "///")
-          Prefix[i] = "/// ";
-        else if (Prefix[i] == "//!")
-          Prefix[i] = "//! ";
-        else if (Prefix[i] == "///<")
-          Prefix[i] = "///< ";
-        else if (Prefix[i] == "//!<")
-          Prefix[i] = "//!< ";
-        else if (Prefix[i] == "#" &&
-                 Style.Language == FormatStyle::LK_TextProto)
-          Prefix[i] = "# ";
+      OriginalPrefix[i] = IndentPrefix;
+      const auto SpacesInPrefix =
+          std::count(IndentPrefix.begin(), IndentPrefix.end(), ' ');
+
+      if (SpacesInPrefix < Style.SpacesInLineComments.Minimum &&
+          Lines[i].size() > IndentPrefix.size() &&
+          isAlphanumeric(Lines[i][IndentPrefix.size()]) &&
+          (Style.Language != FormatStyle::LK_TextProto ||
+           OriginalPrefix[i].substr(0, 2) != "##")) {
+        Prefix[i] = IndentPrefix.str();
+        Prefix[i].append(Style.SpacesInLineComments.Minimum - SpacesInPrefix,
+                         ' ');
+      } else if (SpacesInPrefix > Style.SpacesInLineComments.Maximum) {
+        Prefix[i] =
+            IndentPrefix
+                .drop_back(SpacesInPrefix - Style.SpacesInLineComments.Maximum)
+                .str();
+      } else {
+        Prefix[i] = IndentPrefix.str();
       }
 
       Tokens[i] = LineTok;
@@ -968,14 +971,18 @@
   }
   if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
     // Adjust the prefix if necessary.
-
-    // Take care of the space possibly introduced after a decoration.
-    assert(Prefix[LineIndex] == (OriginalPrefix[LineIndex] + " ").str() &&
-           "Expecting a line comment prefix to differ from original by at most "
-           "a space");
+    bool RemoveWhitespace =
+        OriginalPrefix[LineIndex].size() > Prefix[LineIndex].size();
+    int SpacesToRemove = RemoveWhitespace ? OriginalPrefix[LineIndex].size() -
+                                                Prefix[LineIndex].size()
+                                          : 0;
+    int SpacesToAdd = RemoveWhitespace ? 0
+                                       : Prefix[LineIndex].size() -
+                                             OriginalPrefix[LineIndex].size();
     Whitespaces.replaceWhitespaceInToken(
-        tokenAt(LineIndex), OriginalPrefix[LineIndex].size(), 0, "", "",
-        /*InPPDirective=*/false, /*Newlines=*/0, /*Spaces=*/1);
+        tokenAt(LineIndex), OriginalPrefix[LineIndex].size() - SpacesToRemove,
+        /*ReplaceChars=*/SpacesToRemove, "", "", -/*InPPDirective=*/false,
+        /*Newlines=*/0, /*Spaces=*/SpacesToAdd);
   }
 }
 
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2282,6 +2282,27 @@
   /// \endcode
   bool SpacesInCStyleCastParentheses;
 
+  /// Control of spaces within a single line comment
+  struct SpacesInLineComment {
+    /// The minimum number of spaces at the start of the comment.
+    unsigned Minimum;
+    /// The maximum number of spaces at the start of the comment.
+    unsigned Maximum;
+  };
+
+  /// How many spaces are allowed at the start of a line comment. To disable the
+  /// maximum set it to ``-1``, apart from that the maximum takes precedence
+  /// over the minimum.
+  /// \code Minimum = 1 Maximum = -1
+  /// // One space is forced
+  /// //  but more spaces are possible
+  ///
+  /// Minimum = 0
+  /// Maximum = 0
+  /// //Forces to start every comment directly after the slashes
+  /// \endcode
+  SpacesInLineComment SpacesInLineComments;
+
   /// If ``true``, spaces will be inserted after ``(`` and before ``)``.
   /// \code
   ///    true:                                  false:
@@ -2512,6 +2533,8 @@
            SpacesInConditionalStatement == R.SpacesInConditionalStatement &&
            SpacesInContainerLiterals == R.SpacesInContainerLiterals &&
            SpacesInCStyleCastParentheses == R.SpacesInCStyleCastParentheses &&
+           SpacesInLineComments.Minimum == R.SpacesInLineComments.Minimum &&
+           SpacesInLineComments.Maximum == R.SpacesInLineComments.Maximum &&
            SpacesInParentheses == R.SpacesInParentheses &&
            SpacesInSquareBrackets == R.SpacesInSquareBrackets &&
            SpaceBeforeSquareBrackets == R.SpaceBeforeSquareBrackets &&
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2707,6 +2707,26 @@
      var arr = [ 1, 2, 3 ];         vs.     var arr = [1, 2, 3];
      f({a : 1, b : 2, c : 3});              f({a: 1, b: 2, c: 3});
 
+**SpacesInLineComments** (``SpacesInLineComment``)
+  How many spaces are allowed at the start of a line comment. To disable the
+  maximum set it to ``-1``, apart from that the maximum takes precedence
+  over the minimum.
+  Minimum = 1 Maximum = -1
+  // One space is forced
+  //  but more spaces are possible
+
+  Minimum = 0
+  Maximum = 0
+  //Forces to start every comment directly after the slashes
+
+  Nested configuration flags:
+
+
+  * ``unsigned Minimum`` The minimum number of spaces at the start of the comment.
+
+  * ``unsigned Maximum`` The maximum number of spaces at the start of the comment.
+
+
 **SpacesInParentheses** (``bool``)
   If ``true``, spaces will be inserted after ``(`` and before ``)``.
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to