Skip to content

go/types, types2: scope for :=-defined range iteration variables is incorrect #51437

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
Closed
contrast-jproberts opened this issue Mar 2, 2022 · 12 comments
Closed

go/types, types2: scope for :=-defined range iteration variables is incorrect #51437

contrast-jproberts opened this issue Mar 2, 2022 · 12 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.18

Comments

@contrast-jproberts
Copy link

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

$ go version
go version go1.18rc1 darwin/amd64

Does this issue reproduce with the latest release?

It reproduces with the latest release candidate.

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

go env Output
$ go env
GOOS="darwin"
GOARCH="amd64"

What did you do?

https://go.dev/play/p/cO3Omj1diPk?v=gotip

What did you expect to see?

The same result as running with go1.17.7: "Hello, 世界"

https://go.dev/play/p/cO3Omj1diPk

What did you see instead?

./prog.go:13:3: internal compiler error: 'foo': Value live at entry. It shouldn't be. func foo, node tmpl, value nil

goroutine 6 [running]:
runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0xc887c0?, 0x0?}, {0xc0000e2000, 0x46}, {0xc000629e40, 0x4, 0x4})
	/usr/local/go/src/cmd/compile/internal/base/print.go:227 +0x1d7
cmd/compile/internal/base.Fatalf(...)
	/usr/local/go/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/ssagen.(*ssafn).Fatalf(0xc0000b2ebd?, {0x3?, 0x0?}, {0xd2eec8, 0x40}, {0xc0006814a0, 0x3, 0xa9c859?})
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:7729 +0x17d
cmd/compile/internal/ssagen.(*state).Fatalf(...)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:953
cmd/compile/internal/ssagen.(*state).variable(0xc00001ec00, {0xe7c158?, 0xc000567980?}, 0xc000696010?)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:6451 +0x202
cmd/compile/internal/ssagen.(*state).exprCheckPtr(0xc00001ec00, {0xe7c158?, 0xc000567980?}, 0x1)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:2602 +0x1b7
cmd/compile/internal/ssagen.(*state).expr(...)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:2560
cmd/compile/internal/ssagen.(*state).putArg(0xc00001ec00, {0xe7c158, 0xc000567980}, 0x0?)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:5752 +0x51
cmd/compile/internal/ssagen.(*state).call(0xc00001ec00, 0xc0005783f0, 0x0, 0x0)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:5111 +0x2593
cmd/compile/internal/ssagen.(*state).callResult(0xc00001ec00?, 0x0?, 0x0?)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:4931 +0x1b
cmd/compile/internal/ssagen.(*state).exprCheckPtr(0xc00001ec00, {0xe7add0?, 0xc0005783f0?}, 0x1)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:3207 +0x1ae5
cmd/compile/internal/ssagen.(*state).expr(...)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:2560
cmd/compile/internal/ssagen.(*state).stmt(0xc00001ec00, {0xe7a9e8, 0xc00066f680?})
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:1633 +0xb71
cmd/compile/internal/ssagen.(*state).stmtList(...)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:1399
cmd/compile/internal/ssagen.buildssa(0xc0003b2dc0, 0x0)
	/usr/local/go/src/cmd/compile/internal/ssagen/ssa.go:582 +0x1eb4
cmd/compile/internal/ssagen.Compile(0xc0003b2dc0, 0xc000043f90?)
	/usr/local/go/src/cmd/compile/internal/ssagen/pgen.go:183 +0x4c
cmd/compile/internal/gc.compileFunctions.func4.1(0x0?)
	/usr/local/go/src/cmd/compile/internal/gc/compile.go:153 +0x3a
cmd/compile/internal/gc.compileFunctions.func3.1()
	/usr/local/go/src/cmd/compile/internal/gc/compile.go:140 +0x4d
created by cmd/compile/internal/gc.compileFunctions.func3
	/usr/local/go/src/cmd/compile/internal/gc/compile.go:138 +0x78


Go build failed.
@contrast-jproberts
Copy link
Author

contrast-jproberts commented Mar 2, 2022 •
edited
Loading

@mdempsky I don't know enough to decide whether this is a duplicate of #51413 . This case seemed simpler and doesn't require generics, so I decided to submit a new issue. I will close it if you or others think it's a duplicate.

@ALTree ALTree changed the title cmd/compile/internal/ssagen: internal compiler error Value live at entry. It shouldn't be. cmd/compile: internal compiler error Value live at entry. It shouldn't be. Mar 2, 2022
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 2, 2022
@cixel
Copy link
Contributor

cixel commented Mar 2, 2022

Another repro example which does not involve *template.Template and maybe illustrates the issue a bit more: https://go.dev/play/p/S40yxHlQQUP?v=gotip

In 1.17, this yields ./prog.go:15:15: undefined: val.

@ianlancetaylor
Copy link
Member

CC @randall77

@ianlancetaylor ianlancetaylor added this to the Go1.18 milestone Mar 2, 2022
@ianlancetaylor
Copy link
Member

Marking as a release blocker because this is a compiler crash on valid code.

@cuonglm
Copy link
Member

cuonglm commented Mar 3, 2022

Seems there's disagreement between old and new typechecker, cc @griesemer @mdempsky

If I read the Go spec correctly, the old typechecker behavior seems correct. The val scope is block of the for, not inside the range expression.

@griesemer
Copy link
Contributor

griesemer commented Mar 3, 2022 •
edited
Loading

@cuonglm I'm not sure what you mean with the val scope. The type checker (types2) records val with the for scope:

for scope 0xc00019f2d0 {
.  var val *main.t
}

when I print out the scope info.

Never mind. I see what you mean. Investigating.

@griesemer griesemer self-assigned this Mar 3, 2022
@griesemer
Copy link
Contributor

Simpler reproducer:

package p

type T struct{}

func (T) m() []T { return nil }

func f(x T) {
	for _, x := range func() []T {
		return x.m()
	}() {
		_ = x
	}
}

Renaming the x iteration variable to y (also in the for block) will fix this. Looks like a type checker issue.

@griesemer griesemer changed the title cmd/compile: internal compiler error Value live at entry. It shouldn't be. go/types, types2: internal compiler error Value live at entry. It shouldn't be. Mar 3, 2022
@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 3, 2022
@griesemer
Copy link
Contributor

The following program should type-check but doesn't:

package p

type T struct{}

func (T) m() []int { return nil }

func f(x T) {
	for _, x := range func() []int {
		return x.m()
	}() {
		_ = x
	}
}

Error:

x.m undefined (type int has no field or method m)

Looks like a 0-day bug to me.

@cuonglm
Copy link
Member

cuonglm commented Mar 3, 2022

@griesemer So old typechecker works correctly.

Seems that we need to fix go/types as well.

@griesemer
Copy link
Contributor

Yeah. Fixes forthcoming.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/389594 mentions this issue: types2: fix scoping for iteration variables declared by range clause

@griesemer griesemer changed the title go/types, types2: internal compiler error Value live at entry. It shouldn't be. go/types, types2: scope for :=-defined range iteration variables is incorrect Mar 3, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/390018 mentions this issue: [release-branch.go1.18] go/types, types2: fix scoping for iteration variables declared by range clause

gopherbot pushed a commit that referenced this issue Mar 7, 2022
@griesemer @dmitshur
[release-branch.go1.18] go/types, types2: fix scoping for iteration v…
79cf7c8 
…ariables declared by range clause

Also correct scope position for such variables.
Adjusted some comments.

Fixes #51437.

Change-Id: Ic49a1459469c8b2c7bc24fe546795f7d56c67cb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/389594
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
(cherry picked from commit d3fe4e1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/390018
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
@dmitshur dmitshur mentioned this issue Mar 7, 2022
Closed
33 tasks
@golang golang locked and limited conversation to collaborators Jun 22, 2023
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
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
7 participants
@ianlancetaylor @ALTree @cixel @cuonglm @griesemer @gopherbot @contrast-jproberts