Fix weak-vtables warning (#4500)

* 🔧 remove warning suppression

* 🚨 fix weak-vtables warning #4087

* 🚨 suppress -Wweak-vtables warning

* 🚨 suppress -Wweak-vtables warning

* ✅ fix test

* ✅ fix test

* ✅ fix test

cmake/ci.cmake

@@ -96,7 +96,6 @@ file(GLOB_RECURSE SRC_FILES ${PROJECT_SOURCE_DIR}/include/nlohmann/*.hpp)
 # -Wno-padded                     We do not care about padding warnings.
 # -Wno-covered-switch-default     All switches list all cases and a default case.
 # -Wno-unsafe-buffer-usage        Otherwise Doctest would not compile.
-# -Wno-weak-vtables               The library is header-only.
 # -Wreserved-identifier           See https://github.com/onqtam/doctest/issues/536.
 
 set(CLANG_CXXFLAGS
@@ -109,7 +108,6 @@ set(CLANG_CXXFLAGS
     -Wno-padded
     -Wno-covered-switch-default
     -Wno-unsafe-buffer-usage
-    -Wno-weak-vtables
     -Wno-reserved-identifier
 )
 

include/nlohmann/detail/exceptions.hpp

@@ -25,6 +25,18 @@
 #include <nlohmann/detail/meta/type_traits.hpp>
 #include <nlohmann/detail/string_concat.hpp>
 
+// With -Wweak-vtables, Clang will complain about the exception classes as they
+// have no out-of-line virtual method definitions and their vtable will be
+// emitted in every translation unit. This issue cannot be fixed with a
+// header-only library as there is no implementation file to move these
+// functions to. As a result, we suppress this warning here to avoid client
+// code to stumble over this. See https://github.com/nlohmann/json/issues/4087
+// for a discussion.
+#if defined(__clang__)
+    #pragma clang diagnostic push
+    #pragma clang diagnostic ignored "-Wweak-vtables"
+#endif
+
 NLOHMANN_JSON_NAMESPACE_BEGIN
 namespace detail
 {
@@ -255,3 +267,7 @@ class other_error : public exception
 
 }  // namespace detail
 NLOHMANN_JSON_NAMESPACE_END
+
+#if defined(__clang__)
+    #pragma clang diagnostic pop
+#endif

single_include/nlohmann/json.hpp

@@ -4392,6 +4392,18 @@ inline OutStringType concat(Args && ... args)
 NLOHMANN_JSON_NAMESPACE_END
 
 
+// With -Wweak-vtables, Clang will complain about the exception classes as they
+// have no out-of-line virtual method definitions and their vtable will be
+// emitted in every translation unit. This issue cannot be fixed with a
+// header-only library as there is no implementation file to move these
+// functions to. As a result, we suppress this warning here to avoid client
+// code to stumble over this. See https://github.com/nlohmann/json/issues/4087
+// for a discussion.
+#if defined(__clang__)
+    #pragma clang diagnostic push
+    #pragma clang diagnostic ignored "-Wweak-vtables"
+#endif
+
 NLOHMANN_JSON_NAMESPACE_BEGIN
 namespace detail
 {
@@ -4623,6 +4635,10 @@ class other_error : public exception
 }  // namespace detail
 NLOHMANN_JSON_NAMESPACE_END
 
+#if defined(__clang__)
+    #pragma clang diagnostic pop
+#endif
+
 // #include <nlohmann/detail/macro_scope.hpp>
 
 // #include <nlohmann/detail/meta/cpp_future.hpp>

tests/src/unit-regression2.cpp

@@ -233,7 +233,33 @@ inline void from_json(const nlohmann::json& j, FooBar& fb) // NOLINT(misc-use-in
 struct for_3171_base // NOLINT(cppcoreguidelines-special-member-functions)
 {
     for_3171_base(const std::string& /*unused*/ = {}) {}
-    virtual ~for_3171_base() = default;
+    virtual ~for_3171_base();
+
+    for_3171_base(const for_3171_base& other) // NOLINT(hicpp-use-equals-default,modernize-use-equals-default)
+        : str(other.str)
+    {}
+
+    for_3171_base& operator=(const for_3171_base& other)
+    {
+        if (this != &other)
+        {
+            str = other.str;
+        }
+        return *this;
+    }
+
+    for_3171_base(for_3171_base&& other) noexcept
+        : str(std::move(other.str))
+    {}
+
+    for_3171_base& operator=(for_3171_base&& other) noexcept
+    {
+        if (this != &other)
+        {
+            str = std::move(other.str);
+        }
+        return *this;
+    }
 
     virtual void _from_json(const json& j)
     {
@@ -243,12 +269,43 @@ struct for_3171_base // NOLINT(cppcoreguidelines-special-member-functions)
     std::string str{}; // NOLINT(readability-redundant-member-init)
 };
 
+for_3171_base::~for_3171_base() = default;
+
 struct for_3171_derived : public for_3171_base
 {
     for_3171_derived() = default;
+    ~for_3171_derived() override;
     explicit for_3171_derived(const std::string& /*unused*/) { }
+
+    for_3171_derived(const for_3171_derived& other) // NOLINT(hicpp-use-equals-default,modernize-use-equals-default)
+        : for_3171_base(other)
+    {}
+
+    for_3171_derived& operator=(const for_3171_derived& other)
+    {
+        if (this != &other)
+        {
+            for_3171_base::operator=(other); // Call base class assignment operator
+        }
+        return *this;
+    }
+
+    for_3171_derived(for_3171_derived&& other) noexcept
+        : for_3171_base(std::move(other))
+    {}
+
+    for_3171_derived& operator=(for_3171_derived&& other) noexcept
+    {
+        if (this != &other)
+        {
+            for_3171_base::operator=(std::move(other)); // Call base class move assignment operator
+        }
+        return *this;
+    }
 };
 
+for_3171_derived::~for_3171_derived() = default;
+
 inline void from_json(const json& j, for_3171_base& tb) // NOLINT(misc-use-internal-linkage)
 {
     tb._from_json(j);