Fix weak vtables warnings

config/Make.rules.Darwin

@@ -48,10 +48,9 @@ cppflags        = -fvisibility=hidden -Wall -Wextra -Wshadow -Wshadow-all -Wredu
                   $(if $(filter yes,$(OPTIMIZE)),-O2 -DNDEBUG,-g) \
                   -std=c++20
 
-# TODO review this setting
-ifeq ($(MAXWARN),yes)
-    cppflags	+=  -Wweak-vtables
-endif
+# When we export an API from a shared library, we check for weak vtables. This flag also works for static
+# libraries/exes, but is not necessary.
+api_exports_cppflags = -Wweak-vtables
 
 # We compile static and shared with the same flags so we want their .o to share directories.
 static_objdir   = obj

cpp/include/DataStorm/InternalI.h

@@ -14,6 +14,7 @@
 #if defined(__clang__)
 #    pragma clang diagnostic push
 #    pragma clang diagnostic ignored "-Wshadow-field-in-constructor"
+#    pragma clang diagnostic ignored "-Wweak-vtables"
 #elif defined(__GNUC__)
 #    pragma GCC diagnostic push
 #    pragma GCC diagnostic ignored "-Wshadow"

cpp/include/Ice/BatchRequest.h

@@ -3,6 +3,8 @@
 #ifndef ICE_BATCH_REQUEST_H
 #define ICE_BATCH_REQUEST_H
 
+#include "Config.h"
+
 #include <string_view>
 
 namespace Ice
@@ -13,10 +15,10 @@ namespace Ice
      * Represents an invocation on a proxy configured for batch-oneway or batch-datagram.
      * \headerfile Ice/Ice.h
      */
-    class BatchRequest
+    class ICE_API BatchRequest
     {
     public:
-        virtual ~BatchRequest() = default;
+        virtual ~BatchRequest();
 
         /**
          * Queues the request for an eventual flush.

cpp/include/Ice/Connection.h

@@ -63,7 +63,7 @@ namespace Ice
     class ICE_API Connection
     {
     public:
-        virtual ~Connection() = 0;
+        virtual ~Connection();
 
         /**
          * Aborts this connection.
@@ -265,6 +265,8 @@ namespace Ice
         IPConnectionInfo(const IPConnectionInfo&) = delete;
         IPConnectionInfo& operator=(const IPConnectionInfo&) = delete;
 
+        ~IPConnectionInfo() override;
+
         /**
          * The local address.
          */

cpp/include/Ice/Endpoint.h

@@ -112,6 +112,8 @@ namespace Ice
         IPEndpointInfo(const IPEndpointInfo&) = delete;
         IPEndpointInfo& operator=(const IPEndpointInfo&) = delete;
 
+        ~IPEndpointInfo() override;
+
         /**
          * The host or address configured with the endpoint.
          */

cpp/include/Ice/Instrumentation.h

@@ -12,6 +12,11 @@
 #include <memory>
 #include <optional>
 
+#if defined(__clang__)
+#    pragma clang diagnostic push
+#    pragma clang diagnostic ignored "-Wweak-vtables"
+#endif
+
 namespace Ice
 {
     struct Current;
@@ -372,4 +377,8 @@ namespace Ice::Instrumentation
     };
 }
 
+#if defined(__clang__)
+#    pragma clang diagnostic pop
+#endif
+
 #endif

cpp/include/Ice/Logger.h

@@ -3,6 +3,8 @@
 #ifndef ICE_LOGGER_H
 #define ICE_LOGGER_H
 
+#include "Config.h"
+
 #include <memory>
 #include <string>
 
@@ -15,10 +17,10 @@ namespace Ice
      * The Ice message logger. Applications can provide their own logger by implementing this interface and installing
      * it in a communicator. \headerfile Ice/Ice.h
      */
