Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/cmd/bundle: fails to rename references to embedded generic structs #60647

Open
norbs57 opened this issue Jun 6, 2023 · 4 comments
Open

x/tools/cmd/bundle: fails to rename references to embedded generic structs #60647

norbs57 opened this issue Jun 6, 2023 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@norbs57
Copy link

norbs57 commented Jun 6, 2023 •
edited

What version of Go are you using?

go version go1.20.5 linux/amd64
x/tools/bundle version: v0.9.3

What did you do?

I ran the command bundle -o bundled.go ./lib where lib is a folder containing a single file with this code (see link)

What did you expect to see?

I expected bundle to produce this code (see link). Note in particular line 22:

	x := s.lib_Stack.Pop()

What did you see instead?

bundle produced this code (see link) which does not compile:

./prog.go:22:9: s.Stack undefined (type *lib_StackTwo[T] has no field or method Stack)

The problem is that bundle failed to rename the field selector referring to the embedded generic struct Stack in line 22:

	x := s.Stack.Pop()

I have had similar problems running bundle on other programs featuring embedded generic structs.
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jun 6, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jun 6, 2023
@dr2chase
Copy link
Contributor

dr2chase commented Jun 8, 2023

@adonovan @golang/tools-team

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 8, 2023
@adonovan
Copy link
Member

adonovan commented Jun 8, 2023

I suspect this TypeName pointer comparison is the issue. We'll need to adapt it to test whether two types have the same generic declaration, even if one of them is an instantiation of the other. A quick workaround might be to check something like obj == from || is[*types.TypeName](obj) && obj.Pos() == from.Pos() instead.

@norbs57
Copy link
Author

norbs57 commented Jun 9, 2023

Thanks. I have implemented a variation of the suggested workaround that seems to do the trick. I would be happy to share it if you think it could be of interest. Would it be ok to put the modified source code of bundle on the Go playground?

@adonovan
Copy link
Member

I have implemented a variation of the suggested workaround that seems to do the trick. I would be happy to share it if you think it could be of interest.

Thanks, please do share it. The best way to share patches is by sending a Gerrit CL or GitHub PR, whichever is easiest. You can find instructions for both here: https://go.dev/doc/contribute#sending_a_change_github.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@dr2chase @adonovan @gopherbot @norbs57