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

Add support for user-supplied prefix/suffix in function declarations #154

Merged
merged 1 commit into from
Feb 15, 2021

Conversation

kpet
Copy link
Contributor

@kpet kpet commented Feb 9, 2021

This change adds the ability for a user of the headers to pass
prefixes or suffixes to be used in the function prototypes via
(respectively) CL_API_PREFIX_USER and CL_API_SUFFIX_USER.

This can be used to pass additional attributes (e.g. to use
weak symbols).

Signed-off-by: Kévin Petit kpet@free.fr

This change adds the ability for a user of the headers to pass
prefixes or suffixes to be used in the function prototypes via
(respectively) CL_API_PREFIX_USER and CL_API_SUFFIX_USER.

This can be used to pass additional attributes (e.g. to use
weak symbols).

Signed-off-by: Kévin Petit <kpet@free.fr>
@kpet
Copy link
Contributor Author

kpet commented Feb 9, 2021

Also see the (related) proposed tidy up on #153

Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

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

LGTM, just one small comment / question that doesn't need to hold up merging.

Comment on lines +52 to +53
#define CL_API_SUFFIX_COMMON CL_API_SUFFIX_USER
#define CL_API_PREFIX_COMMON CL_API_PREFIX_USER
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know how much we care about this, but I think we could avoid defining these symbols and just use the user-defined prefix and suffixes directly. Alternatively, we could #undef these symbols after they're used.

As a separate issue (not for this PR), I think we should also formally reserve all of the CL_API_XXX names in the spec for usage in the headers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added these to make it easier to manage common definitions in the future. I agree they're not strictly required for this change. We could indeed #undef them after but I'm not sure it matters too much, especially if we're reserving all the CL_API* names (I've added this to a checlist in #153).

@kpet
Copy link
Contributor Author

kpet commented Feb 15, 2021

I'll merge this one as is to get forward progress but I'm happy to discuss other changes as part of #153. I've broadened the scope of that issue to cover all related problems and added a checklist there so we don't lose track of what needs doing.

@kpet kpet merged commit d0d6457 into KhronosGroup:master Feb 15, 2021
@kpet kpet deleted the user-prefix-suffix branch February 15, 2021 20:35
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