Skip to content

Getting a weak-vtables warning with clang on ubuntu 22.04 #4087

New issue
New issue
Closed
#4500
Closed
Getting a weak-vtables warning with clang on ubuntu 22.04#4087
#4500
Assignees
nlohmann
Labels
kind: bugsolution: proposed fixa fix for the issue has been proposed and waits for confirmation
Milestone
Release 3.12.0

Description

@tanja84dk
tanja84dk
opened on Jul 26, 2023

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

Activity

  1. tanja84dk
    added
    kind: bug
    on Jul 26, 2023

  2. dantrag commented on Aug 8, 2024

    @dantrag
    dantrag
    on Aug 8, 2024

    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.

  3. nlohmann commented on Nov 18, 2024

    @nlohmann
    nlohmann
    on Nov 18, 2024
    Owner

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

  4. nlohmann
    self-assigned this
    on Nov 18, 2024
  5. nlohmann
    added a commit that references this issue on Nov 18, 2024

    🚨 fix weak-vtables warning #4087

    3e0d562

  6. nlohmann commented on Nov 18, 2024

    @nlohmann
    nlohmann
    on Nov 18, 2024
    Owner

    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.

  7. nlohmann
    added
    state: help neededthe issue needs help to proceed
    on Nov 18, 2024

  8. gregmarr commented on Nov 18, 2024

    @gregmarr
    gregmarr
    on Nov 18, 2024
    Contributor

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

  9. gregmarr commented on Nov 18, 2024

    @gregmarr
    gregmarr
    on Nov 18, 2024
    Contributor

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

  10. gregmarr commented on Nov 18, 2024

    @gregmarr
    gregmarr
    on Nov 18, 2024
    Contributor

    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:

    json/include/nlohmann/detail/macro_scope.hpp

    Line 160 in 64f68dc

    #pragma clang diagnostic ignored "-Wdocumentation-unknown-command"

  11. nlohmann
    added
    solution: proposed fixa fix for the issue has been proposed and waits for confirmation
    and removed
    state: help neededthe issue needs help to proceed
    on Nov 19, 2024

  12. nlohmann commented on Nov 19, 2024

    @nlohmann
    nlohmann
    on Nov 19, 2024
    Owner

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

  13. nlohmann
    added this to the Release 3.11.4 milestone on Nov 20, 2024
  14. nlohmann
    added a commit that references this issue on Nov 20, 2024

    Fix weak-vtables warning (#4500)

    378e091
  15. nlohmann
    closed this as completedin #4500on Nov 20, 2024
  16. slowriot
    added a commit that references this issue on Jan 10, 2025

    Fix weak-vtables warning (nlohmann#4500)

    d9a5007
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

kind: bugsolution: proposed fixa fix for the issue has been proposed and waits for confirmation

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions
    ⌂ 换网址