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

Jump to bottom

Use __has_declspec_attribute for shared builds #3616

Closed
wants to merge 1 commit into from
Closed

Use __has_declspec_attribute for shared builds #3616

wants to merge 1 commit into from

Conversation

donny-dont
Copy link
Contributor

Clang compilation targets may support __declspec attributes. Because of that it has a __has_declspec_attribute check.

Currently cURL just assumes that __declspec is Windows only. This adds a compatibility macro and checks whether dllimport and dllexport are available.

@bagder
Copy link
Member

bagder commented Feb 25, 2019

Okay, but do we really want this for non-windows?

@donny-dont
Copy link
Contributor Author

Any toolchain that's clang based could potentially use __declspec. We use it on the PlayStation and ship cURL

@bagder
Copy link
Member

bagder commented Feb 26, 2019

Right, they can use it but with this change they will use it - unconditionally. I'm not even sure what the significance is of using this, but it is certainly a change to how it worked without this change.

BTW, the added line is too long which why the CI turns red:

../../include/curl/curl.h:117:146: warning: Longer than 79 columns (LONGLINE)

@donny-dont
Copy link
Contributor Author

When you build clang out for a platform you have to explicitly turn on support for __declspec so this would only affect a platform that wants to use it.

I figured I'd see if the change was palatable to upstream. If its not I'm fine with maintaining it outside of curl.

@bagder
Copy link
Member

bagder commented Feb 27, 2019

this would only affect a platform that wants to use it.

Ah, right. Thanks for explaining this for an ignorant like me! 😁 I'll merge...

@bagder bagder closed this in 50482b8 Feb 27, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@donny-dont @bagder