kentsommer updated this revision to Diff 319804.
kentsommer added a comment.

Added release notes, updated commit message and summary


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95017/new/

https://reviews.llvm.org/D95017

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortImportsTestJava.cpp
  clang/unittests/Format/SortIncludesTest.cpp

Index: clang/unittests/Format/SortIncludesTest.cpp
===================================================================
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -269,7 +269,7 @@
 }
 
 TEST_F(SortIncludesTest, IncludeSortingCanBeDisabled) {
-  FmtStyle.SortIncludes = false;
+  FmtStyle.SortIncludes = FormatStyle::SI_Never;
   EXPECT_EQ("#include \"a.h\"\n"
             "#include \"c.h\"\n"
             "#include \"b.h\"\n",
@@ -598,6 +598,49 @@
                  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, SupportOptionalCaseSensitiveSorting) {
+  EXPECT_FALSE(FmtStyle.SortIncludes == FormatStyle::SI_CaseSensitive);
+
+  FmtStyle.SortIncludes = FormatStyle::SI_CaseSensitive;
+
+  EXPECT_EQ("#include \"A/B.h\"\n"
+            "#include \"A/b.h\"\n"
+            "#include \"a/b.h\"\n"
+            "#include \"B/A.h\"\n"
+            "#include \"B/a.h\"\n",
+            sort("#include \"B/a.h\"\n"
+                 "#include \"B/A.h\"\n"
+                 "#include \"A/B.h\"\n"
+                 "#include \"a/b.h\"\n"
+                 "#include \"A/b.h\"\n",
+                 "a.h"));
+
+  Style.IncludeBlocks = clang::tooling::IncludeStyle::IBS_Regroup;
+  Style.IncludeCategories = {
+      {"^\"", 1, 0, false}, {"^<.*\\.h>$", 2, 0, false}, {"^<", 3, 0, false}};
+
+  StringRef UnsortedCode = "#include \"qt.h\"\n"
+                           "#include <algorithm>\n"
+                           "#include <qtwhatever.h>\n"
+                           "#include <Qtwhatever.h>\n"
+                           "#include <Algorithm>\n"
+                           "#include \"vlib.h\"\n"
+                           "#include \"Vlib.h\"\n"
+                           "#include \"AST.h\"\n";
+
+  EXPECT_EQ("#include \"AST.h\"\n"
+            "#include \"qt.h\"\n"
+            "#include \"Vlib.h\"\n"
+            "#include \"vlib.h\"\n"
+            "\n"
+            "#include <Qtwhatever.h>\n"
+            "#include <qtwhatever.h>\n"
+            "\n"
+            "#include <Algorithm>\n"
+            "#include <algorithm>\n",
+            sort(UnsortedCode));
+}
+
 TEST_F(SortIncludesTest, SupportCaseInsensitiveMatching) {
   // Setup an regex for main includes so we can cover those as well.
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
Index: clang/unittests/Format/SortImportsTestJava.cpp
===================================================================
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -32,7 +32,7 @@
   SortImportsTestJava() {
     FmtStyle = getGoogleStyle(FormatStyle::LK_Java);
     FmtStyle.JavaImportGroups = {"com.test", "org", "com"};
-    FmtStyle.SortIncludes = true;
+    FmtStyle.SortIncludes = FormatStyle::SI_CaseInsensitive;
   }
 };
 
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15463,7 +15463,6 @@
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
   CHECK_PARSE_BOOL(Cpp11BracedListStyle);
   CHECK_PARSE_BOOL(ReflowComments);
-  CHECK_PARSE_BOOL(SortIncludes);
   CHECK_PARSE_BOOL(SortUsingDeclarations);
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
@@ -15959,6 +15958,16 @@
   CHECK_PARSE("IncludeIsMainSourceRegex: 'abc$'",
               IncludeStyle.IncludeIsMainSourceRegex, "abc$");
 
+  Style.SortIncludes = FormatStyle::SI_Never;
+  CHECK_PARSE("SortIncludes: true", SortIncludes,
+              FormatStyle::SI_CaseInsensitive);
+  CHECK_PARSE("SortIncludes: false", SortIncludes, FormatStyle::SI_Never);
+  CHECK_PARSE("SortIncludes: CaseInsensitive", SortIncludes,
+              FormatStyle::SI_CaseInsensitive);
+  CHECK_PARSE("SortIncludes: CaseSensitive", SortIncludes,
+              FormatStyle::SI_CaseSensitive);
+  CHECK_PARSE("SortIncludes: Never", SortIncludes, FormatStyle::SI_Never);
+
   Style.RawStringFormats.clear();
   std::vector<FormatStyle::RawStringFormat> ExpectedRawStringFormats = {
       {
@@ -17950,7 +17959,7 @@
                             "#include \"b.h\"\n")});
 
   format::FormatStyle Style = format::getLLVMStyle();
