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.

Sign up for GitHub

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

Replace exported C types with unsafe.Pointer #101

Merged
merged 9 commits into from Oct 26, 2018
Merged

Replace exported C types with unsafe.Pointer #101

merged 9 commits into from Oct 26, 2018

Conversation

hajimehoshi
Copy link
Member

@hajimehoshi hajimehoshi commented Oct 22, 2018 •
edited

This PR replaces exported C types with unsafe.Pointer so that gl will not expose C types. This PR also adds comments describing the original C types.

Fixes go-gl/gl#113

Output Example:

// Parameter context has type *C.struct__cl_context.
// Parameter event has type *C.struct__cl_event.
func CreateSyncFromCLeventARB(context unsafe.Pointer, event unsafe.Pointer, flags uint32) uintptr {
...
}

@hajimehoshi
Copy link
Member Author

@dmitshur @slimsag PTAL

@hajimehoshi hajimehoshi changed the title Replace C types with unsafe.Pointer Replace exported C types with unsafe.Pointer Oct 22, 2018
dmitshur
dmitshur reviewed Oct 23, 2018
View reviewed changes
Copy link
Member

@dmitshur dmitshur left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor style comments for now.

I think it's more important to review this at the functional level and confirm this is the right direction. I'll need more time to think about that.

Edit: Never mind, I've already done that in go-gl/gl#113. (We could really use NeedsDecision, NeedsFix labels so it's easier to track when decisions are made.)

package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
hajimehoshi
hajimehoshi commented Oct 23, 2018
View reviewed changes
Copy link
Member Author

@hajimehoshi hajimehoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
@hajimehoshi
Copy link
Member Author

ping @slimsag

slimsag
slimsag reviewed Oct 25, 2018
View reviewed changes
package.go Outdated Show resolved Hide resolved
@hajimehoshi
Address on reviews
e6d3b1a 
Use GoCType instead of CType for documents
@hajimehoshi
Remove string.TrimSpace
84edf6b
@hajimehoshi hajimehoshi mentioned this pull request Oct 25, 2018
Merged
dmitshur
dmitshur reviewed Oct 26, 2018
View reviewed changes
package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
hajimehoshi
hajimehoshi commented Oct 26, 2018
View reviewed changes
Copy link
Member Author

@hajimehoshi hajimehoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
package.go Outdated Show resolved Hide resolved
dmitshur
dmitshur approved these changes Oct 26, 2018
View reviewed changes
Copy link
Member

@dmitshur dmitshur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@dmitshur dmitshur merged commit 92b1359 into go-gl:master Oct 26, 2018
dmitshur pushed a commit to go-gl/gl that referenced this pull request Oct 26, 2018
@hajimehoshi @dmitshur
all: Replace C types with unsafe.Pointer. (#114)
55b76b7 
This change applies the glow change go-gl/glow#101.

Done with:

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

Fixes #113.
@hajimehoshi hajimehoshi deleted the noc branch October 26, 2018 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmitshur dmitshur dmitshur approved these changes

@slimsag slimsag slimsag approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hajimehoshi @dmitshur @slimsag