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 extern to global vars #53

Merged
merged 1 commit into from
Mar 31, 2023
Merged

Conversation

darbyjohnston
Copy link
Contributor

Doing some searching it seems that extern and extern "C" are not the same; the first declares that something has external linkage, while the second declares that the symbols are not mangled in C++. This PR hopefully fixes #22 by adding extern to the global variables, along with extern "C" when compiling with C++.

To use extern with extern "C" I needed to add curly brackets, so I created a couple of new macros for defining a block of statements as extern "C".

Signed-off-by: Darby Johnston <[email protected]>
@meshula
Copy link
Member

meshula commented Mar 31, 2023

Oh, a legendary hero has arrived!!! Thanks!

That's a pattern I hadn't thought of at all. I wonder if this would work, also

#ifdef __cplusplus
#define OTIO_EXTERN extern "C" extern
#else
#define OTIO_EXTERN extern
#endif

and then OTIO_EXTERN below, instead of OTIO_API in those cases.

The only reason for the alternate pattern being that one wouldn't need the scoping braces, so the macro could be used anywhere.

If you want to try this, great, otherwise, I'm ready to land as is. LMK which you prefer.

@darbyjohnston
Copy link
Contributor Author

That works with MSVC but not with GCC:

error: invalid use of ‘extern’ in linkage specification
   31 | #define OTIO_EXTERN extern "C" extern

I'm not super happy with the macros I added but couldn't think of a better way to add the curly brackets. Though one positive is you can now replace multiple uses of OTIO_API with a pair of OTIO_API_BEGIN and OTIO_API_END.

@meshula
Copy link
Member

meshula commented Mar 31, 2023

Ok, thanks for checking!

@meshula meshula merged commit 58ed923 into OpenTimelineIO:main Mar 31, 2023
BadSingleton pushed a commit to Unity-Technologies/OpenTimelineIO-C-Bindings that referenced this pull request May 12, 2023
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.

Some symbols are not properly defined as extern
2 participants