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

#define EXPORT causing macro redefinition warning #108

Open
reister-kenneth opened this issue Nov 21, 2022 · 8 comments
Open

#define EXPORT causing macro redefinition warning #108

reister-kenneth opened this issue Nov 21, 2022 · 8 comments

Comments

@reister-kenneth
Copy link

In xpl.h EXPORT is being defined. This conflicts with a Microsoft lib

...\windows kits\10\include\10.0.15063.0\um\sqltypes.h

Perhaps a namespace could be added or just a rename of the symbol?

@penduin
Copy link
Member

penduin commented Nov 21, 2022

looks like both projects just define it with no value unless you're static-linking, in which case WJE sets it to __declspec(dllexport)

maybe just wrapping our definition in include/xpl.h:

#ifdef _WIN32
# define		WIN_CDECL		__cdecl
# define		WIN_STDCALL		__stdcall
# ifndef COMPILE_AS_STATIC
#  define		EXPORT			__declspec(dllexport)
#  define		IMPORT			__declspec(dllimport)
# else
#  ifndef EXPORT
#   define		EXPORT
#  endif
#  define		IMPORT
#endif

would make your build happy?

it's not the tidiest change (and it leaves unsolved the case of static linking and including sqltypes.h) but i think i'd rather go with that than find-and-replace EXPORT to something else. (internally, we've got enough XPL code and macros for that to be troublesome.)

@reister-kenneth
Copy link
Author

When I get some free time I'll apply your changes and see how it likes it.

@reister-kenneth
Copy link
Author

Mostly I'm concerned about our upgrade path. For now I've just renamed the symbol to prevent the collision, but the next time we update WJElement it will come back.

@reister-kenneth
Copy link
Author

I think you misread the code

#ifndef COMPILE_AS_STATIC

We're compiling the code directly into the project, not as an external DLL/SO so we're not defining the above. I'm getting the

#  define		EXPORT			__declspec(dllexport)

I changed it to

# ifndef COMPILE_AS_STATIC
#ifndef EXPORT
#  define		EXPORT			__declspec(dllexport)
#endif
#  define		IMPORT			__declspec(dllimport)
# else

and all is good.

For a quick fix maybe something more like

# ifndef COMPILE_AS_STATIC
#ifndef XPL_DONT_DEFINE_EXPORT
#  define		EXPORT			__declspec(dllexport)
#endif
#  define		IMPORT			__declspec(dllimport)
# else

And I can just add that to our project -DXPL_DONT_DEFINE_EXPORT

@penduin
Copy link
Member

penduin commented Nov 21, 2022

You're right, I had it backwards. I'm glad you found a local fix; we'll mull over what might be the best solution. After looking through our stuff, I think we should indeed be able to change it to XPLEXPORT or something.

It's interesting that you've basically told it to not bother defining EXPORT anymore, and everything still works; I'm not much of a windows guy, so I do not know what __declspec(dllexport) does (or did) for us. Maybe it's obsolete at this point?

@minego
Copy link
Member

minego commented Nov 21, 2022

Have you tried something like:

#include <wjelement.h>
#undef EXPORT

@penduin
Copy link
Member

penduin commented Nov 21, 2022

Have you tried something like:

#include <wjelement.h>
#undef EXPORT

I knew I was overthinking this. :^)

@reister-kenneth
Copy link
Author

__declspec(dllexport) :
https://learn.microsoft.com/en-us/cpp/build/exporting-from-a-dll-using-declspec-dllexport?view=msvc-170
TLDR - It's about name mangling.

I found this snippet in our code

#ifndef EXPORT
#define EXPORT extern "C" __declspec(dllexport)
#endif

With 3 different uses (wjelement, ours, sqltypes.h) I'm starting to think that EXPORT is going to be a "natural" choice for anyone wanting to make that define. That said, my preferred fix at this point would be to rename it. I'm probably going to go rename ours as well shortly.

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

No branches or pull requests

3 participants