-    class Logger
+    class ICE_API Logger
     {
     public:
-        virtual ~Logger() = default;
+        virtual ~Logger();
 
         // We use const std::string& and not std::string_view for the log messages because implementations commonly
         // send the message to C APIs that require null-terminated strings.

cpp/include/Ice/MarshaledResult.h

@@ -20,7 +20,7 @@ namespace Ice
         MarshaledResult(const MarshaledResult&) = delete;
         MarshaledResult(MarshaledResult&&) = default;
 
-        virtual ~MarshaledResult() = default;
+        virtual ~MarshaledResult();
 
         MarshaledResult& operator=(const MarshaledResult&) = delete;
         MarshaledResult& operator=(MarshaledResult&&) = default;

cpp/include/Ice/MetricsObserverI.h

@@ -14,6 +14,11 @@
 #include <sstream>
 #include <stdexcept>
 
+#if defined(__clang__)
+#    pragma clang diagnostic push
+#    pragma clang diagnostic ignored "-Wweak-vtables"
+#endif
+
 namespace IceInternal
 {
     class StopWatch
@@ -577,4 +582,8 @@ namespace IceMX
     /// \endcond
 }
 
+#if defined(__clang__)
+#    pragma clang diagnostic pop
+#endif
+
 #endif

cpp/include/Ice/NativePropertiesAdmin.h

@@ -19,7 +19,7 @@ namespace Ice
     class ICE_API NativePropertiesAdmin
     {
     public:
-        virtual ~NativePropertiesAdmin() = 0;
+        virtual ~NativePropertiesAdmin();
 
         /**
          * Register an update callback that will be invoked when property updates occur.

cpp/include/Ice/ObjectAdapter.h

@@ -26,10 +26,10 @@ namespace Ice
      * @see ServantLocator
      * \headerfile Ice/Ice.h
      */
-    class ObjectAdapter
+    class ICE_API ObjectAdapter
     {
     public:
-        virtual ~ObjectAdapter() = default;
+        virtual ~ObjectAdapter();
 
         /**
          * Get the name of this object adapter.

cpp/include/Ice/OutgoingAsync.h

@@ -23,6 +23,7 @@
 #    pragma clang diagnostic push
 // See #2747
 #    pragma clang diagnostic ignored "-Wshadow-uncaptured-local"
+#    pragma clang diagnostic ignored "-Wweak-vtables"
 #endif
 
 namespace IceInternal

cpp/include/Ice/Plugin.h

@@ -15,10 +15,10 @@ namespace Ice
      * invokes {@link Plugin#initialize} on each one.
      * \headerfile Ice/Ice.h
      */
-    class Plugin
+    class ICE_API Plugin
     {
     public:
-        virtual ~Plugin() = default;
+        virtual ~Plugin();
 
         /**
          * Perform any necessary initialization steps.
@@ -36,10 +36,10 @@ namespace Ice
      * Each communicator has a plug-in manager to administer the set of plug-ins.
      * \headerfile Ice/Ice.h
      */
-    class PluginManager
+    class ICE_API PluginManager
     {
     public:
-        virtual ~PluginManager() = default;
+        virtual ~PluginManager();
 
         /**
          * Initialize the configured plug-ins. The communicator automatically initializes the plug-ins by default, but

cpp/include/Ice/Proxy.h

@@ -301,7 +301,7 @@ namespace Ice
 
         ObjectPrx(const Ice::CommunicatorPtr& communicator, std::string_view proxyString);
 
-        virtual ~ObjectPrx() = default;
+        virtual ~ObjectPrx();
 
         ObjectPrx& operator=(const ObjectPrx& rhs) noexcept = default;
         ObjectPrx& operator=(ObjectPrx&& rhs) noexcept = default;

cpp/include/Ice/ServantLocator.h

@@ -15,10 +15,10 @@ namespace Ice
      * @see ObjectAdapter#findServantLocator
      * \headerfile Ice/Ice.h
      */
-    class ServantLocator
+    class ICE_API ServantLocator
     {
     public:
-        virtual ~ServantLocator() = default;
+        virtual ~ServantLocator();
 
         /**
          * Called before a request is dispatched if a servant cannot be found in the object adapter's active servant

cpp/include/Ice/Timer.h

@@ -30,6 +30,11 @@ namespace IceInternal
     };
     using TimerTaskPtr = std::shared_ptr<TimerTask>;
 
+#ifdef __clang__
+#    pragma clang diagnostic push
+#    pragma clang diagnostic ignored "-Wweak-vtables"
+#endif
+
     // Adapts a function<void()> to a TimerTask.
     class InlineTimerTask final : public TimerTask
     {
@@ -42,6 +47,10 @@ namespace IceInternal
         std::function<void()> _function;
     };
 
+#ifdef __clang__
+#    pragma clang diagnostic pop
+#endif
+
     // The timer class is used to schedule tasks for one-time execution or repeated execution. Tasks are executed by a
     // dedicated timer thread sequentially.
     class ICE_API Timer

cpp/include/Ice/ValueFactory.h

@@ -32,7 +32,7 @@ namespace Ice
     class ICE_API ValueFactoryManager
     {
     public:
-        virtual ~ValueFactoryManager() = 0;
+        virtual ~ValueFactoryManager();
 
         /**
          * Adds a value factory. Attempting to add a factory with a type ID for which a factory is already registered

cpp/include/IceBox/Service.h

@@ -32,10 +32,10 @@ namespace IceBox
      * An application service managed by a {@link ServiceManager}.
      * \headerfile IceBox/IceBox.h
      */
-    class Service
+    class ICEBOX_API Service
     {
     public:
-        virtual ~Service() = default;
+        virtual ~Service();
 
         /**
          * Start the service. The given communicator is created by the {@link ServiceManager} for use by the service.

cpp/src/DataStorm/Makefile.mk

@@ -6,7 +6,7 @@ $(project)_generated_includedir := $(project)/generated/DataStorm
 
 DataStorm_sliceflags    := --include-dir DataStorm
 DataStorm_targetdir     := $(libdir)
-DataStorm_cppflags      := -DDATASTORM_API_EXPORTS
+DataStorm_cppflags      := -DDATASTORM_API_EXPORTS $(api_exports_cppflags)
 DataStorm_dependencies  := Ice
 
 projects += $(project)

cpp/src/Glacier2CryptPermissionsVerifier/Makefile.mk

@@ -4,7 +4,7 @@ $(project)_libraries                            += Glacier2CryptPermissionsVerif
 
 Glacier2CryptPermissionsVerifier_targetdir        := $(libdir)
 Glacier2CryptPermissionsVerifier_dependencies     := Glacier2 Ice
-Glacier2CryptPermissionsVerifier_cppflags         := -DCRYPT_PERMISSIONS_VERIFIER_API_EXPORTS
+Glacier2CryptPermissionsVerifier_cppflags         := -DCRYPT_PERMISSIONS_VERIFIER_API_EXPORTS $(api_exports_cppflags)
 Glacier2CryptPermissionsVerifier_devinstall       := no
 
 projects += $(project)

cpp/src/Glacier2Lib/Makefile.mk

@@ -4,7 +4,7 @@ $(project)_libraries    := Glacier2
 
 Glacier2_targetdir              := $(libdir)
 Glacier2_dependencies           := Ice
-Glacier2_cppflags               := -DGLACIER2_API_EXPORTS
+Glacier2_cppflags               := $(api_exports_cppflags)
 Glacier2_sliceflags             := --include-dir Glacier2
 
 projects += $(project)

cpp/src/Ice/Acceptor.h

@@ -8,6 +8,11 @@
 #include "Network.h"
 #include "TransceiverF.h"
 
+#ifdef __clang__
+#    pragma clang diagnostic push
+#    pragma clang diagnostic ignored "-Wweak-vtables"
+#endif
+
 namespace IceInternal
 {
     class Acceptor
@@ -29,4 +34,8 @@ namespace IceInternal
     };
 }
 
+#ifdef __clang__
+#    pragma clang diagnostic pop
+#endif
+
 #endif

cpp/src/Ice/BatchRequestQueue.cpp

@@ -41,6 +41,8 @@ namespace
     };
 }
 
+BatchRequest::~BatchRequest() = default; // avoid weak vtable
+
 BatchRequestQueue::BatchRequestQueue(const InstancePtr& instance, bool datagram)
     : _interceptor(instance->initializationData().batchRequestInterceptor),
       _batchStream(instance.get(), Ice::currentProtocolEncoding),

cpp/src/Ice/Connection.cpp

@@ -7,12 +7,13 @@ using namespace std;
 
 // Implement virtual destructors out of line to avoid weak vtables.
 Ice::ConnectionInfo::~ConnectionInfo() = default;
+Ice::IPConnectionInfo::~IPConnectionInfo() = default;
 Ice::TCPConnectionInfo::~TCPConnectionInfo() = default;
 Ice::UDPConnectionInfo::~UDPConnectionInfo() = default;
 Ice::WSConnectionInfo::~WSConnectionInfo() = default;
 Ice::IAPConnectionInfo::~IAPConnectionInfo() = default;
 
-Ice::Connection::~Connection() = default;
+Ice::Connection::~Connection() = default; // avoid weak vtable
 
 void
 Ice::Connection::flushBatchRequests(CompressBatch compress)

cpp/src/Ice/Connector.h

@@ -9,6 +9,11 @@
 #include <cstdint>
 #include <string>
 
+#ifdef __clang__
+#    pragma clang diagnostic push
+#    pragma clang diagnostic ignored "-Wweak-vtables"
+#endif
+
 namespace IceInternal
 {
     class Connector
@@ -26,4 +31,8 @@ namespace IceInternal
     };
 }
 
+#ifdef __clang__
+#    pragma clang diagnostic pop
+#endif
+
 #endif

cpp/src/Ice/EndpointI.cpp

@@ -29,14 +29,11 @@ Ice::EndpointInfo::secure() const noexcept
     return underlying ? underlying->secure() : false;
 }
 
+Ice::IPEndpointInfo::~IPEndpointInfo() = default;
 Ice::TCPEndpointInfo::~TCPEndpointInfo() = default;
-
 Ice::UDPEndpointInfo::~UDPEndpointInfo() = default;
-
 Ice::WSEndpointInfo::~WSEndpointInfo() = default;
-
 Ice::IAPEndpointInfo::~IAPEndpointInfo() = default;
-
 Ice::OpaqueEndpointInfo::~OpaqueEndpointInfo() = default;
 
 void

cpp/src/Ice/LocatorInfo.cpp

@@ -396,6 +396,8 @@ IceInternal::LocatorInfo::Request::addCallback(
     }
 }
 
+IceInternal::LocatorInfo::GetEndpointsCallback::~GetEndpointsCallback() = default; // avoid weak vtable
+
 IceInternal::LocatorInfo::Request::Request(LocatorInfoPtr locatorInfo, ReferencePtr ref)
     : _locatorInfo(std::move(locatorInfo)),
       _reference(std::move(ref)),
@@ -404,6 +406,8 @@ IceInternal::LocatorInfo::Request::Request(LocatorInfoPtr locatorInfo, Reference
 {
 }
 
+IceInternal::LocatorInfo::Request::~Request() = default; // avoid weak vtable
+
 void
 IceInternal::LocatorInfo::Request::response(const optional<ObjectPrx>& proxy)
 {