Skip to content

[clang-format] Add MainIncludeChar option. #78752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

j-jorge
Copy link
Contributor

@j-jorge j-jorge commented Jan 19, 2024

Resolves #27008, #39735, #53013, #63619.

Hello, this PR adds the MainIncludeChar option to clang-format, allowing to select which include syntax must be considered when searching for the main header: quotes (#include "foo.hpp", the default), brackets (#include <foo.hpp>), or both.

The lack of support for brackets has been reported many times, see the linked issues, so I am pretty sure there is a need for it :)

A short note about why I did not implement a regex approach as discussed in #53013: while a regex would have allowed many extra ways to describe the main header, the bug descriptions listed above suggest a very simple need: support brackets for the main header. This PR answers this needs in a quite simple way, with a very simple style option. IMHO the feature space covered by the regex (again, for which there is no demand :)) can be implemented latter, in addition to the proposed option.

The PR also includes tests for the option with and without grouped includes.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang-format labels Jan 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 19, 2024

@llvm/pr-subscribers-clang-format

@llvm/pr-subscribers-clang

Author: None (j-jorge)

Changes

Resolves #27008, #39735, #53013, #63619.

Hello, this PR adds the MainIncludeChar option to clang-format, allowing to select which include syntax must be considered when searching for the main header: quotes (#include "foo.hpp", the default), brackets (#include &lt;foo.hpp&gt;), or both.

The lack of support for brackets has been reported many times, see the linked issues, so I am pretty sure there is a need for it :)

A short note about why I did not implement a regex approach as discussed in #53013: while a regex would have allowed many extra ways to describe the main header, the bug descriptions listed above suggest a very simple need: support brackets for the main header. This PR answers this needs in a quite simple way, with a very simple style option. IMHO the feature space covered by the regex (again, for which there is no demand :)) can be implemented latter, in addition to the proposed option.

The PR also includes tests for the option with and without grouped includes.


Full diff: https://github.com/llvm/llvm-project/pull/78752.diff

8 Files Affected:

  • (modified) clang/docs/ClangFormatStyleOptions.rst (+18)
  • (modified) clang/include/clang/Tooling/Inclusions/IncludeStyle.h (+22)
  • (modified) clang/lib/Format/Format.cpp (+2)
  • (modified) clang/lib/Tooling/Inclusions/HeaderIncludes.cpp (+12-2)
  • (modified) clang/lib/Tooling/Inclusions/IncludeStyle.cpp (+7)
  • (added) clang/test/Format/main-include-char-bracket-group.cpp (+27)
  • (added) clang/test/Format/main-include-char-quote-group.cpp (+29)
  • (added) clang/test/Format/main-include-char.cpp (+15)
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 8bc13e45bf2f5f..af72b725640e9b 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3392,6 +3392,24 @@ the configuration (without a prefix: ``Auto``).
         Priority:        1
         SortPriority:    0
 
+.. _MainIncludeChar:
+
+**MainIncludeChar** (``MainIncludeCharDiscriminator``) :versionbadge:`clang-format 18` :ref:`¶ <MainIncludeChar>`
+  When guessing whether a #include is the "main" include (to assign
+  category 0, see above), only the include directives that use the
+  specified character are considered.
+
+  Possible values:
+
+  * ``MICD_Quote`` (the default, in configuration: ``Quote``)
+    The main include uses quotes, e.g. ``#include "foo.hpp"``.
+
+  * ``MICD_Bracket`` (in configuration: ``Bracket``)
+    The main include uses brackets, e.g. ``#include <foo.hpp>``.
+
+  * ``MICD_Any`` (in configuration: ``Any``)
+    The main include uses either quotes or brackets.
+
 .. _IncludeIsMainRegex:
 
 **IncludeIsMainRegex** (``String``) :versionbadge:`clang-format 3.9` :ref:`¶ <IncludeIsMainRegex>`
diff --git a/clang/include/clang/Tooling/Inclusions/IncludeStyle.h b/clang/include/clang/Tooling/Inclusions/IncludeStyle.h
index d6b2b0192477dc..5ec72dac227a09 100644
--- a/clang/include/clang/Tooling/Inclusions/IncludeStyle.h
+++ b/clang/include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -151,6 +151,21 @@ struct IncludeStyle {
   /// before any other include.
   /// \version 10
   std::string IncludeIsMainSourceRegex;
+
+  /// Character to consider in the include directives for the main header.
+  enum MainIncludeCharDiscriminator {
+    /// Main include uses quotes: ``#include "foo.hpp"``
+    MICD_Quote,
+    /// Main include uses brackets: ``#include <foo.hpp>``
+    MICD_Bracket,
+    /// Main include uses either quotes or brackets.
+    MICD_Any
+  };
+
+  /// When guessing whether a #include is the "main" include, only the include
+  /// directives that use the specified character are considered.
+  /// \version 18
+  MainIncludeCharDiscriminator MainIncludeChar;
 };
 
 } // namespace tooling
