Add Clang -ftime-trace support to rules_cc

[PikachuHyA]

Declare trace JSON files as compile action outputs and expose them via the trace_files output group, enabled with features = ["trace"] on Clang toolchains.

Fix bazelbuild/bazel#9047

[PikachuHyA]

i think the failed case is not related with this PR.

https://buildkite.com/bazel/rules-cc/builds/5543/canvas?sid=019f030e-cfd4-495f-888e-9f123c072e07&tab=output

@fmeum could you take a look?

[fmeum]

I don't know why this happens. @trybka

[lilygorsheneva]

Slowki in bazelbuild/bazel#9047 mentions using an aspect to collect this (as does an internal project), this makes me unsure whether it needs to be in the rules (though it should be @trybka or @pzembrod 's call)

[PikachuHyA]

Slowki in bazelbuild/bazel#9047 mentions using an aspect to collect this (as does an internal project), this makes me unsure whether it needs to be in the rules (though it should be @trybka or @pzembrod 's call)

@lilygorsheneva Thanks for flagging this
I considered the aspect-based approach before but don't think it's a viable full replacement, for two reasons:

1. An aspect would be fragile for C++20 Modules. The current implementation in _create_cc_compile_actions_with_cpp20_module is not a flat "one source → one object" flow. rules_cc creates c++-module-deps-scanning actions, aggregates .ddi files into modules info, generates per-source .modmap / .modmap.input files, distinguishes module interface sources from normal compilation units, wires direct/transitive .pcm module files, and passes module-specific build variables into _create_compile_source_action. An external aspect would have to duplicate that internal wiring to correctly instrument module builds, and would be very easy to break as the modules pipeline evolves.

2. It also pushes a compatibility burden onto users. Bazel 9 moves C++ compilation onto rules_cc's own Starlark internals, so an aspect written for the pre-9 native C++ rules may need additional adaptation for Bazel 9. Implementing this inside rules_cc keeps the compatibility story simpler: as long as the user's rules_cc version is compatible with their Bazel version, trace collection should work normally. With an external aspect, users may need to handle version-specific differences themselves, so it is less likely to work out of the box across Bazel / rules_cc versions.

For example, to make bazelbuild/bazel#9047 (comment) work for bazel 9, the patch needed.

diff --git a/MODULE.bazel b/MODULE.bazel
new file mode 100644
index 0000000..133dd0f
--- /dev/null
+++ b/MODULE.bazel
@@ -0,0 +1,4 @@
+module(name = "clang_aspects")
+
+bazel_dep(name = "rules_cc", version = "0.2.17")
+bazel_dep(name = "bazel_skylib", version = "1.9.0")
diff --git a/clang_time_trace.bzl b/clang_time_trace.bzl
index b88be59..a6db9e8 100644
--- a/clang_time_trace.bzl
+++ b/clang_time_trace.bzl
@@ -1,6 +1,8 @@
 load("@bazel_skylib//lib:paths.bzl", "paths")
 load("@rules_cc//cc:action_names.bzl", "ACTION_NAMES")
 load("@rules_cc//cc:find_cc_toolchain.bzl", "find_cc_toolchain", "use_cc_toolchain")
+load("@rules_cc//cc/common:cc_common.bzl", "cc_common")
+load("@rules_cc//cc/common:cc_info.bzl", "CcInfo")

 def _clang_time_trace_aspect_impl(target, ctx):
     # Get the compile commands

Given that -ftime-trace needs to be attached to the actual compile actions constructed by rules_cc, I think keeping this in rules_cc is less fragile than requiring users to maintain an external aspect that mirrors those internals. Happy to defer to @trybka / @pzembrod on the final call.

[trybka]

I don't understand the Android failures but they certainly seem unrelated. @keith any thoughts?

Re: aspect vs. not

We currently use an aspect for this internally ourselves.
I don't know that we want to go down the path of adding bespoke logic for every compiler/linker feature that emits another file. (One could imagine adding link support for --cref -Map, for instance).

I wonder if there's a better way to define a feature with a generic additional output file that gets fed in with a build variable, then we don't need the rules to handle every permutation, just a magic feature with a specific build variable.

That might require that they have unique names though.

[trybka]

is ftime-trace clang specific?

[13steinj]

AFAIK yes, there's a GCC plugin that gives some advanced metrics but I believe it's kinda defunct.

https://github.com/royjacobson/externis

[PikachuHyA]

is ftime-trace clang specific?

Yes, -ftime-trace is Clang-specific.

I also asked some colleagues who have used GCC, and none of them have used this repo:
https://github.com/royjacobson/externis

[13steinj]

I've used it, it's not the best. Someone else has also made other time-trace plugins.

I agree with @keith and @lilygorsheneva ; it would generally be good to have a generic way to add functionality to ancillary data generation and collection (there's also --time-trace in in lld.

[trybka]

do we need to track the .pic files separately, or could we bundle them together?

[PikachuHyA]

I followed the existing gcno_files / pic_gcno_files pattern, but if there is no need to preserve the distinction for traces, I can merge them into one list and keep the output group unchanged.

[keith]

android stuff is fixed now so we can ignore

[keith]

IMO we should support someway for toolchains to add files like this in a generic way. we have had quite a few cases of this overtime and I think it would be nice to not have to add them 1 by 1.

rules_cc/cc/private/toolchain/unix_cc_toolchain_config.bzl

Line 1557 in ad214a9

flags = ["--serialize-diagnostics", "%{serialized_diagnostics_file}"],

this is another case

[lilygorsheneva]

IMO we should support someway for toolchains to add files like this in a generic way. we have had quite a few cases of this overtime and I think it would be nice to not have to add them 1 by 1.

rules_cc/cc/private/toolchain/unix_cc_toolchain_config.bzl

Line 1557 in ad214a9

flags = ["--serialize-diagnostics", "%{serialized_diagnostics_file}"],

this is another case

Could we have the toolchain provide semi-arbitrary (we'd need to define what context objects we can pass) starlark functions to the rules?

[trybka]

Can you also sync / remerge?

[trybka]

can you inline the _clang_trace enabled function here rather than pass through the enable_trace parameter? (we have the feature configuration here)

[PikachuHyA]

done

[PikachuHyA]

@lilygorsheneva some error on copybara, could you take a look?