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: document cgo [0]byte bug fix #7958

Closed
gopherbot opened this issue May 9, 2014 · 11 comments

Comments

@gopherbot
Copy link

commented May 9, 2014

by miha.dimec@visionect.com:

What does 'go version' print?
go version devel +f8b50ad4cac4 Mon Apr 21 17:00:27 2014 -0700 + linux/amd64

What steps reproduce the problem?
Code snippet that does compile under go.1.2.2 but does not under go.1.3
cairo.go:
package cairo

// #cgo pkg-config: cairo cairo-gobject
// #include <stdlib.h>
// #include <cairo.h>
// #include <cairo-gobject.h>
import "C"
...
type Context struct {
    context *C.cairo_t
}

...
func WrapContext(context *C.cairo_t) *Context {
    ctx := &Context{context}
    ctx.reference()
    return ctx
}
...

gdk.go:
func CairoCreate(window *Window) *cairo.Context {
    ctx := C.gdk_cairo_create(window.native())
    if ctx == nil {
        return nil
    }
    return cairo.WrapContext(ctx)
}

I am trying to compile a local branch of github.com/confomal/gotk3 library using latest
go.1.3 build.

What happened?
$ go build github.com/visionect/gotk3/gdk
# github.com/visionect/gotk3/gdk
src/github.com/visionect/gotk3/gdk/gdk.go:287: cannot use ctx (type *C.cairo_t) as type
*cairo.C.cairo_t in argument to cairo.WrapContext

What should have happened instead?
Don't know if this is the correct behavior, but code compiled normally with Go 1.2.2 and
older.
@gopherbot

This comment has been minimized.

Copy link
Author

commented May 9, 2014

Comment 1 by miha.dimec@visionect.com:

Ups sorry for typing error. I meant cgo namespace in the title.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 9, 2014

Comment 2:

I suspect there is a consequence of a bug fix, but since it is a change it at least
needs to be mentioned in go1.3.html.

Labels changed: added repo-main, release-go1.3.

@gopherbot

This comment has been minimized.

Copy link
Author

commented May 9, 2014

Comment 4 by miha.dimec@visionect.com:

This is correct behavior then?
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 9, 2014

Comment 5:

I haven't really thought about it.  It looks right at first glance.
It would help a lot if you could provide a small self-contained example, rather than
code fragments.
@rsc

This comment has been minimized.

Copy link
Contributor

commented May 9, 2014

Comment 6:

You are using C.foo types in your exported API. I don't believe this ever worked.
Internally C.foo becomes a name like *_C_foo. Those names are different in different
packages: you cannot pass a *gdk._C_foo where a *cairo._C_foo is expected.
This may have worked in the past because cgo was incorrectly turning all undefined
structs into *[0]byte, so that they were all interchangeable.
CL https://golang.org/cl/76450043 fixed this. I think the fix is correct and
that your original program was incorrect.
This issue is now about documenting the [0]byte change in that CL.

Status changed to Accepted.

@dominikh

This comment has been minimized.

Copy link
Member

commented May 11, 2014

Comment 7:

Maybe http://golang.org/cmd/cgo should also talk about how C.foo names get translated.
Currently, it only speaks of a "pseudo-package", which might lead one to believe that
all C.foo refer to the same C.foo, no matter in which package they appear.
@gopherbot

This comment has been minimized.

Copy link
Author

commented May 12, 2014

Comment 8 by miha.dimec@visionect.com:

Ok thank you. Then corrections need to be made inside library.
@gopherbot

This comment has been minimized.

Copy link
Author

commented May 17, 2014

Comment 9:

CL https://golang.org/cl/91520043 mentions this issue.
@gopherbot

This comment has been minimized.

Copy link
Author

commented May 20, 2014

Comment 10:

CL https://golang.org/cl/91590044 mentions this issue.
@rsc

This comment has been minimized.

Copy link
Contributor

commented May 20, 2014

Comment 11:

This issue was closed by revision 7ef0eb1.

Status changed to Fixed.

@minux

This comment has been minimized.

Copy link
Member

commented May 31, 2014

Comment 12:

This issue was closed by revision 3f66c0c.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
Fixes golang#7958.

LGTM=rsc
R=golang-codereviews, rsc, r, gobot
CC=golang-codereviews
https://golang.org/cl/91520043
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.