json Getting a weak-vtables warning with clang on ubuntu 22.04

Description

I noticed that when ever I try to compile with clang ( 11 - 15 as I have installed) on with my normal warning flags on linux then I always ends up with a warning from the exceptions header about weak-vtables, I do know that some would say I have alot of warning flags activated but that is because that is what I saw some time ago that fmtlib uses and chose to use it myself also to catch errors and posible bad practices from my side

I do use CPM-cmake to pull in libraries purely because that is able to store them in a local cache so I don't have to pull the libraries over and over for each project and it does use the github release

Reproduction steps

Tested with clang 11, 12, 13, 14, 15 on ubuntu jammy (22.04)

The shell command is taken from cmake log

/usr/bin/clang++-15  -I/home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include -g -Werror -Wall -Wextra -pedantic -Wconversion -Wundef -Wdeprecated -Wweak-vtables -Wshadow -Wno-gnu-zero-variadic-macro-arguments -Wzero-as-null-pointer-constant -std=gnu++17 -MD -MT CMakeFiles/tanja84dk_testproject.dir/src/main.cpp.o -MF CMakeFiles/tanja84dk_testproject.dir/src/main.cpp.o.d -o CMakeFiles/tanja84dk_testproject.dir/src/main.cpp.o -c /buildtest/Cpp-Tanja84dk-DockerClient/src/main.cpp

I'm using CPM-cmake to pull in libraries and in essence its just pulling the 3.11.2 release from the release tab by tag

cmake_minimum_required(VERSION 3.16)
project(tanja84dk_testproject LANGUAGES CXX VERSION 0.1.0
)

list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")

include(cmake/CPM.cmake)

include(CheckCXXCompilerFlag)

if (CMAKE_CXX_COMPILER_ID MATCHES "GNU")
  set(PEDANTIC_COMPILE_FLAGS -pedantic-errors -Wall -Wextra -pedantic
      -Wold-style-cast -Wundef
      -Wredundant-decls -Wwrite-strings -Wpointer-arith
      -Wcast-qual -Wformat=2 -Wmissing-include-dirs
      -Wcast-align
      -Wctor-dtor-privacy -Wdisabled-optimization
      -Winvalid-pch -Woverloaded-virtual
      -Wconversion -Wundef
      -Wno-ctor-dtor-privacy -Wno-format-nonliteral)
  if (NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.6)
      set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS}
         -Wno-dangling-else -Wno-unused-local-typedefs)
  endif ()
  if (NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 5.0)
      set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS} -Wdouble-promotion
          -Wtrampolines -Wzero-as-null-pointer-constant -Wuseless-cast
          -Wvector-operation-performance -Wsized-deallocation -Wshadow)
  endif ()
  if (NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 6.0)
      set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS} -Wshift-overflow=2
          -Wnull-dereference -Wduplicated-cond)
  endif ()
  set(WERROR_FLAG -Werror)
endif ()

if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
  set(PEDANTIC_COMPILE_FLAGS -Wall -Wextra -pedantic -Wconversion -Wundef
    -Wdeprecated -Wweak-vtables -Wshadow
    -Wno-gnu-zero-variadic-macro-arguments)
  check_cxx_compiler_flag(-Wzero-as-null-pointer-constant HAS_NULLPTR_WARNING)
  if (HAS_NULLPTR_WARNING)
    set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS} -Wzero-as-null-pointer-constant)
  endif ()
  set(WERROR_FLAG -Werror)
endif ()

if (MSVC)
  set(PEDANTIC_COMPILE_FLAGS /W4)
  set(WERROR_FLAG /WX)
endif ()

set(tanja84dk_testproject_SOURCES
    src/main.cpp
)

set(tanja84dk_testproject_LIBRARIES
    nlohmann_json
)

CPMAddPackage(
    NAME            nlohmann_json
    GIT_REPOSITORY  https://github.com/nlohmann/json
    VERSION         3.11.2
    GIT_TAG         v3.11.2
)

add_executable(tanja84dk_testproject ${tanja84dk_testproject_SOURCES})

set_target_properties(tanja84dk_testproject
    PROPERTIES
        ARCHIVE_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib"
        LIBRARY_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib"
        RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
        CXX_STANDARD 17
    FOLDER
        Application
)

target_link_libraries(tanja84dk_testproject PRIVATE ${tanja84dk_testproject_LIBRARIES})

target_compile_options(tanja84dk_testproject
    PRIVATE
        ${WERROR_FLAG}
        ${PEDANTIC_COMPILE_FLAGS})

target_compile_features(tanja84dk_testproject
    PRIVATE
        cxx_std_17
)

### Expected vs. actual results

Expected it to build successfully specially because I didn't do anything specially in the small test but it warned/errored with the weak-vtables in the include/nlohmann/detail/exceptions.hpp

### Minimal code example

