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

Disabling MSVC warnings 4251 and 4275 #3143

Open
martingalvan-volue opened this issue Jul 24, 2024 · 3 comments
Open

Disabling MSVC warnings 4251 and 4275 #3143

martingalvan-volue opened this issue Jul 24, 2024 · 3 comments

Comments

@martingalvan-volue
Copy link

Hi, I noticed that

target_compile_options(spdlog PUBLIC $<$<AND:$<CXX_COMPILER_ID:MSVC>,$<NOT:$<COMPILE_LANGUAGE:CUDA>>>:/wd4251
disables the following MSVC linker warnings:

https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4251
https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4275

There's no explanation given in the code as to why these are disabled; however, looking at the description it seems like it's not correct to do so (4251 seems particularly concerning). Is there a reason why they're disabled instead of fixed?

@tt4g
Copy link
Contributor

tt4g commented Jul 24, 2024

This option was added when Windows shared libraries were supported (#1467).
I do not see the need for this option. It could be removed if there is a clear reason why the option is not needed.

@martingalvan-volue
Copy link
Author

martingalvan-volue commented Sep 24, 2024

Hi, I came back to this today and I think I know why these warnings might've been disabled. What happens is that spdlog has classes which are marked as dllexport when using spdlog as a DLL, but inherit from or use non-dllexport classes. An example of this is spdlog_ex, which inherits from std::exception and has a non-static data member of type std::string. According to the MSVC docs, these warnings are safe to disable if the non-dllexport class is part of the C++ standard library (which seems to be the case here).

I'm not sure, however, of how useful it is to disable these warnings when building spdlog itself as opposed to disabling them within the header files so they're not triggered in user code. In my case, I had to surround all inclusions of spdlog.h with #pragma warning(disable : 4251 4275) in order to be able to build my project. It would be nice if we could consider disabling these warnings within spdlog's own headers (as we do e.g. in os-onl.h with warning 4702).

@martingalvan-volue martingalvan-volue changed the title Windows linker warnings disabled Disabling MSVC warnings 4251 and 4275 Sep 24, 2024
@tt4g
Copy link
Contributor

tt4g commented Sep 24, 2024

PR is welcome.

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

2 participants