Skip to content

Wrap callback in extern "C" block.#2

Open
Kerilk wants to merge 1 commit into
StreamHPC:settings_location_test_winfrom
Kerilk:callback-calling-convention
Open

Wrap callback in extern "C" block.#2
Kerilk wants to merge 1 commit into
StreamHPC:settings_location_test_winfrom
Kerilk:callback-calling-convention

Conversation

@Kerilk
Copy link
Copy Markdown

@Kerilk Kerilk commented May 6, 2022

This is probably me being pedantic, but the dispatch table is defined inside an extern "C" block, while the callbacks were not. This shouldn't have been a problem, but this feels cleaner.

I created a pull request (rather than a suggestion) in order to have the CI validate the change.

CI ran: see https://github.com/Kerilk/OpenCL-Layers/actions/runs/2282819938 and https://github.com/Kerilk/OpenCL-Layers/actions/runs/2282819937

edit: if this looks good, I will make a separate pull-request for the other layers

@Kerilk
Copy link
Copy Markdown
Author

Kerilk commented May 9, 2022

@MathiasMagnus does this makes sense to you? do you want me to make a similar pull request on the layer repo, but for all C++ layers?

@MathiasMagnus
Copy link
Copy Markdown

MathiasMagnus commented May 10, 2022

@Kerilk I totally agree, having them in an extern c block would be cleaner. This branch is the source branch of an already merged PR to upstream, so I may just cherry pick this commit on our next PR, just to keep attribution.

@Kerilk
Copy link
Copy Markdown
Author

Kerilk commented May 10, 2022

Let's proceed as you see best. I can also just redo this on all the layers on the OpenCL repo as well and open a PR there, just tell me.

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.

2 participants