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.

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

cmd/compile/internal/importer: flip enable alias to true [1.23 backport] #70517

Closed
xieyuschen opened this issue Nov 22, 2024 · 10 comments
Closed
Labels
CherryPickApproved Used during the release process for point releases
Milestone

Comments

@xieyuschen
Copy link
Contributor

xieyuschen commented Nov 22, 2024

This issue is created to request a backport at go1.23 to fix #70394. The issue doesn't exist in go1.24(master) as the code has refactored a bit before and fixed this issue.

The backport fix allows ureader to read exportdata with alias correctly and it doesn't affect the correctness of compile to construct exportdata for the following packages.

@timothy-king

@gopherbot gopherbot added this to the Go1.23.4 milestone Nov 22, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/629997 mentions this issue: [release-branch.go1.23] cmd/compile: enable alias for pkgReader

@dmitshur dmitshur added the CherryPickCandidate Used during the release process for point releases label Nov 22, 2024
@timothy-king
Copy link
Contributor

Thoughts for cherrypicking: Setting enableAlias: true, seems plausible. I would like to do more tests first though before weighing in on whether it is worth the risk.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/631855 mentions this issue: [release-branch.go1.23] cmd/compile/internal/importer: enable aliases

@timothy-king
Copy link
Contributor

My investigation makes me believe that this should be safe enough for 1.23. The CL to cherry pick would be https://go.dev/cl/631855.

@mvdan
Copy link
Member

mvdan commented Dec 4, 2024

When could I expect a 1.23.x release with this fix? I was hoping that would be the case with 1.23.4, given that the patch was approved last week, but unfortunately I see it's not the case.

@timothy-king
Copy link
Contributor

timothy-king commented Dec 4, 2024

@mvdan The patch has been approved on gerrit, but the status on the issue is CherryPickCandidate. For it to go forward, its must be CherryPickApproved. See https://go.dev/wiki/MinorReleases. (Yes this can be confusing.)

@mvdan
Copy link
Member

mvdan commented Dec 4, 2024

I see. Hopefully this can move along relatively quickly at this point, as I am unable to use alias tracking still.

@cagedmantis
Copy link
Contributor

This change has been approved as it is a problem without a viable workaround.

@cagedmantis cagedmantis added the CherryPickApproved Used during the release process for point releases label Dec 11, 2024
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Dec 11, 2024
gopherbot pushed a commit that referenced this issue Dec 11, 2024
Flips the pkgReader.enableAlias flag to true when reading unified IR.
This was disabled while resolving #66873. This resolves the TODO to
flip it back to true.

Fixes #70394
Fixes #70517
Updates #66873

Change-Id: Ifd52b0f9510d6bcf151de1c9a18d71ab548c14e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/604099
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 209ed1a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/631855
Commit-Queue: Tim King <taking@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
@gopherbot
Copy link
Contributor

Closed by merging CL 631855 (commit 69c8cfe) to release-branch.go1.23.

@mvdan
Copy link
Member

mvdan commented Dec 18, 2024

Hi again - is there perhaps a chance that 1.23.5 with this backport will happen before the holidays?

yangjuncode pushed a commit to yangjuncode/go that referenced this issue Dec 27, 2024
Flips the pkgReader.enableAlias flag to true when reading unified IR.
This was disabled while resolving golang#66873. This resolves the TODO to
flip it back to true.

Fixes golang#70394
Fixes golang#70517
Updates golang#66873

Change-Id: Ifd52b0f9510d6bcf151de1c9a18d71ab548c14e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/604099
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 209ed1a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/631855
Commit-Queue: Tim King <taking@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPickApproved Used during the release process for point releases
Projects
None yet
Development

No branches or pull requests

6 participants