These libraries are only ever used in clang-tidy itself, so there is no
need to unconditionally keep all symbols.
Details
- Reviewers
njames93 GMNGeoffrey jathu - Group Reviewers
Restricted Project - Commits
- rG004c76155cda: [bazel] Don't alwayslink clang-tidy libraries
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I would've thought it was necessary to alwayslink them because maybe they use some kind of auto-registration system into clang-tidy, so without alwayslink your clang-tidy binary wouldn't link any of them in? But I could be wrong, maybe they're explicitly referenced in the clang-tidy binary?
I think the reason alwayslink was in here was because https://reviews.llvm.org/D143804 also took https://github.com/eomii/rules_ll/commit/678b1e4698aca2c8d5767c95e5245e60921754e into consideration, which originally copied the general structure from https://github.com/llvm/llvm-project/blob/main/utils/bazel/llvm-project-overlay/llvm/cc_plugin_library.bzl.
As far as I can remember this was just an oversight in the original implementation. I wasn't aware of the implications of alwayslink until @GMNGeoffrey mentioned it in https://reviews.llvm.org/D143804. But the clang-tidy "modules" don't seem to work like plugins and a Bazel-built clang-tidy executable seems to work fine without this flag.
But I could be wrong, maybe they're explicitly referenced in the clang-tidy binary?
I think this is what the ClangTidyForceLinker.h file (also included by ClangTidyMain.cpp) does.
There is no always-link in cmake as far as I know, so we should not need any in Bazel either in the LLVM project hopefully.
There is no always-link in cmake as far as I know, so we should not need any in Bazel either in the LLVM project hopefully.
It seems that only mlir actually needs this and there are custom build files for that.
The only actual usage of the cc_plugin_library which explicitly exposes alwayslink is for the libclang target, but its left to default (False) there. I think it should be fine to remove alwayslink from the llvm and clang build files entirely.
Ah, fair enough. It was just a vague guess at why this might've been included in the first place. If it doesn't apply - no worries.
Hmmm actually our internal version of these build files also uses alwayslink with the comment "Registration of the clang-tidy checker modules relies on the linker, so we need to enforce linking". That comment was written in 2014 though, so things may have changed. Let me try seeing what tests fail if I remove that internally.
I tried removing it internally and nothing fails, so I think this is vestigial. Let's kill it!