Skip to content

[clang] Refactor to remove clangDriver dependency from clangFrontend and flangFrontend #165277

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[clang] Refactor to remove clangDriver dependency from clangFrontend and flangFrontend #165277

wants to merge 9 commits into from
+936 −681

Conversation

@naveen-seth
Copy link
Contributor

@naveen-seth naveen-seth commented Oct 27, 2025 •
edited
Loading

This removes the dependency on clangDriver from clangFrontend and flangFrontend

This refactoring is part of a broader effort to support driver-managed builds for compilations using C++ named modules and/or Clang modules.
It is required for linking the dependency scanning tooling against the driver without introducing cyclic dependencies, which would otherwise cause build failures when dynamic linking is enabled.
In particular, clangFrontend must no longer depend on clangDriver for this to be possible.

This change is motivated by the following review comment: #152770 (comment)

@naveen-seth naveen-seth requested a review from Bigcheese October 27, 2025 16:38
@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra lldb backend:ARM backend:AArch64 backend:AMDGPU backend:Hexagon backend:MIPS backend:RISC-V backend:PowerPC backend:Sparc backend:SystemZ backend:X86 clangd clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang:as-a-library libclang and C++ API llvm:mc Machine (object) code flang:driver flang Flang issues not falling into any other category backend:CSKY backend:SPIR-V backend:loongarch labels Oct 27, 2025
@naveen-seth naveen-seth mentioned this pull request Oct 31, 2025
Merged
@naveen-seth
Copy link
Contributor Author

naveen-seth commented Nov 1, 2025

I've tested this with BUILD_SHARED_LIBS=On for clang, clang-tools-extra, lldb and flang, including the CMake targets check-clang-unit, check-flang-unit, and check-lldb-unit.

kiranchandramohan
kiranchandramohan reviewed Nov 1, 2025
View reviewed changes
flang/lib/Frontend/CMakeLists.txt Outdated

CLANG_LIBS
clangBasic
clangDriver
Copy link
Contributor

@kiranchandramohan kiranchandramohan Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the title of the patch I was hoping that you could remove the clangDriver dependency from flangFrontend as well. Any reason you could not remove?

Copy link
Contributor Author

@naveen-seth naveen-seth Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, and thanks for pointing that out!

@naveen-seth naveen-seth force-pushed the removeDriverDepFromFrontend branch from e978ccd to 080ace2 Compare November 1, 2025 21:13
@naveen-seth naveen-seth changed the title [clang] Refactor to remove clangDriver dependency from clangFrontend [clang] Refactor to remove clangDriver dependency from clangFrontend and flangFrontentd Nov 1, 2025
tarunprabhu
tarunprabhu reviewed Nov 3, 2025
View reviewed changes
Copy link
Contributor

@tarunprabhu tarunprabhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes in flang as well. Those LGTM.

Since the majority of the changes are in clang, I'll defer to someone from there to approve.

@tarunprabhu
Copy link
Contributor

tarunprabhu commented Nov 4, 2025 •
edited
Loading

[EDIT]

Just noticed the typo in the title: flangFrontentd should be flangFrontend

  • My original message was: "Just noticed the type in the title: ...". Ah the irony!

@naveen-seth naveen-seth changed the title [clang] Refactor to remove clangDriver dependency from clangFrontend and flangFrontentd [clang] Refactor to remove clangDriver dependency from clangFrontend and flangFrontend Nov 4, 2025
@naveen-seth naveen-seth force-pushed the removeDriverDepFromFrontend branch 2 times, most recently from 26a940e to 93ede86 Compare November 7, 2025 17:30
@naveen-seth
[clang] Refactor to remove clangDriver dependency from clangFrontend
278df1d 
This PR removes the clangDriver dependency from clangFrontend.
The goal of this change is to remove dependencies on the Driver.

This refactoring is part of a broader effort to support driver-managed
builds for compilations using C++ named modules and/or Clang modules.
It is required for linking the dependency scanning tooling against the
driver without introducing cyclic dependencies, which would otherwise
cause build failures when dynamic linking is enabled.
In particular, clangFrontend must no longer depend on clangDriver for
this to be possible.