@@ -174,6 +189,13 @@ struct ScalarEnumerationTraits<
   enumeration(IO &IO, clang::tooling::IncludeStyle::IncludeBlocksStyle &Value);
 };
 
+template <>
+struct ScalarEnumerationTraits<
+    clang::tooling::IncludeStyle::MainIncludeCharDiscriminator> {
+  static void
+  enumeration(IO &IO, clang::tooling::IncludeStyle::MainIncludeCharDiscriminator &Value);
+};
+
 } // namespace yaml
 } // namespace llvm
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 7c2f4dcf3d2308..edb4acf9fe74df 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1032,6 +1032,7 @@ template <> struct MappingTraits<FormatStyle> {
     IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
     IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
     IO.mapOptional("Macros", Style.Macros);
+    IO.mapOptional("MainIncludeChar", Style.IncludeStyle.MainIncludeChar);
     IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
     IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);
     IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
@@ -1514,6 +1515,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
       {".*", 1, 0, false}};
   LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
   LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
+  LLVMStyle.IncludeStyle.MainIncludeChar = tooling::IncludeStyle::MICD_Quote;
   LLVMStyle.IndentAccessModifiers = false;
   LLVMStyle.IndentCaseLabels = false;
   LLVMStyle.IndentCaseBlocks = false;
diff --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
index d275222ac6b587..94f26e3ec80500 100644
--- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -234,8 +234,18 @@ int IncludeCategoryManager::getSortIncludePriority(StringRef IncludeName,
   return Ret;
 }
 bool IncludeCategoryManager::isMainHeader(StringRef IncludeName) const {
-  if (!IncludeName.starts_with("\""))
-    return false;
+  switch (Style.MainIncludeChar) {
+  case IncludeStyle::MICD_Quote:
+    if (!IncludeName.starts_with("\""))
+      return false;
+    break;
+  case IncludeStyle::MICD_Bracket:
+    if (!IncludeName.starts_with("<"))
+      return false;
+    break;
+  case IncludeStyle::MICD_Any:
+    break;
+  }
 
   IncludeName =
       IncludeName.drop_front(1).drop_back(1); // remove the surrounding "" or <>
diff --git a/clang/lib/Tooling/Inclusions/IncludeStyle.cpp b/clang/lib/Tooling/Inclusions/IncludeStyle.cpp
index da5bb00d1013a6..3e756f8ff0427d 100644
--- a/clang/lib/Tooling/Inclusions/IncludeStyle.cpp
+++ b/clang/lib/Tooling/Inclusions/IncludeStyle.cpp
@@ -28,5 +28,12 @@ void ScalarEnumerationTraits<IncludeStyle::IncludeBlocksStyle>::enumeration(
   IO.enumCase(Value, "Regroup", IncludeStyle::IBS_Regroup);
 }
 
+void ScalarEnumerationTraits<IncludeStyle::MainIncludeCharDiscriminator>::enumeration(
+    IO &IO, IncludeStyle::MainIncludeCharDiscriminator &Value) {
+  IO.enumCase(Value, "Quote", IncludeStyle::MICD_Quote);
+  IO.enumCase(Value, "Bracket", IncludeStyle::MICD_Bracket);
+  IO.enumCase(Value, "Any", IncludeStyle::MICD_Any);
+}
+
 } // namespace yaml
 } // namespace llvm
diff --git a/clang/test/Format/main-include-char-bracket-group.cpp b/clang/test/Format/main-include-char-bracket-group.cpp
new file mode 100644
index 00000000000000..3309b923ebed4d
--- /dev/null
+++ b/clang/test/Format/main-include-char-bracket-group.cpp
@@ -0,0 +1,27 @@
+// Test the combination of regrouped include directives, via regexes and
+// priorities, with a main header included with brackets.
+
+// RUN: clang-format %s -style="{MainIncludeChar: Bracket, IncludeBlocks: Regroup, IncludeCategories: [{Regex: 'lib-a', Priority: 1}, {Regex: 'lib-b', Priority: 2}, {Regex: 'lib-c', Priority: 3}]}" | FileCheck %s
+
+#include <lib-c/header-1.hpp>
+#include <lib-c/header-2.hpp>
+#include <lib-c/header-3.hpp>
+#include <lib-b/header-1.hpp>
+#include <lib-b/main-include-char-bracket-group.hpp>
+#include <lib-b/header-3.hpp>
+#include <lib-a/header-1.hpp>
+#include <lib-a/main-include-char-bracket-group.hpp>
+#include <lib-a/header-3.hpp>
+
+// CHECK: <lib-b/main-include-char-bracket-group.hpp>
+// CHECK-EMPTY:
+// CHECK-NEXT: <lib-a/header-1.hpp>
+// CHECK-NEXT: <lib-a/header-3.hpp>
+// CHECK-NEXT: <lib-a/main-include-char-bracket-group.hpp>
+// CHECK-EMPTY:
+// CHECK-NEXT: <lib-b/header-1.hpp>
+// CHECK-NEXT: <lib-b/header-3.hpp>
+// CHECK-EMPTY:
+// CHECK-NEXT: <lib-c/header-1.hpp>
+// CHECK-NEXT: <lib-c/header-2.hpp>
+// CHECK-NEXT: <lib-c/header-3.hpp>
diff --git a/clang/test/Format/main-include-char-quote-group.cpp b/clang/test/Format/main-include-char-quote-group.cpp
new file mode 100644
index 00000000000000..b8419761544133
--- /dev/null
+++ b/clang/test/Format/main-include-char-quote-group.cpp
@@ -0,0 +1,29 @@
+// Test the combination of regrouped include directives, via regexes and
+// priorities, with a main header included with quotes. Quotes for the main
+// header being the default behavior, the first test does not specify it.
+
+// RUN: clang-format %s -style="{IncludeBlocks: Regroup, IncludeCategories: [{Regex: 'lib-a', Priority: 1}, {Regex: 'lib-b', Priority: 2}, {Regex: 'lib-c', Priority: 3}]}" | tee delme | FileCheck %s
+// RUN: clang-format %s -style="{MainIncludeChar: Quote, IncludeBlocks: Regroup, IncludeCategories: [{Regex: 'lib-a', Priority: 1}, {Regex: 'lib-b', Priority: 2}, {Regex: 'lib-c', Priority: 3}]}" | FileCheck %s
+
+#include <lib-c/header-1.hpp>
+#include <lib-c/header-2.hpp>
+#include <lib-c/header-3.hpp>
+#include <lib-b/header-1.hpp>
+#include "lib-b/main-include-char-quote-group.hpp"
+#include <lib-b/header-3.hpp>
+#include <lib-a/header-1.hpp>
+#include <lib-a/main-include-char-quote-group.hpp>
+#include <lib-a/header-3.hpp>
+
+// CHECK: "lib-b/main-include-char-quote-group.hpp"
+// CHECK-EMPTY:
+// CHECK-NEXT: <lib-a/header-1.hpp>
+// CHECK-NEXT: <lib-a/header-3.hpp>
+// CHECK-NEXT: <lib-a/main-include-char-quote-group.hpp>
+// CHECK-EMPTY:
+// CHECK-NEXT: <lib-b/header-1.hpp>
+// CHECK-NEXT: <lib-b/header-3.hpp>
+// CHECK-EMPTY:
+// CHECK-NEXT: <lib-c/header-1.hpp>
+// CHECK-NEXT: <lib-c/header-2.hpp>
+// CHECK-NEXT: <lib-c/header-3.hpp>
diff --git a/clang/test/Format/main-include-char.cpp b/clang/test/Format/main-include-char.cpp
new file mode 100644
index 00000000000000..1835fc069f3597
--- /dev/null
+++ b/clang/test/Format/main-include-char.cpp
@@ -0,0 +1,15 @@
+// RUN: clang-format %s -style="{}" | FileCheck %s -check-prefix=QUOTE
+// RUN: clang-format %s -style="{MainIncludeChar: Quote}" | FileCheck %s -check-prefix=QUOTE
+// RUN: clang-format %s -style="{MainIncludeChar: Bracket}" | FileCheck %s -check-prefix=BRACKET
+
+#include <a>
+#include "quote/main-include-char.hpp"
+#include <bracket/main-include-char.hpp>
+
+// QUOTE: "quote/main-include-char.hpp"
+// QUOTE-NEXT: <a>
+// QUOTE-NEXT: <bracket/main-include-char.hpp>
+
+// BRACKET: <bracket/main-include-char.hpp>
+// BRACKET-NEXT: "quote/main-include-char.hpp"
+// BRACKET-NEXT: <a>

Copy link

github-actions bot commented Jan 19, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@j-jorge j-jorge force-pushed the format-main-header-bracket branch from db59f23 to 11c7a60 Compare January 19, 2024 18:14
@j-jorge j-jorge force-pushed the format-main-header-bracket branch 2 times, most recently from 7e06f6b to 6561598 Compare January 21, 2024 21:05
@j-jorge j-jorge force-pushed the format-main-header-bracket branch from 6561598 to b6284b7 Compare January 22, 2024 08:13
@j-jorge
Copy link
Contributor Author

j-jorge commented Feb 6, 2024

Hey, now that the PR has been reviewed, approved, and all checked as passed, do I have something else to do to have it merged?

@owenca owenca merged commit 984dd15 into llvm:main Feb 6, 2024
HazardyKnusperkeks added a commit to HazardyKnusperkeks/llvm-project that referenced this pull request Feb 8, 2024
llvm#78752 was not merged in time for clang-format 18
HazardyKnusperkeks added a commit that referenced this pull request Feb 9, 2024
#78752 was not merged in time for clang-format 18.
@owenca owenca removed the clang Clang issues not falling into any other category label May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants