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

spec: package-level variable initialization with constant dependencies not explicitly specified in Go spec #66575

Open
bigwhite opened this issue Mar 28, 2024 · 10 comments
Assignees
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@bigwhite
Copy link

bigwhite commented Mar 28, 2024

Go version

go 1.22.0

Output of go env in your module/workspace:

$go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/Users/tonybai/Library/Caches/go-build'
GOENV='/Users/tonybai/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/tonybai/Go/pkg/mod'
GONOPROXY='bitbucket.org/bigwhite/t'
GOOS='darwin'
GOPATH='/Users/tonybai/Go'
GOPROXY='https://goproxy.cn'
GOROOT='/Users/tonybai/.bin/go1.22.0'
GOSUMDB='off'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/tonybai/.bin/go1.22.0/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/cz/sbj5kg2d3m3c6j650z0qfm800000gn/T/go-build1615674647=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I have example below:

// main.go

package main

import (
	"fmt"
)

var (
	v0 = constInitCheck()
	v1 = variableInit("v1")
	v2 = variableInit("v2")
)

const (
	c1 = "c1"
	c2 = "c2"
)

func constInitCheck() string {
	if c1 != "" {
		fmt.Println("main: const c1 has been initialized")
	}
	if c1 != "" {
		fmt.Println("main: const c2 has been initialized")
	}
	return ""
}

func variableInit(name string) string {
	fmt.Printf("main: var %s has been initialized\n", name)
	return name
}

func init() {
	fmt.Println("main: first init func invoked")
}

func init() {
	fmt.Println("main: second init func invoked")
}

func main() {
	// do nothing
}

What did you see happen?

Compile and run the code in go 1.22.0,I got this output:

$go run main.go
main: var v1 has been initialized
main: var v2 has been initialized
main: const c1 has been initialized
main: const c2 has been initialized
main: first init func invoked
main: second init func invoked

What did you expect to see?

According to the latest go spec:

Within a package, package-level variable initialization proceeds stepwise, with each step selecting the variable earliest in declaration order which has no dependencies on uninitialized variables.

More precisely, a package-level variable is considered ready for initialization if it is not yet initialized and either has no [initialization expression](https://go.dev/ref/spec#Variable_declarations) or its initialization expression has no dependencies on uninitialized variables. Initialization proceeds by repeatedly initializing the next package-level variable that is earliest in declaration order and ready for initialization, until there are no variables ready for initialization.

v0、v1 and v2 all have initializaiton expression , and should be considered as "not ready for initialization"。their init order should be v0 -> v1 -> v2。but the real init order is v1 -> v2 -> v0。

@bigwhite
Copy link
Author

If we place const declaration block before var declaration block, the init order will become v0 -> v1 -> v2:

$go run main.go // go1.22.0
main: const c1 has been initialized
main: const c2 has been initialized
main: var v1 has been initialized
main: var v2 has been initialized
main: first init func invoked
main: second init func invoked

@seankhliao
Copy link
Member

constInitCheck() has a dependency on c1 and c2, which are uninitialized at that point.
variableInit has no dependency. this all appears consistent with the current spec.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Mar 28, 2024
@bigwhite
Copy link
Author

@seankhliao thanks for instant reply. Go spec says "no dependencies on uninitialized variables" , but c1 and c2 are uninitialized constants. there is a little bit of inconsistency.

@randall77
Copy link
Contributor

This looks like it changed between 1.21 and 1.22.
I agree with the OP, constants are not variables.
@gri

@randall77 randall77 reopened this Mar 28, 2024
@randall77
Copy link
Contributor

@griesemer

@seankhliao seankhliao changed the title Package-level variable initialization with constant dependencies not explicitly specified in Go spec spec: package-level variable initialization with constant dependencies not explicitly specified in Go spec Mar 28, 2024
@gopherbot
Copy link

Change https://go.dev/cl/575075 mentions this issue: cmd/compile: put constants before variables in initialization order

@randall77
Copy link
Contributor

Bisect points to https://go-review.googlesource.com/c/go/+/517617, switching to use types2 for init order, which makes sense. types2 always had this bug.
FYI @mdempsky

@griesemer griesemer self-assigned this Mar 28, 2024
@griesemer griesemer added this to the Go1.23 milestone Mar 28, 2024
@go101
Copy link

go101 commented Mar 28, 2024

[edit]: BTW, should this be backported? As it may cause some wrong initial values: https://go.dev/play/p/2HA4DR3_cjD


Just a curious BTW question: why is a initialized after b?
By my understanding of the spec, the order should be inverted.

package main

var x = 0
var a = foo()
var b = x

func foo() int {
	x++
	return x
}

func main() {
	println(a, b) // 1 0
}

@ianlancetaylor
Copy link
Contributor

@go101 Thanks. Would you mind opening a separate issue for that? The gc compiler prints 1, 0 while gccgo prints 1, 1.

@randall77
Copy link
Contributor

Reopening for @griesemer to decide if anything needs to change in the spec.

@go101 I don't think this issue warrants backporting. The workaround is pretty easy.

@randall77 randall77 reopened this Mar 28, 2024
@seankhliao seankhliao added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

7 participants