add multiverse lookup to Type #309

ziyue-101 · 2025-06-25T23:32:32Z

Fixes: #298

linux-foundation-easycla · 2025-06-25T23:32:36Z

The committers listed above are authorized under a signed CLA.

✅ login: ziyue-101 / name: ziyueyan (9dec20c)

k8s-ci-robot · 2025-06-25T23:32:41Z

Welcome @ziyue-101!

It looks like this is your first PR to kubernetes/gengo 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/gengo has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-06-25T23:32:42Z

Hi @ziyue-101. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

ziyue-101 · 2025-06-25T23:37:32Z

/assign @thockin @yongruilin

jpbetz · 2025-06-25T23:49:40Z

Thanks @ziyue-101! Can you please add tests for this?

k8s-ci-robot · 2025-06-26T20:49:09Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ziyue-101
Once this PR has been reviewed and has the lgtm label, please ask for approval from thockin. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ziyue-101 · 2025-06-26T20:51:08Z

@jpbetz
I added some unit test, Let me know if they look ok!

yongruilin · 2025-06-26T23:14:16Z

/ok-to-test

thockin

I think this can be simpler still.

v2/types/types.go

v2/parser/parse.go

v2/types/types.go

k8s-ci-robot · 2025-07-01T18:54:50Z

@ziyue-101: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gengo-verify	`9dec20c`	link	true	`/test pull-gengo-verify`
pull-gengo-test	`9dec20c`	link	true	`/test pull-gengo-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

thockin · 2025-07-02T05:16:20Z

v2/types/types.go

+// generation. For example the imported code my have T but not *T, while the generated code needs *T. This
+// can't be part of the main Universe because that is a simple map, with no locking, and we all know what happens
+// when you modify a map while it is being iterated. Storing the multiverse alongside a Type ensures that there's
+// at most one *Type for every type, and maintains the invariant that PointerTo(String) == PointerTo(String).


Super not since I am leaving other comments. s/String/T in the invariant

thockin · 2025-07-02T05:16:58Z

v2/types/types.go

+
+// InitMultiverse inits a multiverse for a Type.
+// It panics if called twice.
+func (t *Type) InitMultiverse(u Universe) {


Just call it Init -- multiverse is an implementation detail, it doesn't need to leak into the API

thockin · 2025-07-02T05:18:09Z

v2/types/types.go

+// It panics if called twice.
+func (t *Type) InitMultiverse(u Universe) {
+	if t.multiverse != nil {
+		panic("Can't initialize a non-empty multiverse on Type")


"can't re-initialize type %v"...

thockin · 2025-07-02T05:22:33Z

v2/types/types_test.go

+		multiverse: &multiverse{
+			real: u,
+			synthetic: map[string]*Type{
+				"*pkgname.thridstructname": &Type{


s/thrid/third

Or just call these T1, T2, T3 :)

thockin · 2025-07-02T05:30:48Z

v2/parser/parse.go

 func (p *Parser) walkType(u types.Universe, useName *types.Name, in gotypes.Type) *types.Type {
+	var out *types.Type
+	defer func() {
+		out.InitMultiverse(u)


I just realized someone needs to call this on all of the builtin types. This means that there can be only 1 Universe because the builtins are global variables, which have a single multiverse pointer each. That makes me sad.

Can you think of any alternative that is not so limited?

Is the cure worse than the sickness? I don't like that PointerTo(T) != PointerTo(T) but if there were two universes active T != T, anyway...

Have to think on this...

Revisiting this - I still don't know what we should do.

Large-scale redesign of gengo apps (e.g. making the Universe the central abstraction, passing it everywhere, so instead of types.String you might use u.String() or u.PointerTo(t)) seems unlikely.

Making a global universe (so there can't be more than one) seems plausible but amounts to gengo v3, and I don't see anyone working on that right now, especially without some other driver.

We could pre-populate a t.Pointer up to some level of depth (e.g. ***T) and thast "should be enough for anyone" but requires more gengo work.

I'm letting this sit for now until it is clearer what we need.

k8s-triage-robot · 2025-09-30T05:49:57Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

thockin · 2025-09-30T17:14:41Z

/remove-lifecycle stale
/remove-lifecycle rotten

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jun 25, 2025

k8s-ci-robot requested review from alexzielenski and jpbetz June 25, 2025 23:32

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 25, 2025

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 25, 2025

k8s-ci-robot assigned thockin and yongruilin Jun 25, 2025

ziyue-101 mentioned this pull request Jun 25, 2025

PointerTo() produces different results for repeat invocations #298

Open

ziyue-101 force-pushed the pointer-to branch from e9adae9 to f6cb864 Compare June 26, 2025 20:49

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 26, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 26, 2025

thockin reviewed Jun 27, 2025

View reviewed changes

v2/types/types.go Outdated Show resolved Hide resolved

v2/types/types.go Outdated Show resolved Hide resolved

v2/types/types.go Outdated Show resolved Hide resolved

v2/parser/parse.go Outdated Show resolved Hide resolved

ziyue-101 force-pushed the pointer-to branch 2 times, most recently from 41c00a5 to 7c1013b Compare June 30, 2025 20:04

ziyue-101 changed the title ~~add cache and reverse pointer to Universe to Type~~ add multiverse lookup to Type Jun 30, 2025

yongruilin reviewed Jun 30, 2025

View reviewed changes

v2/types/types.go Outdated Show resolved Hide resolved

v2/types/types.go Show resolved Hide resolved

thockin reviewed Jul 1, 2025

View reviewed changes

add multiverse lookup to Type

9dec20c

ziyue-101 force-pushed the pointer-to branch from 7c1013b to 9dec20c Compare July 1, 2025 18:51

thockin reviewed Jul 2, 2025

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 30, 2025

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 30, 2025

Uh oh!

add multiverse lookup to Type #309

Are you sure you want to change the base?

add multiverse lookup to Type #309

Conversation

ziyue-101 commented Jun 25, 2025

Uh oh!

linux-foundation-easycla bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jun 25, 2025

Uh oh!

k8s-ci-robot commented Jun 25, 2025

Uh oh!

ziyue-101 commented Jun 25, 2025

Uh oh!

jpbetz commented Jun 25, 2025

Uh oh!

k8s-ci-robot commented Jun 26, 2025

Uh oh!

ziyue-101 commented Jun 26, 2025

Uh oh!

yongruilin commented Jun 26, 2025

Uh oh!

thockin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

k8s-ci-robot commented Jul 1, 2025

Uh oh!

thockin Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

thockin Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

thockin Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

thockin Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

thockin Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

thockin Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

k8s-triage-robot commented Sep 30, 2025

Uh oh!

thockin commented Sep 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

linux-foundation-easycla bot commented Jun 25, 2025 •

edited

Loading