```Shell
#include <iostream>
#include <nlohmann/json.hpp>

int main() { return 0; }

Error messages

[main] Building folder: Cpp-Tanja84dk-DockerClient 
[build] Starting build
[proc] Executing command: /usr/bin/cmake --build /buildtest/Cpp-Tanja84dk-DockerClient/build --config Debug --target all --
[build] [1/2  50% :: 2.195] Building CXX object CMakeFiles/tanja84dk_testproject.dir/src/main.cpp.o
[build] FAILED: CMakeFiles/tanja84dk_testproject.dir/src/main.cpp.o 
[build] /usr/bin/clang++-15  -I/home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include -g -Werror -Wall -Wextra -pedantic -Wconversion -Wundef -Wdeprecated -Wweak-vtables -Wshadow -Wno-gnu-zero-variadic-macro-arguments -Wzero-as-null-pointer-constant -std=gnu++17 -MD -MT CMakeFiles/tanja84dk_testproject.dir/src/main.cpp.o -MF CMakeFiles/tanja84dk_testproject.dir/src/main.cpp.o.d -o CMakeFiles/tanja84dk_testproject.dir/src/main.cpp.o -c /buildtest/Cpp-Tanja84dk-DockerClient/src/main.cpp
[build] In file included from /buildtest/Cpp-Tanja84dk-DockerClient/src/main.cpp:2:
[build] In file included from /home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include/nlohmann/json.hpp:35:
[build] In file included from /home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include/nlohmann/adl_serializer.hpp:14:
[build] In file included from /home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include/nlohmann/detail/conversions/from_json.hpp:23:
[build] /home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include/nlohmann/detail/exceptions.hpp:36:7: error: 'exception' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
[build] class exception : public std::exception
[build]       ^
[build] /home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include/nlohmann/detail/exceptions.hpp:134:7: error: 'parse_error' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
[build] class parse_error : public exception
[build]       ^
[build] /home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include/nlohmann/detail/exceptions.hpp:187:7: error: 'invalid_iterator' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
[build] class invalid_iterator : public exception
[build]       ^
[build] /home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include/nlohmann/detail/exceptions.hpp:205:7: error: 'type_error' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
[build] class type_error : public exception
[build]       ^
[build] /home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include/nlohmann/detail/exceptions.hpp:222:7: error: 'out_of_range' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
[build] class out_of_range : public exception
[build]       ^
[build] /home/buildclient/.cache/CPM/nlohmann_json/b3708972f6694fe462e4112e47aa04f10d2390b4/nlohmann_json/include/nlohmann/detail/exceptions.hpp:239:7: error: 'other_error' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Werror,-Wweak-vtables]
[build] class other_error : public exception
[build]       ^
[build] 6 errors generated.
[build] ninja: build stopped: subcommand failed.
[proc] The command: /usr/bin/cmake --build /buildtest/Cpp-Tanja84dk-DockerClient/build --config Debug --target all -- exited with code: 1
[driver] Build completed: 00:00:02.244
[build] Build finished with exit code 1

Compiler and operating system

Clang 11, 12, 13, 14, 15 on ubuntu 22.04

Library version

3.11.2

Validation

[X] The bug also occurs if the latest version from the develop branch is used.
[ ] I can successfully compile and run the unit tests.

Jul 26 '23 16:07 tanja84dk

I am facing the same issue (my project has to have -Wweak-vtables enabled), and it seems like a low effort to fix it? std::exception has a virtual destructor, which is the reason for this warning. All that is needed is to declare an explicit destructor in each of the exception classes and add out-of-line default definitions like "exception::~exception() = default;" alongside.

Aug 08 '24 12:08 dantrag

Indeed we compile with -Wno-weak-vtables in the CI. I will have a look.

Nov 18 '24 18:11 nlohmann

Ok, I'm getting to the boundaries of my C++ knowledge.

I can fix the -Wweak-vtables warning by adding

~exception() override;

to the class definition and add

exception::~exception() = default;

outside the class. This makes Clang happy, but Clang-Tidy complains:

error: function '~exception' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers,-warnings-as-errors]

I am note sure how to fix this, because the library is header-only.

Furthermore, GCC complains (-Wdeprecated-copy-dtor):

implicitly-declared 'parse_error::parse_error(const parse_error&)' is deprecated [-Werror=deprecated-copy-dtor]
note: because 'parse_error' has user-provided 'parse_error::~parse_error()'

I guess this can be fixed by just adding a copy constructor and a copy assignment operator.

Nov 18 '24 20:11 nlohmann

I think maybe you should add inline before the definition. I've never tried that with a out of line defaulted function before.

Nov 18 '24 21:11 gregmarr

Oh, yeah, if there wasn't a destructor there before and you add one, it suppresses the other of the special 5.

Nov 18 '24 21:11 gregmarr

Okay, I've done some reading and some thinking, and I don't think it's possible to fix this in a header-only library. I mean that we MIGHT be able to silence the warning, but it will still result in the exact same behavior everywhere. There is no implementation file where we can define any virtual function. All the virtual functions will still be defined in every translation unit that includes the header. The best we can do is to forcefully silence the warning, and I'm not sure that we should actually do that. It is not possible to be 100% warning free, and this particular warning is only enabled explicitly or by using -Weverything, which even the clang devs say shouldn't be used. https://quuxplusone.github.io/blog/2018/12/06/dont-use-weverything/

On the other hand, we already have a diagnostic push/pop, so maybe just add it here and document that since it isn't possible to honor the spirit of the warning in a header-only library, we just disable it: https://github.com/nlohmann/json/blob/64f68dc5635b821ef93996acd5e391792814c07d/include/nlohmann/detail/macro_scope.hpp#L160

Nov 18 '24 21:11 gregmarr

I added a diagnostic push to suppress the warning in #4500. PTAL.

Nov 19 '24 08:11 nlohmann