toolchains_llvm icon indicating copy to clipboard operation
toolchains_llvm copied to clipboard

Support for injecting custom toolchain features

Open limdor opened this issue 1 month ago • 7 comments

There are situations where it is helpful to implement a toolchain feature for all toolchains that your project supports. In our case, this was used for implementing different warning levels/groups that is compiler agnostic. That allows us to have something like:

cc_library(
    name = "foo",
    srcs = ["foo.cpp"],
    hdrs = ["foo.h"],
    features = [
        "safety_related_warnings",
    ],
)

cc_library(
    name = "bar",
    srcs = ["bar.cpp"],
    hdrs = ["bar.h"],
    features = [
        "security_related_warnings",
    ],
)

The main question for the maintainers would be, are you interested in that feature? Do you see this as something that we could add to toolchains_llvm? If yes just add a comment here and I would start working right away.

limdor avatar Oct 30 '25 14:10 limdor

The short answer is yes, we absolutely want this, but the proper way to achieve that would probably be by porting toolchains_llvm over to the new rules-based C++ toolchain approach.

CC @thesayyn @keith @dzbarsky for their thoughts

fmeum avatar Oct 31 '25 08:10 fmeum

Understood. Let me know if a contribution could be made before toolchains_llvm fully switches to new rules-based C++ toolchain approach or not. I assume it would take some time to be fully on the new approach.

limdor avatar Oct 31 '25 08:10 limdor

That's true and I don't want to let perfect be the enemy of good here. If we find a way to make this possible that has a clear migration path, we could also decide to do it now.

fmeum avatar Oct 31 '25 09:10 fmeum

Ok, I will explore a bit and see what is best. If there is any work in progress for the new way that is not on master let me know. I could also check if I find time to contribute there.

limdor avatar Oct 31 '25 10:10 limdor

I definitely think the rules based toolchain can provide a much better API for doing something like this, which I'm very interested in. I haven't attempted to make that work yet though, there are still some subtle issues we have to worry about like ordering vs the defaults, but should be nicer with that approach for sure

keith avatar Nov 04 '25 00:11 keith

I took a first look and the problem at least without rules-based toolchains is that toolchains_llvm depends on cc_toolchain_config from rules_cc and that does not support injecting features. https://github.com/bazel-contrib/toolchains_llvm/blob/19dc6cf50de936407198f19e9ffc1af90aef01a7/toolchain/cc_toolchain_config.bzl#L17 https://github.com/bazelbuild/rules_cc/blob/b5a65591334f74371f4d75003768957a740cd868/cc/private/toolchain/unix_cc_toolchain_config.bzl#L1954 Even though the underlying create_cc_toolchain_config_info does support it https://github.com/bazelbuild/rules_cc/blob/b5a65591334f74371f4d75003768957a740cd868/cc/private/toolchain/unix_cc_toolchain_config.bzl#L1936 This is inside private in rules_cc but I assume that this was agreed with rules_cc authors. To me it makes sense that specific toolchains can make use of unix_cc_toolchain_config.bzl that is nothing else than a configurable template. I think that whatever it is decided, needs to be agreed with rules_cc maintainers. Because supporting injecting of features without support from rules_cc I do not think it is worth it, otherwise other C++ toolchains would have the same problem. I will create a ticket in rules_cc to discuss this topic.

limdor avatar Nov 04 '25 13:11 limdor

Here is a draft on how the injection in rules_cc for legacy toolchains could look like. If anyone here has an opinion please just comment on it. In the end toolchains_llvm would have to use such API.

https://github.com/bazelbuild/rules_cc/pull/523

limdor avatar Nov 12 '25 15:11 limdor