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

Jump to bottom

cmd/compile: sparse slices with struct items are not initialized [1.12 backport] #32013

Closed
gopherbot opened this issue May 13, 2019 · 5 comments
Closed

cmd/compile: sparse slices with struct items are not initialized [1.12 backport] #32013

gopherbot opened this issue May 13, 2019 · 5 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.12.6

Comments

@gopherbot
Copy link

@randall77 requested issue #31987 to be considered for backport to the next 1.12 minor release.

@gopherbot Please open a backport to 1.12.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label May 13, 2019
@gopherbot gopherbot mentioned this issue May 13, 2019
Closed
@gopherbot gopherbot added this to the Go1.12.6 milestone May 13, 2019
@randall77
Copy link
Contributor

This bug causes incorrect code generation for some slice literals, with keyed entries, with values which are statically computable structs or arrays. There's no easy workaround.
The bug was introduced in 1.12, 1.11 is ok.

@gopherbot
Copy link
Author

Change https://golang.org/cl/177040 mentions this issue: cmd/compile: make sure to initialize static entries of slices

@FiloSottile FiloSottile added the CherryPickApproved Used during the release process for point releases label May 14, 2019
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label May 14, 2019
@gopherbot
Copy link
Author

Closed by merging ecf7695 to release-branch.go1.12.

gopherbot pushed a commit that referenced this issue May 14, 2019
@randall77 @FiloSottile
[release-branch.go1.12] cmd/compile: make sure to initialize static e…
ecf7695 
…ntries of slices

If a slice's entries are sparse, we decide to initialize it dynamically
instead of statically. That's CL 151319.

But if we do initialize it dynamically, we still need to initialize
the static entries. Typically we do that, but the bug fixed here is
that we don't if the entry's value is itself an array or struct.

To fix, use initKindLocalCode to ensure that both static and
dynamic entries are initialized via code.

Fixes #32013

Change-Id: I1192ffdbfb5cd50445c1206c4a3d8253295201dd
Reviewed-on: https://go-review.googlesource.com/c/go/+/176904
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit a9e107c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/177040
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@dolmen
Copy link
Contributor

dolmen commented Jun 12, 2019

@randall77 In https://golang.org/cl/177040, a complex testcase has been proposed by the reviewer. I don't see it added in the codebase. Is there a reason why this non-regression check has not been added?

@randall77
Copy link
Contributor

@dolmen: I'm confused. There's no such complex test case proposed in that CL.
Do you mean Josh's test case in https://go-review.googlesource.com/c/go/+/176904/ ?
There's no reason it wasn't added. You can make a CL with it if you'd like.

@golang golang locked and limited conversation to collaborators Jun 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.12.6
Development

No branches or pull requests
4 participants
@dolmen @FiloSottile @randall77 @gopherbot