From 47f5390b18397b1f5e9f4ccf6626a97f647ac510 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Tue, 15 Apr 2025 22:42:34 +0200 Subject: [PATCH] Upgrade: address warnings -- shaddowing overloaded-virtual This is an advanced diagnostics added (presumably) with GCC-13 and attempts to protect against an insidious side-effect of ''overload resolution'' Basically C++ (like its ancestor C) is oriented towards direct linkage and adds the OO-style dynamic dispatch (through virtual functions and a VTable) only as an extension, which must be requested explicitly. Thus the resolution of ''overloads'' (as opposed to ''overridden'' virtual functions) always takes precedence and happens within the directly visible scope, which can cause the compiler to perform an implicit conversion instead of invoking a different virtual function, which is defined in a base class. However, this diagnostics seems to be implemented in an overly zealous way: The compiler warns at the time of the type instantiation, and even in cases where it is effectively impossible to encounter this dangerous shadowing situation. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109740 This leads to several ill-guided warnings in the Lumiera code base, which unfortunately can only be addressed by disabling this diagnostics for all usages of some header. The reason is, we often generate chains of template instantiations driven by type lists, and in such usage pattern, it is not even possible to bring all the other inherited overloads into scope (with a using `BASE::func` clause), because such a specification would be ambiguous and result in a real compile error, because even the interface is generated from a chain of mix-in templates --- src/lib/meta/tuple-record-init.hpp | 9 ++++++++- src/lib/meta/virtual-copy-support.hpp | 18 ++++++++++++++++-- .../mobject/session/query/fake-configrules.hpp | 11 +++++++++-- tests/library/meta/generator-test.cpp | 7 +++++++ 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/lib/meta/tuple-record-init.hpp b/src/lib/meta/tuple-record-init.hpp index 3742fd980..6ac6e7247 100644 --- a/src/lib/meta/tuple-record-init.hpp +++ b/src/lib/meta/tuple-record-init.hpp @@ -57,6 +57,13 @@ #include "lib/meta/trait.hpp" +// GCC > 13 warns at class definition when a new overload shadows an inherited virtual function. +// While theoretically correct, in practice any call will be dispatched through the base interface, +// which in turn is also generated by metaprogramming from a type list. +// See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109740 +_Pragma("GCC diagnostic push") \ +_Pragma("GCC diagnostic ignored \"-Woverloaded-virtual\"") + namespace lib { namespace meta { @@ -159,7 +166,7 @@ namespace meta { return accessTargetElement(); } }; - + }//(End)implementation details diff --git a/src/lib/meta/virtual-copy-support.hpp b/src/lib/meta/virtual-copy-support.hpp index 44b9a6f75..6e90094e9 100644 --- a/src/lib/meta/virtual-copy-support.hpp +++ b/src/lib/meta/virtual-copy-support.hpp @@ -148,6 +148,7 @@ namespace meta{ class NoCopyMoveSupport : public B { + public: virtual void copyInto (void*) const override { @@ -178,6 +179,7 @@ namespace meta{ class MoveSupport : public NoCopyMoveSupport { + public: virtual void copyInto (void*) const override { @@ -190,6 +192,10 @@ namespace meta{ D& src = static_cast (*this); new(targetStorage) D(move(src)); } + + // irrelevant for any practical usage, but formally required + using NoCopyMoveSupport::copyInto; + using NoCopyMoveSupport::moveInto; }; @@ -197,12 +203,15 @@ namespace meta{ class CloneSupport : public MoveSupport { + public: virtual void copyInto (void* targetStorage) const override { D const& src = static_cast (*this); new(targetStorage) D(src); } + + using MoveSupport::copyInto; }; @@ -210,6 +219,7 @@ namespace meta{ class FullCopySupport : public CloneSupport { + public: virtual void copyInto (I& target) const override { @@ -225,8 +235,12 @@ namespace meta{ D& s = static_cast (*this); t = move(s); } - }; - + + using CloneSupport::copyInto; + using CloneSupport::moveInto; + }; // see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109740 + // In our case here, calls will always be through VirtualCopySupportInterface + // and void* is incompatible with I&, so the warning is irrelevant... diff --git a/src/steam/mobject/session/query/fake-configrules.hpp b/src/steam/mobject/session/query/fake-configrules.hpp index ada86ed59..caac535b6 100644 --- a/src/steam/mobject/session/query/fake-configrules.hpp +++ b/src/steam/mobject/session/query/fake-configrules.hpp @@ -157,6 +157,15 @@ namespace session { }; + // GCC > 13 warns at class definition when a new overload shadows an inherited virtual function. + // While theoretically correct, in practice any call will be dispatched through the base interface + // and it is not even possible to reach the concrete classes (and the implementation function is private). + // The workaround with a `using` clause is not applicable, since the name is ambiguous (by design) + // See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109740 + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Woverloaded-virtual\"") + + /** * building block providing the * mock implementation for a \em single type. @@ -167,7 +176,6 @@ namespace session { { typedef typename WrapReturn::Wrapper Ret; - /** (dummy) implementation of the QueryHandler interface */ virtual bool resolve (Ret& solution, Query const& q) @@ -184,7 +192,6 @@ namespace session { return try_special_case(solution, q); } - private: bool try_special_case (Ret& solution, Query const& q) { diff --git a/tests/library/meta/generator-test.cpp b/tests/library/meta/generator-test.cpp index ed2777460..3b4f04df1 100644 --- a/tests/library/meta/generator-test.cpp +++ b/tests/library/meta/generator-test.cpp @@ -39,6 +39,13 @@ using util::_Fmt; using std::string; using std::cout; +// GCC > 13 warns at class definition when a new overload shadows an inherited virtual function. +// While theoretically correct, this warning is besides the point when an interface is assembled +// by metaprogramming from a chain of template instantiations, driven by a type list +// See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109740 +_Pragma("GCC diagnostic push") \ +_Pragma("GCC diagnostic ignored \"-Woverloaded-virtual\"") + namespace lib { namespace meta {