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

Squelch MSVC warning exporting subclasses of runtime_error #1433

Merged
merged 1 commit into from
Nov 30, 2019
Merged

Squelch MSVC warning exporting subclasses of runtime_error #1433

merged 1 commit into from
Nov 30, 2019

Conversation

marti4d
Copy link

@marti4d marti4d commented Nov 29, 2019

When compiling {fmt} as a DLL, MSVC complains that we are exporting
classes that inherit from "std::runtime_error", which we are not
exporting.

In this case, it's not really a problem because that symbol is already
exported via the C++ stdlib. So we just add a pragma to silence the
warning.

I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.

@marti4d
Copy link
Author

marti4d commented Nov 29, 2019

After I looked more at the code and saw how runtime_error is used, I strongly agree with your reasoning for closing down my earlier issue. Sorry about that - If I had looked and thought about it a bit more, I probably wouldn't have filed the ticket.

What I have here is one potential solution to fix the warning. One of the only things that may-or-may-not matter is that it will suppress this warning now for everything - So in the future if you actually did make the mistake of exporting a class without exporting its parent, no warning would be given.

Alternatively, I suppose it's possible to use #pragma warning(push) and #pragma warning(pop) to only disable the warning for those specific declarations - But that may negatively affect code readability.

@codicodi
Copy link
Contributor

codicodi commented Nov 29, 2019

I would advise for using push and pop, so that warning supression doesn't leak into user code.

Edit:
Actually fmt seems to use pragma warning(suppress ...) for those cases. It disables warning only on next line.

# pragma warning(suppress : 6102)

When compiling {fmt} as a DLL, MSVC complains that we are exporting
classes that inherit from "std::runtime_error", which we are not
exporting.

In this case, it's not really a problem because that symbol is already
exported via the C++ stdlib. So we just add a pragma to silence the
warning.
@marti4d
Copy link
Author

marti4d commented Nov 30, 2019

Indeed - It also appears that MSVC supports this __pragma(warning()) directive, and that it can even be put inline with the other API errata.

Perhaps you like this solution better?

@vitaut vitaut merged commit 3bc28fc into fmtlib:master Nov 30, 2019
@vitaut
Copy link
Contributor

vitaut commented Nov 30, 2019

Thanks!

vitaut added a commit that referenced this pull request Dec 3, 2019
iPherian pushed a commit to iPherian/fmt that referenced this pull request Dec 10, 2019
Based on pr fmtlib#1433 with changes to deal with issue fmtlib#1450

"When compiling {fmt} as a DLL, MSVC complains that we are exporting
classes that inherit from "std::runtime_error", which we are not
exporting.

In this case, it's not really a problem because that symbol is already
exported via the C++ stdlib. So we just add a pragma to silence the
warning."

Puts a pragma before the relevant class def instead of within it's
declaration for greater support among older msvc.
iPherian added a commit to iPherian/fmt that referenced this pull request Dec 10, 2019
Based on pr fmtlib#1433 with changes to deal with issue fmtlib#1450

"When compiling {fmt} as a DLL, MSVC complains that we are exporting
classes that inherit from "std::runtime_error", which we are not
exporting.

In this case, it's not really a problem because that symbol is already
exported via the C++ stdlib. So we just add a pragma to silence the
warning."

Puts a pragma before the relevant class def instead of within it's
declaration for greater support among older msvc.
vitaut pushed a commit that referenced this pull request Dec 13, 2019
…#1433) (#1470)

* Squelch MSVC warning exporting subclasses of runtime_error

When compiling {fmt} as a DLL, MSVC complains that we are exporting
classes that inherit from "std::runtime_error", which we are not
exporting.

In this case, it's not really a problem because that symbol is already
exported via the C++ stdlib. So we just add a pragma to silence the
warning.

* Fix compilation with MinGW

Commit 3bc28fc ("Squelch MSVC warning exporting subclasses of
runtime_error", 2019-11-29) silenced a MSVC warning under. The MinGW
compiler also defines _WIN32, but does not support the "warning" pragma.

Introduce a helper macro to squelch the MSVC warning only when using the
Microsoft compiler.

Signed-off-by: Beat Bolli <dev@drbeat.li>

* Fix compilation with VS2015 (#1450)

VS2015 does not support the __pragma(...) syntax in the midst of a
class declaration, so move it to just before the declaration.
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.

3 participants