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 compiler error: expected struct { F uintptr; L *lua.Lua_State } value to have type struct { F uintptr; L *lua.Lua_State } [1.21 backport] #62545

Closed
gopherbot opened this issue Sep 9, 2023 · 5 comments
Assignees
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone

Comments

@gopherbot
Copy link

@mdempsky requested issue #62498 to be considered for backport to the next 1.21 minor release.

Here's a standalone repro of the issue: https://go.dev/play/p/5eDYV-mzqg9

There's nothing wrong with this code, and it compiled in Go 1.20. So I think this is a regression that should be backported.

@gopherbot Please backport to Go 1.21. This is a compilation failure of valid code that used to work in Go 1.20.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Sep 9, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Sep 9, 2023
@gopherbot gopherbot added this to the Go1.21.2 milestone Sep 9, 2023
@joedian joedian added the CherryPickApproved Used during the release process for point releases label Sep 20, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Sep 20, 2023
@thanm
Copy link
Contributor

thanm commented Sep 25, 2023

Hi @mdempsky wondering what that status of this bug is -- if it is going to be included in Go 1.21.2 we'll need to have the cherry-pick submitted in the next couple of days. Let me know if it needs to be bumped to 1.21.3. Thanks.

@mdempsky mdempsky self-assigned this Sep 25, 2023
@mdempsky
Copy link
Member

Thanks for the reminder.

@gopherbot gopherbot modified the milestones: Go1.21.2, Go1.21.3, Go1.21.4 Oct 5, 2023
@dmitshur
Copy link
Contributor

dmitshur commented Oct 12, 2023

Since the backport applies cleanly to release-branch.go1.21, I can try creating it and starting trybots on it. See go.dev/cl/534916. If it's not helpful, I can close it.

@gopherbot
Copy link
Author

Change https://go.dev/cl/534916 mentions this issue: [release-branch.go1.21] cmd/compile/internal/typecheck: fix closure field naming

gopherbot pushed a commit that referenced this issue Oct 13, 2023
…ield naming

When creating the struct type to hold variables captured by a function
literal, we currently reuse the captured variable names as fields.

However, there's no particular reason to do this: these struct types
aren't visible to users, and it adds extra complexity in making sure
fields belong to the correct packages.

Further, it turns out we were getting that subtly wrong. If two
function literals from different packages capture variables with
identical names starting with an uppercase letter (and in the same
order and with corresponding identical types) end up in the same
function (e.g., due to inlining), then we could end up creating
closure struct types that are "different" (i.e., not types.Identical)
yet end up with equal LinkString representations (which violates
LinkString's contract).

The easy fix is to just always use simple, exported, generated field
names in the struct. This should allow further struct reuse across
packages too, and shrink binary sizes slightly.

For #62498.
Fixes #62545.

Change-Id: I9c973f5087bf228649a8f74f7dc1522d84a26b51
Reviewed-on: https://go-review.googlesource.com/c/go/+/527135
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit e3ce312)
Reviewed-on: https://go-review.googlesource.com/c/go/+/534916
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
@gopherbot
Copy link
Author

Closed by merging f9a31cd to release-branch.go1.21.

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 compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Development

No branches or pull requests

5 participants