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 · 6 comments
Open

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

bcmills opened this issue Apr 4, 2017 · 6 comments
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Contributor

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
Copy link
Contributor Author

bcmills commented Apr 4, 2017

(CC: @ianlancetaylor @thanm)

@anacrolix
Copy link
Contributor

I've also encountered this. It seems unnecessary.

@andybons
Copy link
Member

Ping Ian and Tham. Thoughts?

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 11, 2018
@andybons andybons added this to the Unplanned milestone Apr 11, 2018
@ianlancetaylor
Copy link
Contributor

Seems fine if someone wants to do it.

@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Apr 11, 2018
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 11, 2018
@thanm
Copy link
Contributor

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

@samhocevar
Copy link

Another reason why it really should be cgo that handles the declaration is that for now you have to manually adapt to e.g. the way these functions are exported on Windows and do things like:

#if _WIN32
void __declspec(dllexport) go_print_hello();
#else
void go_print_hello();
#endif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants