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: panic on generic struct literal #48538

Closed
rogpeppe opened this issue Sep 22, 2021 · 10 comments
Closed

cmd/compile: panic on generic struct literal #48538

rogpeppe opened this issue Sep 22, 2021 · 10 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.18

Comments

@rogpeppe
Copy link
Contributor

commit a83a558

I compiled this program:

package main

func main() {
}

type C interface {
	struct{ b1, b2 string }
}

func f[A C]() A {
	return A{
		b1: "a",
		b2: "b",
	}
}

I saw this panic:

./tst.go:12:3: internal compiler error: missing type for &{b1 {{{0xc000114000 12 3}}}} (*syntax.Name)

goroutine 1 [running]:
runtime/debug.Stack()
	/home/rogpeppe/go/src/runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0xc61920, 0x0}, {0xce878d, 0x18}, {0xc0000aed30, 0x2, 0x2})
	/home/rogpeppe/go/src/cmd/compile/internal/base/print.go:227 +0x154
cmd/compile/internal/noder.(*irgen).expr(0xc00013c000, {0xe35d48, 0xc0001281c0})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/expr.go:32 +0x214
cmd/compile/internal/noder.(*irgen).compLit(0xc00013c000, {0xe34dc0, 0xc0001145d0}, 0xc000124050)
	/home/rogpeppe/go/src/cmd/compile/internal/noder/expr.go:391 +0x4c5
cmd/compile/internal/noder.(*irgen).expr0(0xc00013c000, {0xe34dc0, 0xc0001145d0}, {0xe359e8, 0xc000124050})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/expr.go:107 +0x7ae
cmd/compile/internal/noder.(*irgen).expr(0xc00013c000, {0xe359e8, 0xc000124050})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/expr.go:81 +0x5ff
cmd/compile/internal/noder.(*irgen).exprs(0xc0000af378, {0xc0000af368, 0x1, 0xc000114000})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/expr.go:369 +0x8e
cmd/compile/internal/noder.(*irgen).exprList(0x40ce0b, {0xe359e8, 0xc000124050})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/expr.go:352 +0x85
cmd/compile/internal/noder.(*irgen).stmt(0xc00013c000, {0xe35dd8, 0xc000128180})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stmt.go:130 +0x9c5
cmd/compile/internal/noder.(*irgen).stmts(0xc0001307e0, {0xc00011c060, 0x1, 0xc0000af6a0})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/stmt.go:19 +0xaf
cmd/compile/internal/noder.(*irgen).funcBody(0xc00013c000, 0xc00011e420, 0xb7151d, 0xc00012a0c0, 0xc00012a100)
	/home/rogpeppe/go/src/cmd/compile/internal/noder/func.go:45 +0x25f
cmd/compile/internal/noder.(*irgen).funcDecl.func1()
	/home/rogpeppe/go/src/cmd/compile/internal/noder/decl.go:128 +0x68
cmd/compile/internal/noder.(*irgen).generate(0xc00013c000, {0xc000072b50, 0x2, 0xb})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/irgen.go:262 +0x1df
cmd/compile/internal/noder.check2({0xc000072b50, 0x2, 0x2})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/irgen.go:93 +0x175
cmd/compile/internal/noder.LoadPackage({0xc00001e210, 0x2, 0x0})
	/home/rogpeppe/go/src/cmd/compile/internal/noder/noder.go:90 +0x335
cmd/compile/internal/gc.Main(0xd08420)
	/home/rogpeppe/go/src/cmd/compile/internal/gc/main.go:190 +0xaf3
main.main()
	/home/rogpeppe/go/src/cmd/compile/main.go:55 +0xdd
@rogpeppe
Copy link
Contributor Author

See also #45396

@WangLeonard
Copy link
Contributor

I tried to analyze this problem.
I think we should record the key of return A{} in

go/src/cmd/compile/internal/types2/expr.go

Line 1282 in 91c2318

// 0 <= i < len(fields)

like this,
check.recordTypeAndValue(key, variable, etyp, nil)
or may be need to record the value too, like this
check.recordTypeAndValue(key, variable, etyp, constant.Make(kv.Value))

But there are still other problems, will Fatalf when transform A{} on

go/src/cmd/compile/internal/noder/transform.go

Line 934 in 91c2318

base.Fatalf("transformCompLit %v", t.Kind())

hope it will be helpful to who fix the problem in the future.
Thank you!

@danscales
Copy link
Contributor

@griesemer @findleyr Unusual case where the type of the return value of f is strictly (and only) determined by the type constraint C which specifies a specific struct type.

I'm assuming that's why types2 is not returning a type (as it normally does) for the b1 field of A in the struct literal. If we don't want to disallow this case somehow, then I guess types2 has to understand and use the type of A from the type constraint, so it can give a real type for b1.

@danscales danscales mentioned this issue Sep 22, 2021
Closed
@findleyr findleyr added this to the Go1.18 milestone Sep 22, 2021
@findleyr findleyr added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Sep 22, 2021
@griesemer
Copy link
Contributor

Per discussion, the issue here is that we need to agree upon what to do exactly with underlying/operational types of type parameters. Once we have made that decision, this (and many other cases) should fall out naturally.

@toothrot
Copy link
Contributor

Friendly ping that this issue is marked as a release-blocker for Go 1.18. Are there any updates?

@griesemer griesemer added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 13, 2021
@griesemer
Copy link
Contributor

This is easy to address but needs a decision. Notably, the type checker accepts this at the moment, but perhaps it shouldn't, at least for now. @ianlancetaylor any thoughts?

@danscales
Copy link
Contributor

danscales commented Oct 21, 2021 •
edited by griesemer

Robert has a singleUnder() type structure function that he will be checking in (and exporting) in the next few days, which will allow me to fix this bug.

@gopherbot
Copy link

Change https://golang.org/cl/359275 mentions this issue: cmd/compile/internal/types2: export Structure function

gopherbot pushed a commit that referenced this issue Oct 28, 2021
@griesemer
cmd/compile/internal/types2: export Structure function
12dc48f 
For #48538.

Change-Id: I258b0c8af5801692ad238e47397dde0b4e3c44c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/359275
Trust: Robert Griesemer <gri@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
@griesemer
Copy link
Contributor

This program is now considered valid and will be accepted by the type checker because A is a type parameter with a single underlying type (this will apply generally to many other operations).

@griesemer griesemer removed their assignment Oct 28, 2021
@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 28, 2021
@gopherbot
Copy link

Change https://golang.org/cl/359335 mentions this issue: cmd/compile: use Structure() to get single underlying type of typeparam.

@danscales danscales mentioned this issue Dec 16, 2021
Closed
@golang golang locked and limited conversation to collaborators Jun 23, 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
@toothrot @rogpeppe @danscales @griesemer @gopherbot @WangLeonard @findleyr