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

Represent GLsync as uintptr, not unsafe.Pointer #79

Merged
merged 1 commit into from
May 9, 2017
Merged

Represent GLsync as uintptr, not unsafe.Pointer #79

merged 1 commit into from
May 9, 2017

Conversation

dominikh
Copy link
Contributor

@dominikh dominikh commented May 9, 2017

Go treats unsafe.Pointer as actual pointers, requiring them to be valid,
and allowing them to keep Go memory referenced.

GLsync, however, can be of arbitrary value, such as simple incrementing
IDs, or values that look like – but aren't – Go memory.

In order to avoid faulty garbage collection behaviour, these values must
not be stored in unsafe.Pointer. Instead we use uintptr, which is
guaranteed to be large enough to hold GLsync values.

Closes go-gl/gl#71
Updates #63

Go treats unsafe.Pointer as actual pointers, requiring them to be valid,
and allowing them to keep Go memory referenced.

GLsync, however, can be of arbitrary value, such as simple incrementing
IDs, or values that look like – but aren't – Go memory.

In order to avoid faulty garbage collection behaviour, these values must
not be stored in unsafe.Pointer. Instead we use uintptr, which is
guaranteed to be large enough to hold GLsync values.

Closes go-gl/gl#71
Updates #63
@dominikh
Copy link
Contributor Author

dominikh commented May 9, 2017

/cc @errcw @shurcooL

@errcw
Copy link
Member

errcw commented May 9, 2017

LGTM, thanks!

We should probably regenerate everything in go-gl/gl after this change.

@errcw errcw merged commit 7ec9805 into go-gl:master May 9, 2017
@dmitshur
Copy link
Member

dmitshur commented May 9, 2017

We should probably regenerate everything in go-gl/gl after this change.

Yep. I'll reopen go-gl/gl#71 to track that.

dmitshur added a commit to go-gl/gl that referenced this pull request May 9, 2017
Regenerate all bindings after generator change in go-gl/glow#79.

Done with latest version of glow:

	go generate -tags=gen github.com/go-gl/gl

Resolves #71.
@dmitshur
Copy link
Member

Should we revert this PR until we get to a resolution in go-gl/gl#71? glow is in a bad state now, and will block other changes (e.g., #80).

@errcw
Copy link
Member

errcw commented May 11, 2017

Alas, yes. We can submit it again once we have a resolution.

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

Successfully merging this pull request may close these issues.

3 participants