-  Style.SortIncludes = true;
+  Style.SortIncludes = FormatStyle::SI_CaseInsensitive;
   auto FormattedReplaces = formatReplacements(Code, Replaces, Style);
   EXPECT_TRUE(static_cast<bool>(FormattedReplaces))
       << llvm::toString(FormattedReplaces.takeError()) << "\n";
Index: clang/tools/clang-format/ClangFormat.cpp
===================================================================
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -402,8 +402,12 @@
     return true;
   }
 
-  if (SortIncludes.getNumOccurrences() != 0)
-    FormatStyle->SortIncludes = SortIncludes;
+  if (SortIncludes.getNumOccurrences() != 0) {
+    if (SortIncludes)
+      FormatStyle->SortIncludes = FormatStyle::SI_CaseInsensitive;
+    else
+      FormatStyle->SortIncludes = FormatStyle::SI_Never;
+  }
   unsigned CursorPosition = Cursor;
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
                                        AssumedFileName, &CursorPosition);
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -415,6 +415,18 @@
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::SortIncludesOptions> {
+  static void enumeration(IO &IO, FormatStyle::SortIncludesOptions &Value) {
+    IO.enumCase(Value, "Never", FormatStyle::SI_Never);
+    IO.enumCase(Value, "CaseInsensitive", FormatStyle::SI_CaseInsensitive);
+    IO.enumCase(Value, "CaseSensitive", FormatStyle::SI_CaseSensitive);
+
+    // For backward compatibility.
+    IO.enumCase(Value, "false", FormatStyle::SI_Never);
+    IO.enumCase(Value, "true", FormatStyle::SI_CaseInsensitive);
+  }
+};
+
 template <>
 struct ScalarEnumerationTraits<FormatStyle::SortJavaStaticImportOptions> {
   static void enumeration(IO &IO,
@@ -1013,7 +1025,7 @@
   LLVMStyle.PenaltyIndentedWhitespace = 0;
 
   LLVMStyle.DisableFormat = false;
-  LLVMStyle.SortIncludes = true;
+  LLVMStyle.SortIncludes = FormatStyle::SI_CaseInsensitive;
   LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
   LLVMStyle.SortUsingDeclarations = true;
   LLVMStyle.StatementAttributeLikeMacros.push_back("Q_EMIT");
@@ -1216,7 +1228,7 @@
         "java",
         "javax",
     };
-    ChromiumStyle.SortIncludes = true;
+    ChromiumStyle.SortIncludes = FormatStyle::SI_CaseInsensitive;
   } else if (Language == FormatStyle::LK_JavaScript) {
     ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
     ChromiumStyle.AllowShortLoopsOnASingleLine = false;
@@ -1330,7 +1342,7 @@
 FormatStyle getNoStyle() {
   FormatStyle NoStyle = getLLVMStyle();
   NoStyle.DisableFormat = true;
-  NoStyle.SortIncludes = false;
+  NoStyle.SortIncludes = FormatStyle::SI_Never;
   NoStyle.SortUsingDeclarations = false;
   return NoStyle;
 }
@@ -2209,10 +2221,23 @@
   for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
     Indices.push_back(i);
   }
-  llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
-    return std::tie(Includes[LHSI].Priority, Includes[LHSI].Filename) <
-           std::tie(Includes[RHSI].Priority, Includes[RHSI].Filename);
-  });
+
+  if (Style.SortIncludes == FormatStyle::SI_CaseSensitive) {
+    llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
+      const auto LHSFilenameLower = Includes[LHSI].Filename.lower();
+      const auto RHSFilenameLower = Includes[RHSI].Filename.lower();
+      return std::tie(Includes[LHSI].Priority, LHSFilenameLower,
+                      Includes[LHSI].Filename) <
+             std::tie(Includes[RHSI].Priority, RHSFilenameLower,
+                      Includes[RHSI].Filename);
+    });
+  } else {
+    llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
+      return std::tie(Includes[LHSI].Priority, Includes[LHSI].Filename) <
+             std::tie(Includes[RHSI].Priority, Includes[RHSI].Filename);
+    });
+  }
+
   // The index of the include on which the cursor will be put after
   // sorting/deduplicating.
   unsigned CursorIndex;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2613,13 +2613,44 @@
   bool ReflowComments;
   // clang-format on
 
-  /// If ``true``, clang-format will sort ``#includes``.
-  /// \code
-  ///    false:                                 true:
-  ///    #include "b.h"                 vs.     #include "a.h"
-  ///    #include "a.h"                         #include "b.h"
-  /// \endcode
-  bool SortIncludes;
+  /// Include sorting options.
+  enum SortIncludesOptions : unsigned char {
+    /// Includes are never sorted.
+    /// \code
+    ///    #include "B/A.h"
+    ///    #include "A/B.h"
+    ///    #include "a/b.h"
+    ///    #include "A/b.h"
+    ///    #include "B/a.h"
+    /// \endcode
+    SI_Never,
+    /// Includes are sorted in an ASCIIbetical or case insensitive fashion.
+    /// \code
+    ///    #include "A/B.h"
+    ///    #include "A/b.h"
+    ///    #include "B/A.h"
+    ///    #include "B/a.h"
+    ///    #include "a/b.h"
+    /// \endcode
+    SI_CaseInsensitive,
+    /// Includes are sorted in an alphabetical or case sensitive fashion.
+    /// \code
+    ///    #include "A/B.h"
+    ///    #include "A/b.h"
+    ///    #include "a/b.h"
+    ///    #include "B/A.h"
+    ///    #include "B/a.h"
+    /// \endcode
+    SI_CaseSensitive,
+  };
+
+  /// Controls if and how clang-format will sort ``#includes``.
+  /// If ``Never``, includes are never sorted.
+  /// If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
+  /// insensitive fashion.
+  /// If ``CaseSensitive``, includes are sorted in an alphabetical or case
+  /// sensitive fashion.
+  SortIncludesOptions SortIncludes;
 
   /// Position for Java Static imports.
   enum SortJavaStaticImportOptions : unsigned char {
@@ -3124,6 +3155,7 @@
                R.PenaltyBreakTemplateDeclaration &&
            PointerAlignment == R.PointerAlignment &&
            RawStringFormats == R.RawStringFormats &&
+           SortIncludes == R.SortIncludes &&
            SortJavaStaticImport == R.SortJavaStaticImport &&
            SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
            SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,6 +156,32 @@
 clang-format
 ------------
 
+- Option ``SortIncludes`` has been updated from a ``bool`` to an
+  ``enum`` with backward compatibility. In addition to the previous
+  ``true``/``false`` states (now ``CaseInsensitive``/``Never``) a third
+  state has been added (``CaseSensitive``) which causes an alphabetical sort
+  with case used as a tie-breaker.
+
+  .. code-block:: c++
+    // Never (previouslly false)
+    #include "B/A.h"
+    #include "A/B.h"
+    #include "a/b.h"
+    #include "A/b.h"
+    #include "B/a.h"
+    // CaseInsensitive (previouslly true)
+    #include "A/B.h"
+    #include "A/b.h"
+    #include "B/A.h"
+    #include "B/a.h"
+    #include "a/b.h"
+    // CaseSensitive
+    #include "A/B.h"
+    #include "A/b.h"
+    #include "a/b.h"
+    #include "B/A.h"
+    #include "B/a.h"
+
 - ...
 
 libclang
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -3004,14 +3004,43 @@
      /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
       * information */
 
-**SortIncludes** (``bool``)
-  If ``true``, clang-format will sort ``#includes``.
+**SortIncludes** (``SortIncludesOptions``)
+  Controls if and how clang-format will sort ``#includes``.
 
-  .. code-block:: c++
+  Possible Values:
 
-     false:                                 true:
-     #include "b.h"                 vs.     #include "a.h"
-     #include "a.h"                         #include "b.h"
+  * ``SI_Never`` (in configuration ``Never``)
+    Includes are never sorted.
+
+    .. code-block:: c++
+
+      #include "B/A.h"
+      #include "A/B.h"
+      #include "a/b.h"
+      #include "A/b.h"
+      #include "B/a.h"
+
+  * ``SI_CaseInsensitive`` (in configuration ``CaseInsensitive``)
+    Includes are sorted in an ASCIIbetical or case insensitive fashion.
+
+    .. code-block:: c++
+
+      #include "A/B.h"
+      #include "A/b.h"
+      #include "B/A.h"
+      #include "B/a.h"
+      #include "a/b.h"
+
+  * ``SI_CaseSensitive`` (in configuration ``CaseSensitive``)
+    Includes are sorted in an alphabetical or case sensitive fashion.
+
+    .. code-block:: c++
+
+      #include "A/B.h"
+      #include "A/b.h"
+      #include "a/b.h"
+      #include "B/A.h"
+      #include "B/a.h"
 
 **SortJavaStaticImport** (``SortJavaStaticImportOptions``)
   When sorting Java imports, by default static imports are placed before
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to