Skip to content

Use Starlark tuples for C++ object-file groups - #814

Open
dzbarsky wants to merge 1 commit into
bazelbuild:mainfrom
dzbarsky:codex/starlarkify-object-file-groups
Open

Use Starlark tuples for C++ object-file groups#814
dzbarsky wants to merge 1 commit into
bazelbuild:mainfrom
dzbarsky:codex/starlarkify-object-file-groups

Conversation

@dzbarsky

Copy link
Copy Markdown
Contributor

Store private _ObjectFileGroupInfo.object_files collections as immutable Starlark tuples instead of Java-backed _cc_internal.freeze lists. Tuple iteration preserves object order, generated-directory expansion, and whole-archive grouping while removing the module's remaining cc_internal dependency.

Validation: buildifier -mode=check and 30 passing link-variable and Linux, macOS, and Windows toolchain-feature analysis tests.

@dzbarsky
dzbarsky marked this pull request as ready for review July 31, 2026 18:26
@trybka

trybka commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

@lberki can you take a quick look at this one? I know you added some of these freezes.
IIRC it was for mutability which the tuple should address, but just wanted to get another pair of 👀 .

@lilygorsheneva

Copy link
Copy Markdown
Collaborator

Ran some benchmarks on this and #809. Effects range from negligible to 0.05% increases in Post-Analysis JVM Heap Memory (nothing that should trip alerts).

@lberki

lberki commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

From my POV, this works; all I care about is that these values are immutable, and a frozen list and a tuple both are good enough for the job. I guess lists fit the use case "philosophically" more, since the arity of a tuple is part of its type and with this change, you are effectively introducing a field whose type is a tuple of unknown arity.

That said, it's appealing to remove the cc_internal dependency. @pzembrod , which approach do you prefer?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants