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.

Sign up for GitHub

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

Squelch MSVC warning exporting subclasses of runtime_error (fix for PR #1433) #1470

Merged
merged 3 commits into from
Dec 13, 2019
Merged

Squelch MSVC warning exporting subclasses of runtime_error (fix for PR #1433) #1470

merged 3 commits into from
Dec 13, 2019

Conversation

iPherian
Copy link
Contributor

@iPherian iPherian commented Dec 10, 2019 •
edited
Loading

Based on PR #1433, which was reverted because of issue #1450 (i.e. fails to build under VS2015). This PR fixes that build while keeping the applicable warnings silenced.

VS2015 does not support the __pragma(...) syntax in the midst of a class declaration, so move it to just before the declaration.

original explanation:

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.

Successful build log:

Log 
cmake -G "Visual Studio 14 2015" -Ax64 -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=ON ..
-- CMake version: 3.15.5
-- The CXX compiler identification is MSVC 19.0.24241.7
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/bin/x86_amd64/cl.exe
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/bin/x86_amd64/cl.exe -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Version: 6.1.1
-- Build type: Release
-- CXX_STANDARD: 11
-- Performing Test has_std_11_flag
-- Performing Test has_std_11_flag - Success
-- Performing Test has_std_0x_flag
-- Performing Test has_std_0x_flag - Failed
-- Performing Test SUPPORTS_VARIADIC_TEMPLATES
-- Performing Test SUPPORTS_VARIADIC_TEMPLATES - Success
-- Performing Test SUPPORTS_USER_DEFINED_LITERALS
-- Performing Test SUPPORTS_USER_DEFINED_LITERALS - Success
-- Performing Test FMT_HAS_VARIANT
-- Performing Test FMT_HAS_VARIANT - Failed
-- Looking for _strtod_l
-- Looking for _strtod_l - found
-- Target 'doc' disabled (requires doxygen)
-- Looking for C++ include pthread.h
-- Looking for C++ include pthread.h - not found
-- Found Threads: TRUE  
-- Performing Test HAVE_FNO_DELETE_NULL_POINTER_CHECKS
-- Performing Test HAVE_FNO_DELETE_NULL_POINTER_CHECKS - Failed
-- FMT_PEDANTIC: OFF
-- Configuring done
-- Generating done
-- Build files have been written to: C:/projects/fmt/build
cmake --build . --config Release
Microsoft (R) Build Engine version 14.0.25420.1
Copyright (C) Microsoft Corporation. All rights reserved.
  Checking Build System
  Building Custom Rule C:/projects/fmt/CMakeLists.txt
  format.cc
  posix.cc
  Generating Code...
     Creating library C:/projects/fmt/build/Release/fmt.lib and object C:/projects/fmt/build/Release/fmt.exp
  fmt.vcxproj -> C:\projects\fmt\build\bin\Release\fmt.dll
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  gmock-gtest-all.cc
  gmock.vcxproj -> C:\projects\fmt\build\test\Release\gmock.lib
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  test-main.cc
  gtest-extra.cc
  util.cc
  Generating Code...
  test-main.vcxproj -> C:\projects\fmt\build\test\Release\test-main.lib
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  assert-test.cc
C:\projects\fmt\test\assert-test.cc(21): warning C4003: not enough actual parameters for macro 'GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_' [C:\projects\fmt\build\test\assert-test.vcxproj]
  assert-test.vcxproj -> C:\projects\fmt\build\bin\Release\assert-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  chrono-test.cc
  chrono-test.vcxproj -> C:\projects\fmt\build\bin\Release\chrono-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  color-test.cc
  color-test.vcxproj -> C:\projects\fmt\build\bin\Release\color-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  compile-test.cc
  compile-test.vcxproj -> C:\projects\fmt\build\bin\Release\compile-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  core-test.cc
  core-test.vcxproj -> C:\projects\fmt\build\bin\Release\core-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  custom-formatter-test.cc
  custom-formatter-test.vcxproj -> C:\projects\fmt\build\bin\Release\custom-formatter-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  format-test.cc
  format-test.vcxproj -> C:\projects\fmt\build\bin\Release\format-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  grisu-test.cc
  grisu-test.vcxproj -> C:\projects\fmt\build\bin\Release\grisu-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  gtest-extra-test.cc
  gtest-extra-test.vcxproj -> C:\projects\fmt\build\bin\Release\gtest-extra-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  header-only-test.cc
  header-only-test2.cc
  test-main.cc
  Generating Code...
  header-only-test.vcxproj -> C:\projects\fmt\build\bin\Release\header-only-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  locale-test.cc
  locale-test.vcxproj -> C:\projects\fmt\build\bin\Release\locale-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  ostream-test.cc
  ostream-test.vcxproj -> C:\projects\fmt\build\bin\Release\ostream-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  posix-mock-test.cc
  format.cc
  test-main.cc
  gtest-extra.cc
  util.cc
  Generating Code...
     Creating library C:/projects/fmt/build/test/Release/posix-mock-test.lib and object C:/projects/fmt/build/test/Release/posix-mock-test.exp
  posix-mock-test.vcxproj -> C:\projects\fmt\build\bin\Release\posix-mock-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  posix-test.cc
  posix-test.vcxproj -> C:\projects\fmt\build\bin\Release\posix-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  printf-test.cc
  printf-test.vcxproj -> C:\projects\fmt\build\bin\Release\printf-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  ranges-test.cc
  ranges-test.vcxproj -> C:\projects\fmt\build\bin\Release\ranges-test.exe
  Building Custom Rule C:/projects/fmt/test/CMakeLists.txt
  scan-test.cc
  scan-test.vcxproj -> C:\projects\fmt\build\bin\Release\scan-test.exe
  Building Custom Rule C:/projects/fmt/CMakeLists.txt
Discovering tests...OK
Build success

(build is of shared libraries under VS2015 i.e the config of the associated issue -- including because it's not currently on the auto tests for the repo)

P.S. I'm not sure if I should have cherry-picked, but the commits this one depends on were reverted.

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

Chris Martin and others added 3 commits December 10, 2019 02:09
@iPherian
Squelch MSVC warning exporting subclasses of runtime_error
a118c65 
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.
@bbolli @iPherian
Fix compilation with MinGW
6d2491b 
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>
@iPherian
Fix compilation with VS2015 (fmtlib#1450)
2a10d75 
VS2015 does not support the __pragma(...) syntax in the midst of a
class declaration, so move it to just before the declaration.
@iPherian iPherian force-pushed the fix_suppress_export_warnings branch from 4d0546a to 2a10d75 Compare December 10, 2019 12:24
vitaut
vitaut reviewed Dec 13, 2019
View reviewed changes
include/fmt/format.h
Comment on lines +691 to 692
FMT_CLASS_API
class FMT_API format_error : public std::runtime_error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define FMT_CLASS_API to include FMT_API and simplify this as follows:

class FMT_CLASS_API format_error : public std::runtime_error {

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, re-reading the PR description I realized that it's not possible.

@vitaut vitaut merged commit 8ab1c5c into fmtlib:master Dec 13, 2019
@vitaut
Copy link
Contributor

vitaut commented Dec 13, 2019

Merged, thanks!

@iPherian iPherian deleted the fix_suppress_export_warnings branch December 14, 2019 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vitaut vitaut vitaut left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iPherian @vitaut @bbolli