Navigation Menu

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: inline static init cause compile time error #58439

Closed
cuonglm opened this issue Feb 9, 2023 · 8 comments
Closed

cmd/compile: inline static init cause compile time error #58439

cuonglm opened this issue Feb 9, 2023 · 8 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@cuonglm
Copy link
Member

cuonglm commented Feb 9, 2023

What version of Go are you using (go version)?

$ go version
go version go1.20 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

package main

var x = f(-1)
var y = f(64)

func f(x int) int {
	return 1 << x
}

func main() {
	println(x, y)
}

What did you expect to see?

Runtime error.

What did you see instead?

Compile time error.

@cuonglm cuonglm added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 9, 2023
@cuonglm cuonglm self-assigned this Feb 9, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 9, 2023
@cuonglm
Copy link
Member Author

cuonglm commented Feb 9, 2023

The fix for #58293 only deal with the integer overflow case.

@cuonglm
Copy link
Member Author

cuonglm commented Feb 9, 2023

This needs to be backport, too cc @randall77 @mdempsky @dr2chase

There're a plenty of issue with inline static init happens, and although I think https://go-review.googlesource.com/c/go/+/466277 should fix them, maybe a safer way to backport is disable inline static init for go1.20, then re-enable by default during go1.21 cycle.

@cuonglm cuonglm added this to the Go1.21 milestone Feb 9, 2023
@gopherbot
Copy link

Change https://go.dev/cl/466277 mentions this issue: cmd/compile: rename EvalConst to EvalExpr

@randall77
Copy link
Contributor

I agree, probably the safest thing to do for 1.20 is just disable inline static init. We can fix for 1.21 properly.

@mdempsky
Copy link
Member

mdempsky commented Feb 9, 2023

Disabling the optimization for 1.20 seems right to me too, unfortunately.

@cuonglm
Copy link
Member Author

cuonglm commented Feb 9, 2023

@gopherbot please backport this issue to 1.20

@gopherbot
Copy link

Backport issue(s) opened: #58444 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot
Copy link

Change https://go.dev/cl/467016 mentions this issue: cmd/compile: reenable inline static init

gopherbot pushed a commit that referenced this issue Apr 14, 2023
Updates #58293
Updates #58339
Fixes #58439

Change-Id: I06d2d92f86fa4a672d69515c4066d69d3e0fc75b
Reviewed-on: https://go-review.googlesource.com/c/go/+/467016
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
@golang golang locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants