cmd/compile: determine root cause for compiler crash in #23823

[griesemer]

Reminder issue: #23823 documents a compiler crash for which we introduced a temp. fix. Fix root cause for the crash, and remove temp. work-around code (see align.go, dowith function).

[cuonglm]

@griesemer With:

package p

type I = interface { 
	I2
}

type I2 interface {
	I
}

Should the error message point to line 3 instead of line 7?

Current workaround point the error to line 7

go/test/fixedbugs/issue23823.go

Line 13 in 8b058cf

type I2 interface { // ERROR "invalid recursive type"

[griesemer]

@cuonglm It would be better for the error to be on line 3, but I'm happy we're getting a reasonable error in the first place. Cyclic interfaces using alias types are likely not working correct in all cases; we need to fix the root problems first (non-trivial) before dealing with positioning of error messages.

[cuonglm]

@griesemer Yay, I have a patch in my local, which does report the error at line 3, that's why I asked question above. I will send the CL when I found a way to explain it easily.

[cuonglm]

@griesemer Hmm, I cleanup my patch, and the only change needed to fix this issue is set n.Type.Nod in typecheck, OTINTER case, after this line

go/src/cmd/compile/internal/gc/typecheck.go

Line 519 in 8b058cf

n.Type = tointerface(n.List.Slice())

The change + workaround removed does pass all the tests + change in issue23823.go to point error at line 3.

Since you said this should be a non-trivial change, I'm afraid I'm missing something.

[griesemer]

@cuonglm Sorry for being unclear: What I meant is that fixing correct handling of cycles with alias types is non-trivial in general. It may well be that this specific line number fix is trivial.

[cuonglm]

@griesemer when enable typecheck tracing, I got completely different error, is it intended behavior?

$ go tool compile -t p.go 
p.go:7:6: typecheck 0xc000354600 DCLTYPE <node DCLTYPE> tc=0
p.go:7:6: . typecheck1 0xc000354600 DCLTYPE <node DCLTYPE> tc=2
p.go:7:6: . . typecheck 0xc0003723c0 TYPE I2 tc=0
p.go:7:6: . . . typecheck1 0xc0003723c0 TYPE I2 tc=2
p.go:7:6: . . . . typecheckdef 0xc0003723c0 TYPE I2 tc=2
p.go:7:6: . . . . . typecheckdeftype 0xc0003723c0 TYPE I2 tc=2
p.go:7:9: . . . . . . typecheck 0xc000354580 TINTER <inter> tc=0
p.go:7:9: . . . . . . . typecheck1 0xc000354580 TINTER <inter> tc=2
p.go:3:6: . . . . . . . . typecheck 0xc0003722d0 TYPE I tc=0
p.go:3:6: . . . . . . . . . typecheck1 0xc0003722d0 TYPE I tc=2
p.go:3:6: . . . . . . . . . . typecheckdef 0xc0003722d0 TYPE I tc=2
p.go:3:10: . . . . . . . . . . . typecheck 0xc000354400 TINTER <inter> tc=0
p.go:3:10: . . . . . . . . . . . . typecheck1 0xc000354400 TINTER <inter> tc=2
p.go:4:2: . . . . . . . . . . . . . typecheck 0xc000354300 NONAME I2 tc=0
p.go:4:2: . . . . . . . . . . . . . . resolve 0xc000354300 NONAME I2 tc=0
p.go:7:6: . . . . . . . . . . . . . . => 0xc0003723c0 TYPE I2 tc=1 type=undefined I2
p.go:7:6: . . . . . . . . . . . . . . typecheck1 0xc0003723c0 TYPE I2 tc=2
p.go:7:6: . . . . . . . . . . . . . . . typecheckdef 0xc0003723c0 TYPE I2 tc=2
p.go:7:6: . . . . . . . . . . . . . . . => 0xc0003723c0 TYPE I2 tc=2 type=undefined I2
p.go:7:6: . . . . . . . . . . . . . . => 0xc0003723c0 TYPE I2 tc=2 type=undefined I2
p.go:7:6: . . . . . . . . . . . . . => 0xc0003723c0 TYPE I2 tc=1 type=undefined I2
p.go:3:10: . . . . . . . . . . . . => 0xc000354400 TYPE interface { I2 } tc=2 type=interface { I2 }
p.go:3:10: . . . . . . . . . . . => 0xc000354400 TYPE interface { I2 } tc=1 type=interface { I2 }
p.go:3:6: . . . . . . . . . . => 0xc0003722d0 TYPE interface { I2 } tc=2 type=interface { I2 }
p.go:3:6: . . . . . . . . . => 0xc0003722d0 TYPE interface { I2 } tc=2 type=interface { I2 }
p.go:3:6: . . . . . . . . => 0xc0003722d0 TYPE interface { I2 } tc=1 type=interface { I2 }
p.go:7:9: . . . . . . . => 0xc000354580 TYPE interface { I2 } tc=2 type=interface { I2 }
p.go:7:9: . . . . . . => 0xc000354580 TYPE interface { I2 } tc=1 type=interface { I2 }
p.go:7:6: . . . . . => 0xc0003723c0 TYPE I2 tc=1 type=interface { I2 }
p.go:7:6: . . . . => 0xc0003723c0 TYPE I2 tc=1 type=interface { I2 }
p.go:7:6: . . . => 0xc0003723c0 TYPE I2 tc=1 type=interface { I2 }
p.go:7:6: . . => 0xc0003723c0 TYPE I2 tc=1 type=interface { I2 }
p.go:7:6: . => 0xc000354600 DCLTYPE <node DCLTYPE> tc=2 type=<T>
p.go:7:6: => 0xc000354600 DCLTYPE <node DCLTYPE> tc=1 type=<T>
p.go:3:6: typecheck 0xc000354480 DCLTYPE <node DCLTYPE> tc=0
p.go:3:6: . typecheck1 0xc000354480 DCLTYPE <node DCLTYPE> tc=2
p.go:3:6: . . typecheck 0xc0003722d0 TYPE interface { I2 } tc=1
p.go:3:6: . . . typecheck1 0xc0003722d0 TYPE interface { I2 } tc=2
p.go:3:6: . . . . typecheckdef 0xc0003722d0 TYPE interface { I2 } tc=2
p.go:3:6: . . . . => 0xc0003722d0 TYPE interface { I2 } tc=2 type=interface { I2 }
p.go:3:6: . . . => 0xc0003722d0 TYPE interface { I2 } tc=2 type=interface { I2 }
p.go:3:6: . . => 0xc0003722d0 TYPE interface { I2 } tc=1 type=interface { I2 }
p.go:3:6: . => 0xc000354480 DCLTYPE <node DCLTYPE> tc=2 type=<T>
p.go:3:6: => 0xc000354480 DCLTYPE <node DCLTYPE> tc=1 type=<T>
p.go:4:2: interface contains embedded non-interface I2

[griesemer]

@cuonglm No. Looks like tracing changes things, which it shouldn't.

[gopherbot]

Change https://golang.org/cl/191540 mentions this issue: cmd/compile: fix embedding interface cycles with type alias