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/cgo: emit forward-declarations for cgo-exported Go functions #19837

Open
bcmills opened this issue Apr 4, 2017 · 5 comments
Open

cmd/cgo: emit forward-declarations for cgo-exported Go functions #19837

bcmills opened this issue Apr 4, 2017 · 5 comments

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 4, 2017

I'm trying to pass a Go function as a C function pointer.

I expect to be able to do that by exporting the Go function to C, then passing the C name of the Go function:

package main

/*
static void invoke(void (*f)()) {
	f();
}

typedef void (*closure)();  // https://golang.org/issue/19835
*/
import "C"

import "fmt"

//export go_print_hello
func go_print_hello() {
	fmt.Println("Hello, !")
}

func main() {
	C.invoke(C.closure(C.go_print_hello))
}

Unfortunately, that results in a cgo error:

bcmills:~$ go build cgocallback
# cgocallback
could not determine kind of name for C.go_print_hello

As a workaround, I can add an explicit forward-declaration of the Go function to the C preamble:

/*
…
void go_print_hello();
*/
import "C"

Given that cgo (presumably) already knows the C declarations for the functions it is exporting, I would prefer that it emit those forward-declarations itself. Since all C types should be defined by the end of the user-defined preamble, it seems like cgo could emit the forward declarations immediately following the preamble.

(If user code needs to refer to the Go functions within the preamble itself, requiring an explicit forward-declaration seems reasonable.)

bcmills:~$ go version
go version devel +2bbfa6f746 Thu Mar 9 15:36:43 2017 -0500 linux/amd64
@bcmills

This comment has been minimized.

Copy link
Member Author

@bcmills bcmills commented Apr 4, 2017

@anacrolix

This comment has been minimized.

Copy link
Contributor

@anacrolix anacrolix commented Jun 3, 2017

I've also encountered this. It seems unnecessary.

@andybons

This comment has been minimized.

Copy link
Member

@andybons andybons commented Apr 11, 2018

Ping Ian and Tham. Thoughts?

@andybons andybons added this to the Unplanned milestone Apr 11, 2018
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 11, 2018

Seems fine if someone wants to do it.

@thanm

This comment has been minimized.

Copy link
Member

@thanm thanm commented Apr 11, 2018

Worth fixing I agree, but I don't think I have time to work on this in the near term

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.