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

Jump to bottom

cmd/cgo: document that static functions work, static variables do not #9673

Closed
jscheel opened this issue Jan 23, 2015 · 8 comments
Closed

cmd/cgo: document that static functions work, static variables do not #9673

jscheel opened this issue Jan 23, 2015 · 8 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@jscheel
Copy link

jscheel commented Jan 23, 2015

Discussion from #go-nuts on irc:

[6:34pm] hyphenated: jscheel: you're right, it should make up its mind about the scope there..
[6:34pm] hyphenated: jscheel: either it's file-scope and it should be able to access C.foo, or it's not and it shouldn't be able to access C.init_foo
[6:36pm] hyphenated: jscheel: if another go file also had an import "C" defining an init_foo, it should be a different one from the above. right now, can call C.init_foo and if it's defined twice in the same package, it uses the first one it finds and ignores the other

Sample code:

package main

/*
static int foo;

static void init_foo() {
  foo = 5;
}
*/
import "C"

import (
    "fmt"
)

func main() {
    C.init_foo()
    fmt.Println(C.foo)
}
@minux
Copy link
Member

minux commented Jan 23, 2015

It depends on how cgo is implemented, you shouldn't rely on being able to
access static C variables or functions from Go.

@ianlancetaylor
Copy link
Contributor

It does seem inconsistent, though, and I think it should be fixed or documented.

@ianlancetaylor ianlancetaylor changed the title Scope of static variables in cgo code is uncertain cmd/cgo: Scope of static variables in cgo code is uncertain Jan 23, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.5 milestone Jan 23, 2015
@minux
Copy link
Member

minux commented Jan 23, 2015

Do you want to fix it or document it?

I think the correct fix is to disallow calling static functions defined in
the cgo preamble, however that might break current code that is working.
(BTW, the fix is not trivial because we need the function prototype to call
it, however, it is not easy to get the function prototype out.)

Given that we have never documented about this, the safest way is to
document it as a bug.

What do you prefer?

@ianlancetaylor
Copy link
Contributor

I agree that people call static functions today, and that it seems undesirable to break that. We even test it in misc/cgo/test, and I seem to recall fixing cgo so that those tests worked with gccgo.

I guess I don't see why static variables can't work in principle. We could generate static getters and setters and rewrite references in the Go code. Unless that seems infeasible perhaps we should leave this open in case somebody wants to fix it.

@minux
Copy link
Member

minux commented Jan 29, 2015

Seems easy to fix because we already use a pointer to access C variables,
we just need to move the definition of the pointer from _cgo_gotypes.go to
.cgo2.c. But we also need to add unique hash to the global name in case
the static C variable is used in multiple cgo packages.

The only problem is that we must make sure to only emit the pointer definition
once. Can cmd/cgo detect if one variable definition is static?

I don't want to go the SWIG way, because Go is more capable than common
scripting languages and can access most C variables/constants without going
through accessor functions.

@ianlancetaylor
Copy link
Contributor

I think cgo can detect a static variable by looking at the ELF symbol binding in the symbol table of cgo.o (or equivalent for the other formats).

@rsc rsc removed cgo labels Apr 10, 2015
@rsc
Copy link
Contributor

rsc commented Jun 8, 2015

As I understand it, static functions have always worked, and static variables have never worked. That seems perfectly fine. Let's document it and move on.

@rsc rsc changed the title cmd/cgo: Scope of static variables in cgo code is uncertain cmd/cgo: document that static functions work, static variables do not Jun 8, 2015
@gopherbot
Copy link

CL https://golang.org/cl/11221 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
5 participants
@rsc @minux @jscheel @ianlancetaylor @gopherbot