golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.13k stars 17.37k forks source link

cmd/compile: type checking fails in cycle involving recursive struct and unsafe.Sizeof #14620

Open rsc opened 8 years ago

rsc commented 8 years ago

package p

import "unsafe"

type hdr struct {
    next *msg
}

type msg struct {
    hdr
    pad [1024-hdrsize]uint8
}

const hdrsize = unsafe.Sizeof(hdr{})

This should type check but does not:

$ go tool compile /tmp/x.go
/tmp/x.go:14: invalid type for composite literal: hdr
/tmp/x.go:14: invalid expression unsafe.Sizeof(hdr literal)

Looks like the compiler gets confused by the not-quite-cycle in the definition.

http://play.golang.org/p/Hbmv1j_UrR

/cc @RLH

davmaz commented 8 years ago

Could this also be the case (when using cgo): phySize := unsafe.Sizeof(C.struct_ad9361_rf_phy) giving a compile time error: invalid expression unsafe.Sizeof(_Ctype_struct_ad9361_rf_phy)

ianlancetaylor commented 8 years ago

@davmaz No, the problem there is simply that unsafe.Sizeof takes a value, not a type. It's not the same as the C sizeof operation.

In general, for the Go project, please ask questions in a forum rather than on the issue tracker. See https://golang.org/wiki/Questions. Thanks.

mdempsky commented 8 years ago

The problem is that when typechecking a pointer to a named type like *msg, cmd/compile recurses and tries to completely typecheck msg. Instead, we should just do the minimum to get a *Type value for msg (in this case a TFORW type) so we can construct the *msg pointer type. Later when we visit msg's type declaration, we can finish typechecking it.

A workaround is to move the hdr type declaration after the msg type declaration.

rsc commented 8 years ago

Quite possibly Robert's front end revamp will take care of this. Maybe not worth worrying about before then.

griesemer commented 8 years ago

FWIW, gotype accepts this code w/o problems. Since there's a work-around per @mdempsky I don't think there's any urgency here.

josharian commented 7 years ago

Reproduced at tip (00263a896856c12cb57ee17355c6f911252b6214) on May 10, 2017.

Still seems low priority; punting to 1.10.

cuonglm commented 9 months ago

FWIW, gotype accepts this code w/o problems. Since there's a work-around per @mdempsky I don't think there's any urgency here.

FYI, both go/types and cmd/compile now reports an error as of go1.21 and tip:

$ go tool compile p.go
p.go:5:6: invalid recursive type hdr
    p.go:5:6: hdr refers to
    p.go:9:6: msg refers to
    p.go:14:7: hdrsize refers to
    p.go:5:6: hdr
$ gotype p.go
p.go:5:6: invalid recursive type hdr
p.go:5:6:   hdr refers to
p.go:9:6:   msg refers to
p.go:14:7:  hdrsize refers to
p.go:5:6:   hdr
mdempsky commented 8 months ago

Related #13890.