This change is motivated by the following review comment:
llvm#152770 (comment)
@naveen-seth
Add missing header
5b47d06
@naveen-seth
Add missing header for LLDB
68bc828
@naveen-seth
Fix StandaloneDiagnostic implementation
35c45ef
@naveen-seth
Fix translateStandaloneDiag
3d2861c
@naveen-seth
Improve comments and formatting
8072b45
@naveen-seth
Add release note under "Potentially Breaking Changes"
78ae453
@naveen-seth
Remove dependency on clangDriver from flangFrontend
59a24e1
@naveen-seth
Move GetResourcesPath from clangDriver into clangOptions
ad35d10
@naveen-seth naveen-seth force-pushed the removeDriverDepFromFrontend branch from 93ede86 to ad35d10 Compare November 11, 2025 18:57
@naveen-seth
Copy link
Contributor Author

naveen-seth commented Nov 11, 2025 •
edited
Loading

This is unblocked now that #163659 has been merged.

I've also tested the current version with BUILD_SHARED_LIBS=ON on Linux for clang, clang-tools-extra, lldb, and flang (including the corresponding check-*-unit targets).

@Bigcheese
Copy link
Contributor

Bigcheese commented Nov 12, 2025

This looks good overall, but do the changes to FilterAndStoreDiagnosticConsumer and StandaloneDiagnostic need to happen in this commit? I don't see any additional users of them here. This change is pretty big, so I'd like to not include anything unneeded.

@naveen-seth
Copy link
Contributor Author

naveen-seth commented Nov 12, 2025 •
edited
Loading

This looks good overall, but do the changes to FilterAndStoreDiagnosticConsumer and StandaloneDiagnostic need to happen in this commit? I don't see any additional users of them here. This change is pretty big, so I'd like to not include anything unneeded.

FilterAndStoreDiagnosticConsumer is needed in the newly moved CreateASTUnitFromCommandLine in Driver/CreateASTUnitFromCommandLine.cpp, but is also still required in Frontend/ASTUnit.cpp here.
To move FilterAndStoreDiagnosticConsumer into a header, we then also need to relocate StandaloneDiagnostic into a header.

Keeping StandaloneDiagnostic as a member of ASTUnit would reduce changes, but moving it allows deduplicating the StandaloneDiagnostic used in the dependency scanning and build-graph patch (the direct follow-up to this, #152770).
What would you prefer for StandaloneDiagnostic?

@Bigcheese
Copy link
Contributor

Bigcheese commented Nov 13, 2025

Ah. I missed this dependency when searching for uses of FilterAndStoreDiagnosticConsumer. That part of the change is fine then. Thanks.

@Bigcheese Bigcheese requested a review from MaskRay November 13, 2025 01:15
Bigcheese
Bigcheese approved these changes Nov 13, 2025
View reviewed changes
Copy link
Contributor

@Bigcheese Bigcheese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but @MaskRay should take a look as the Driver maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kiranchandramohan kiranchandramohan kiranchandramohan left review comments

@tarunprabhu tarunprabhu tarunprabhu left review comments

@Bigcheese Bigcheese Bigcheese approved these changes

@cyndyishida cyndyishida Awaiting requested review from cyndyishida

@rupprecht rupprecht Awaiting requested review from rupprecht

@keith keith Awaiting requested review from keith

@aaronmondal aaronmondal Awaiting requested review from aaronmondal

@JDevlieghere JDevlieghere Awaiting requested review from JDevlieghere JDevlieghere is a code owner

@DavidTruby DavidTruby Awaiting requested review from DavidTruby

@banach-space banach-space Awaiting requested review from banach-space

@tblah tblah Awaiting requested review from tblah

@MaskRay MaskRay Awaiting requested review from MaskRay

Assignees

No one assigned

Labels

backend:AArch64 backend:AMDGPU backend:ARM backend:CSKY backend:Hexagon backend:loongarch backend:MIPS backend:PowerPC backend:RISC-V backend:Sparc backend:SPIR-V backend:SystemZ backend:X86 bazel "Peripheral" support tier build system: utils/bazel clang:as-a-library libclang and C++ API clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category clang-tools-extra clangd flang:driver flang Flang issues not falling into any other category lldb llvm:mc Machine (object) code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@naveen-seth @llvmbot @tarunprabhu @Bigcheese @kiranchandramohan