Skip to content

Conversation

@slackito
Copy link
Collaborator

The Frontend library depends on Serialization. This is an explicit dependency encoded in the CMake target. However, Serialization currently has an implicit dependency on Frontend, as it includes one of its headers. This is not reflected in the CMake build rules, but Bazel is stricter so, in order to avoid a dependency cycle, it hackily declares the Frontend headers as source files for Serialization.

Fortunately, the only Frontend header used by Serialization is clang/Frontend/FrontendDiagnostic.h, which is a legacy header that just includes clang/Basic/DiagnosticFrontend since d076608, back in 2018.

This commit changes Serialization to use the underlying header from Basic instead. Both Serialization and Frontend depend on Basic, so this breaks the dependency cycle.

…tion (NFC)

The Frontend library depends on Serialization. This is an explicit
dependency encoded in the CMake target. However, Serialization currently
has an implicit dependency on Frontend, as it includes one of its
headers. This is not reflected in the CMake build rules, but Bazel is
stricter so, in order to avoid a dependency cycle, it hackily declares
the Frontend headers as source files for Serialization.

Fortunately, the only Frontend header used by Serialization is
clang/Frontend/FrontendDiagnostic.h, which has been just a legacy header
that includes clang/Basic/DiagnosticFrontend since
d076608, back in 2018.

This commit changes Serialization to use the underlying header from
Basic instead. Both Serialization and Frontend depend on Basic, so this
breaks the dependency cycle.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules bazel "Peripheral" support tier build system: utils/bazel labels Jan 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 15, 2025

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Jorge Gorbe Moya (slackito)

Changes

The Frontend library depends on Serialization. This is an explicit dependency encoded in the CMake target. However, Serialization currently has an implicit dependency on Frontend, as it includes one of its headers. This is not reflected in the CMake build rules, but Bazel is stricter so, in order to avoid a dependency cycle, it hackily declares the Frontend headers as source files for Serialization.

Fortunately, the only Frontend header used by Serialization is clang/Frontend/FrontendDiagnostic.h, which is a legacy header that just includes clang/Basic/DiagnosticFrontend since d076608, back in 2018.

This commit changes Serialization to use the underlying header from Basic instead. Both Serialization and Frontend depend on Basic, so this breaks the dependency cycle.


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

2 Files Affected:

  • (modified) clang/lib/Serialization/GeneratePCH.cpp (+1-1)
  • (modified) utils/bazel/llvm-project-overlay/clang/BUILD.bazel (-5)
diff --git a/clang/lib/Serialization/GeneratePCH.cpp b/clang/lib/Serialization/GeneratePCH.cpp
index a3189bb40b1912..12751beb8d7157 100644
--- a/clang/lib/Serialization/GeneratePCH.cpp
+++ b/clang/lib/Serialization/GeneratePCH.cpp
@@ -12,7 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTContext.h"
-#include "clang/Frontend/FrontendDiagnostic.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/Preprocessor.h"
diff --git a/utils/bazel/llvm-project-overlay/clang/BUILD.bazel b/utils/bazel/llvm-project-overlay/clang/BUILD.bazel
index a86c295b04cb10..97445d99dc3784 100644
--- a/utils/bazel/llvm-project-overlay/clang/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/clang/BUILD.bazel
@@ -2148,7 +2148,6 @@ cc_library(
         "include/clang/Serialization/AttrPCHRead.inc",
         "include/clang/Serialization/AttrPCHWrite.inc",
     ] + glob([
-        "include/clang/Frontend/*.h",
         "lib/Serialization/*.cpp",
         "lib/Serialization/*.h",
     ]),
@@ -2160,15 +2159,11 @@ cc_library(
         "include/clang/Serialization/*.def",
     ]),
     deps = [
-        ":apinotes",
         ":ast",
         ":basic",
-        ":driver",
         ":lex",
         ":sema",
         ":serialization_attr_gen",
-        ":static_analyzer_core_options",
-        ":support",
         ":type_nodes_gen",
         "//llvm:BitReader",
         "//llvm:BitWriter",

@slackito slackito merged commit d2d531e into llvm:main Jan 16, 2025
12 checks passed
@slackito slackito deleted the bazel-dependency-cleanup branch January 16, 2025 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bazel "Peripheral" support tier build system: utils/bazel clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants