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: 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
NeedsFix release-blocker
Milestone

Comments

@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented Sep 22, 2021

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

@rogpeppe rogpeppe commented Sep 22, 2021

See also #45396

@WangLeonard
Copy link
Contributor

@WangLeonard WangLeonard commented Sep 22, 2021

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

// 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

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

@danscales danscales commented Sep 22, 2021

@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.

@findleyr findleyr added this to the Go1.18 milestone Sep 22, 2021
@findleyr findleyr added NeedsInvestigation release-blocker labels Sep 22, 2021
@griesemer
Copy link
Contributor

@griesemer griesemer commented Sep 28, 2021

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

@toothrot toothrot commented Oct 13, 2021

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

@griesemer griesemer added NeedsDecision and removed NeedsInvestigation labels Oct 13, 2021
@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 13, 2021

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 danscales commented Oct 21, 2021

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

@gopherbot gopherbot commented Oct 28, 2021

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
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

@griesemer griesemer commented Oct 28, 2021

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 and removed NeedsDecision labels Oct 28, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 28, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix release-blocker
Projects
None yet
Development

No branches or pull requests

7 participants