cmd/cgo: compiler accepts invalid declaration of methods on C data types
Closed this issue · 10 comments
$ cat a.go
package main
/*
typedef int foo;
*/
import "C"
func (C.foo) method() int { return 123 }
func main() {
var x C.foo
println(x.method()) // "123"
}
$ go run a.go
123
Is this program valid? I am surprised that one is allowed to declare Go methods on C datatypes because they are pretending to come from another package called "C". Superficially it looks like it should be a syntax error in the method's receiver declaration.
I understand that this is just an artifact of the translation to Go files, in which C.foo is replaced by something like __C_foo, which is a valid Go type capable of bearing methods, but the cgo build process should probably reject this.
It's probably fairly easy to reject this in cmd/cgo itself.
Actually I used this in the past as a convenient feature. So I think it's not a bug...
We got another gopls bug report related to this. As Ian says, it seems like it should be technically easy to fix this in cgo, but I fear the fix may break existing programs that have exploited the (IMHO) buggy behavior. Does that constitute an incompatible change? Does this need a proposal?
Notice also this variant using a type alias. It may defeat a purely syntactic attempt to fix the problem by detecting method receiver types that are dotted names.
package main
/*
typedef int foo;
*/
import "C"
type T = C.foo
func (T) method() int { return 123 }
func main() {
var x C.foo
println(x.method()) // "123"
}
I'm inclined to think that it's OK to break program that rely on defining methods on types that are defined in C. To me that seems analogous to defining methods on types defined in a different package, which is forbidden. In particular, if we permitted methods defined on types defined in C, then we could never fix #13467 which I think everyone would like fixed if we can figure out how.
It's probably fairly easy to reject this in cmd/cgo itself.
I don't know how to robustly reject this in cgo itself since it needs the type checker to resolve the method type, which could be a type alias to C.foo, or an arbitrarily deep chain of type aliases. Nor do I know how to robustly reject it in the compiler, because the func (_Ctype_foo) method() declaration could appear in ordinary Go source. We could check for the presence of import _ "runtime/cgo" to reduce false positives, but it's just a heuristic.
Got any ideas?
I suppose my first idea is to not worry about type aliases. Because, who cares? We'll fix 99.9% of the problem by just having cmd/cgo reject func (r C.type).
If we really want to solve the type alias issue, then my first thought would be a compiler pragma //go:cgotype that the compiler can recognize.
Change https://go.dev/cl/490819 mentions this issue: cmd/cgo: reject attempts to declare methods on C types
CC @eliasnaur as this will break gio: https://git.sr.ht/~eliasnaur/gio-shader/tree/main/item/piet/elements_abi.go#L29