-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: runtime/cgo: move built-in C helpers from cmd/cgo to runtime/cgo #51134
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
Comments
I like this idea. However, I believe it will require some sort of compiler magic since $ cat main.go
package main
/*
static inline const char* foo() {
return "hello from C";
}
*/
import "C"
import "example.com/ctest/b"
func main() {
p := C.foo()
s := b.GoString(p)
println(s)
}
$ cat b/b.go
package b
import "C"
func GoString(p *C.char) string {
return "wat"
}
$ go run main.go
# command-line-arguments
./main.go:14:18: cannot use p (variable of type *_Ctype_char) as type *b._Ctype_char in argument to b.GoString |
Yes, we could put each name in both C.Foo and cgo.Foo, with cgo translating C.Foo to cgo.Foo. I am not worried about having to look in the cmd/cgo docs. There are many cgo details there. Might as well have them all together. As for tab completion, it seems like gopls could be updated to take care of that issue without putting users through a shift in how they write cgo code. Better to adapt the tools to reality at this point. |
This proposal has been added to the active column of the proposals project |
After reading
That's a very good point, before posting this proposal I did a quick prototype to verify it was feasible, but did not validate calling
These are the features that are missing:
Additionally, and probably nitpicking, having Go functions (which accepts Go types) exposed in the C package goes against the principle of least surprise, as I would expect to find in that package pure C symbols.
It's true that this ship might already sailed. Improving the tools and the documentation for these functions will also solve most of my issues. I will file a proposal to special-case these functions in gopls and also submit some commits that I think will improve their documentation, p.e. instructing |
Change https://go.dev/cl/386414 mentions this issue: |
Based on the discussion above, this proposal seems like a likely decline. |
No change in consensus, so declined. |
Adding comments to these functions help IDE tooling to display meaningful documentation, p.e. on hover. Tested with gopls and vscode. Updates #51134 Change-Id: Ie956f7cf192af0e828def4a141783f3a2589f77d Reviewed-on: https://go-review.googlesource.com/c/go/+/386414 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-by: Russ Cox <[email protected]> Run-TryBot: Russ Cox <[email protected]> Auto-Submit: Russ Cox <[email protected]>
cmd/cgo
generates on the fly several Go functions, namelyC.GoString
,C.GoStringN
,C.GoBytes
,C.CString
, andC.CBytes
.These functions inherit most of cgo usability problems despite being plain Go functions, most notably they do not play well with autocompletion. Additionally, its difficult to learn how to use them, and even know they exist, as they are only documented at https://pkg.go.dev/cmd/cgo in the middle of a long guide about how to use cgo, when one would expect them to appear in the
Functions
section.I propose that we stop treating these function in a special way by defining and implementing them in runtime/cgo.
In order to maintain backwards compatibility, the autogenerated
C
functions won't be removed but they will just be a wrapper for the newruntime/cgo
functions.The proposed API is almost the same as the one already generated by
runtime/cgo
, with the only difference thatGoStringN
andGoBytes
accept anint
instead of aC.int
as length parameter.The text was updated successfully, but these errors were encountered: