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

all: update release-branch.go1.20 with changes for Go 1.20 final #57854

Closed
cherrymui opened this issue Jan 17, 2023 · 16 comments
Closed

all: update release-branch.go1.20 with changes for Go 1.20 final #57854

cherrymui opened this issue Jan 17, 2023 · 16 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker umbrella
Milestone
Go1.20

Comments

@cherrymui
Copy link
Member

The release-branch.go1.20 branch has updated to commit 9088c69 at the master branch. The master branch is then soft reopen for 1.21 development. At this point, any changes targeting Go 1.20 release need to be explicitly cherry-picked to the release-branch.go1.20 branch.

This is the tracking umbrella issue to collect the remaining CLs that need to land on the release branch, and sort them all.
@cherrymui cherrymui added this to the Go1.20 milestone Jan 17, 2023
@cherrymui cherrymui added NeedsFix The path to resolution is known, but the work has not been done. umbrella labels Jan 17, 2023
@mdempsky
Copy link
Member

mdempsky commented Jan 17, 2023 •
edited by heschi

https://go.dev/cl/461686 should go into 1.20. It's just been reviewed. I think it's ready to merge and backport, pending confirmation on the approval process for that.

release team: done in https://go.dev/cl/462535

@gopherbot
Copy link

Change https://go.dev/cl/462535 mentions this issue: cmd/compile: fix static init inlining for hidden node fields

@gopherbot
Copy link

Closed by merging 9eed826 to release-branch.go1.20.

gopherbot pushed a commit that referenced this issue Jan 17, 2023
@mdempsky @gopherbot
[release-branch.go1.20] cmd/compile: fix static init inlining for hid…
9eed826 
…den node fields

Unified IR added several new IR fields for holding *runtime._type
expressions. To avoid throwing off any frontend semantics
(particularly inlining cost heuristics), they were marked as
`mknode:"-"` so that code wouldn't visit them.

Unfortunately, this has a bad interaction with the static init
inlining optimization, because the latter relies on ir.EditChildren to
substitute all parameters. This potentially includes dictionary
parameters, which can appear within the new RType fields.

This CL adds a new ir.EditChildrenWithHidden function that also edits
these fields, and switches staticinit to use it. Longer term, we
should unhide the RType fields so that ir.EditChildren visits them
normally, but that's scarier so late in the release cycle.

Updates #57778.
Updates #57854.

Change-Id: I98c1e8cf366156dc0c81a0cb79029cc5e59c476f
Reviewed-on: https://go-review.googlesource.com/c/go/+/461686
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
(cherry picked from commit 9f2fbedf010d59c3ecaa8c25b07a5f68fcb2e3d5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/462535
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@cherrymui cherrymui reopened this Jan 17, 2023
@cherrymui cherrymui reopened this Jan 17, 2023
@cuonglm
Copy link
Member

cuonglm commented Jan 18, 2023 •
edited by heschi

CL https://go-review.googlesource.com/c/go/+/461759 should go into 1.20, it's reviewed and waiting for trybot.

release team: done in http://go.dev/cl/461760

@gopherbot
Copy link

Change https://go.dev/cl/461760 mentions this issue: [release-branch.go1.20] cmd/compile: fix unsafe.{SliceData,StringData} escape analysis memory corruption

@bcmills
Copy link
Contributor

bcmills commented Jan 18, 2023 •
edited by cherrymui

#57903 too. (Probably https://go.dev/cl/462555?)

release team: done in http://go.dev/cl/462695

@heschi heschi mentioned this issue Jan 18, 2023
Closed
gopherbot pushed a commit that referenced this issue Jan 18, 2023
@cuonglm @cherrymui
[release-branch.go1.20] cmd/compile: fix unsafe.{SliceData,StringData…
d7c6da8 
…} escape analysis memory corruption

Updates #57823
Updates #57854

Change-Id: I54654d3ecb20b75afa9052c5c9db2072a86188d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/461759
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/461760
@anitgandhi
Copy link
Contributor

anitgandhi commented Jan 20, 2023 •
edited by heschi

Hello!

Hopefully I'm not asking out of line - can CL https://go-review.googlesource.com/c/go/+/410496 be included in 1.20?
It was reviewed and approved a while ago but just got merged today. I'm not sure if it constitutes too much of a behavior change to me included this late in the cycle.
If it can be included, that would be awesome; it would be a huge help for our organization.

Thanks in advance!

release team: declined

@cherrymui
Copy link
Member Author

That CL addresses #52113, which is not in Go1.20 milestone and it seems the issues has existed for a long time. So it seems not critical to be included this late in the cycle. At this point we weigh more on stability than feature improvements. If you think it is critical, feel free to comment. Thanks for understanding.

@bcmills
Copy link
Contributor

bcmills commented Jan 20, 2023

https://go.dev/cl/459715 should be backported to the release branch.

(I had thought it would be submitted before the branch was even cut, but apparently it's been sitting in the needs-review queue for ~10 days?)

@cherrymui
Copy link
Member Author

Hmmm, I was doing +1s for the needs-review queue this week and I never saw that CL. Not sure what made it not included in the queue...

@cherrymui cherrymui mentioned this issue Jan 20, 2023
Closed
@thanm
Copy link
Contributor

thanm commented Jan 20, 2023

I'd like to request that https://go.dev/cl/462955 be cherry-picked onto the 1.20 release branch. It is a very low-risk change, and it fixes problems with incorrect coverage data reports (no easy workaround).

@randall77
Copy link
Contributor

Please also backport https://go-review.googlesource.com/c/go/+/463295

@mdempsky
Copy link
Member

@thanm I see https://go.dev/cl/462756 is in that CL stack. Does it need to be cherry picked to 1.20 too, or just 462955?

@randall77 SGTM.

@heschi
Copy link
Contributor

heschi commented Jan 25, 2023

cc @golang/release

@thanm
Copy link
Contributor

thanm commented Jan 25, 2023

@mdempsky you asked about https://go.dev/cl/462756 , ideally it would be nice to have that as well. It is less serious but is also very low risk. Thanks.

@mknyszek
Copy link
Contributor

We're approaching the release and all the above CLs seem to be in. I'm going to close this issue assuming nothing else is going in. Please comment and reopen if that's not the case. Thanks!

@heschi heschi mentioned this issue Mar 1, 2023
Closed
7 tasks
@golang golang locked and limited conversation to collaborators Jan 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker umbrella
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
10 participants
@mdempsky @mknyszek @anitgandhi @cuonglm @bcmills @randall77 @gopherbot @thanm @cherrymui @heschi