Improve loplugin:dyncastvisibility to check for non-inline key functions

This would have caught the issue discussed in
709b1f3ddb "Make sure VCLXPopupMenu has unique
RTTI".  (The commit message talks about RTTI there, while what Clang actually
compared for an optimized implementation of a dynamic_cast to a final class is
vtable pointers, but the overall picture remains the same.  Both RTTI and
vtables are emitted along the key function, and if that is missing or inline,
they are emitted for each dynamic library individually, and as internal symbols
on macOS.)

This commit also addresses all the issues found by the improved
loplugin:dyncastvisibility on Linux.  See the newly added TODO in
compilerplugins/clang/dyncastvisibility.cxx and
86b86ac87e "Give DocumentEventHolder (aka
EventHolder<DocumentEvent>) a key function" for an issue with key functions for
class template instantiations.

Change-Id: Ia19155efb1d23692c92b9c97ff17f18ae7a1f3ee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/176576
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
Tested-by: Jenkins

basic/source/sbx/sbxvar.cxx

@@ -569,6 +569,8 @@ SbxInfo::SbxInfo( OUString a, sal_uInt32 n )
        : aHelpFile(std::move( a )), nHelpId( n )
 {}
 
+SbxHint::~SbxHint() = default;
+
 void SbxVariable::Dump( SvStream& rStrm, bool bFill )
 {
     OString aBNameStr(OUStringToOString(GetName( SbxNameType::ShortTypes ), RTL_TEXTENCODING_ASCII_US));

compilerplugins/clang/dyncastvisibility.cxx

@@ -66,7 +66,7 @@ bool isDerivedFrom(
             }
             derived = true;
         }
-    }
+        }
     return derived;
 }
 
@@ -157,7 +157,14 @@ public:
         assert(rds != nullptr);
         Bases bs;
         bool hidden = false;
-        if (!(isDerivedFrom(rdd, rds, &bs, &hidden) && hidden)) {
+        if (!isDerivedFrom(rdd, rds, &bs, &hidden)) {
+            return true;
+        }
+        Decl const * missing = nullptr;
+        if (rdd->isEffectivelyFinal()) {
+            missing = missingKeyFunction(rdd);
+        }
+        if (!hidden && missing == nullptr) {
             return true;
         }
         report(
@@ -167,26 +174,44 @@ public:
             expr->getExprLoc())
             << ts << vis(getTypeVisibility(rds)) << td
             << vis(getTypeVisibility(rdd)) << expr->getSourceRange();
-        report(
+        if (hidden) {
-            DiagnosticsEngine::Note,
+            report(
-            "base class %0 with %1 type visibility defined here",
+                DiagnosticsEngine::Note,
-            rds->getLocation())
+                "base class %0 with %1 type visibility defined here",
-            << ts << vis(getTypeVisibility(rds)) << rds->getSourceRange();
+                rds->getLocation())
-        for (auto const i: bs) {
+                << ts << vis(getTypeVisibility(rds)) << rds->getSourceRange();
-            if (getTypeVisibility(i) != DefaultVisibility) {
+            for (auto const i: bs) {
+                if (getTypeVisibility(i) != DefaultVisibility) {
+                    report(
+                        DiagnosticsEngine::Note,
+                        ("intermediary class %0 with %1 type visibility defined"
+                         " here"),
+                        i->getLocation())
+                        << i << vis(getTypeVisibility(i)) << i->getSourceRange();
+                }
+            }
+            report(
+                DiagnosticsEngine::Note,
+                "derived class %0 with %1 type visibility defined here",
+                rdd->getLocation())
+                << td << vis(getTypeVisibility(rdd)) << rdd->getSourceRange();
+        }
+        if (missing != nullptr) {
+            if (isa<CXXRecordDecl>(missing)) {
                 report(
                     DiagnosticsEngine::Note,
-                    ("intermediary class %0 with %1 type visibility defined"
+                    "derived class %0 does not have a key function (at least on some platforms)",
-                     " here"),
+                    missing->getLocation())
-                    i->getLocation())
+                    << td << missing->getSourceRange();
-                    << i << vis(getTypeVisibility(i)) << i->getSourceRange();
+            } else {
+                report(
+                    DiagnosticsEngine::Note,
+                    "derived class %0 has a key function (at least on some platforms) that is"
+                        " inline",
+                    missing->getLocation())
+                    << td << missing->getSourceRange();
             }
         }
-        report(
-            DiagnosticsEngine::Note,
-            "derived class %0 with %1 type visibility defined here",
-            rdd->getLocation())
-            << td << vis(getTypeVisibility(rdd)) << rdd->getSourceRange();
         return true;
     }
 
@@ -199,6 +224,29 @@ private:
             TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
         }
     }
+
+    Decl const * missingKeyFunction(CXXRecordDecl const * decl) {
+        auto const md = compiler.getASTContext().getCurrentKeyFunction(decl);
+        if (md != nullptr && !md->isInlined()) {
+            return nullptr;
+        }
+        // Ignore classes defined in the main file:
+        auto const def = decl->getDefinition();
+        assert(def != nullptr);
+        if (compiler.getSourceManager().isInMainFile(def->getLocation())) {
+            return nullptr;
+        }
+        //TODO: Ignore template instantiations, for which any key function would necessarily be
+        // inline, unless there is an explicit extern template instantiation (as there should
+        // arguably be for such cases, cf. comphelper::DocumentEventHolder in
+        // include/comphelper/asyncnotification.hxx, but which might be complicated to check here):
+        auto const tsk = decl->getTemplateSpecializationKind();
+        if (tsk == TSK_ImplicitInstantiation || tsk == TSK_ExplicitInstantiationDeclaration) {
+            return nullptr;
+        }
+        return md == nullptr ? static_cast<Decl const *>(decl) : static_cast<Decl const *>(md);
+    }
+
 };
 
 static loplugin::Plugin::Registration<DynCastVisibility> dyncastvisibility(

emfio/inc/mtftools.hxx

@@ -392,6 +392,8 @@ namespace emfio
         vcl::Font    aFont;
 
         explicit WinMtfFontStyle(LOGFONTW const & rLogFont);
+
+        ~WinMtfFontStyle() override;
     };
 
     enum class WinMtfFillStyleType

emfio/source/reader/mtftools.cxx

@@ -308,6 +308,8 @@ namespace emfio
 #endif
     };
 
+    WinMtfFontStyle::~WinMtfFontStyle() = default;
+
     // tdf#127471
     ScaledFontDetectCorrectHelper::ScaledFontDetectCorrectHelper()
     {

include/basic/sbx.hxx

@@ -82,6 +82,7 @@ class BASIC_DLLPUBLIC SbxHint final : public SfxHint
     SbxVariable* pVar;
 public:
     SbxHint( SfxHintId n, SbxVariable* v ) : SfxHint( n ), pVar( v ) {}
+    ~SbxHint() override;
     SbxVariable* GetVar() const { return pVar; }
 };
 

include/sfx2/app.hxx

@@ -78,11 +78,17 @@ enum class SfxToolsModule
 class SFX2_DLLPUBLIC SfxLinkItem final : public SfxPoolItem
 {
     Link<SfxPoolItem const *, void> aLink;
+
+    SfxLinkItem(SfxLinkItem const &) = default;
+    void operator =(SfxLinkItem const &) = delete;
+
 public:
     SfxLinkItem( sal_uInt16 nWhichId, const Link<SfxPoolItem const *, void>& rValue )
         : SfxPoolItem( nWhichId, SfxItemType::SfxLinkItemType )
     {   aLink = rValue; }
 
+    virtual ~SfxLinkItem() override;
+
     virtual SfxLinkItem*     Clone( SfxItemPool* = nullptr ) const override
     {   return new SfxLinkItem( *this ); }
     virtual bool             operator==( const SfxPoolItem& rL) const override

include/svx/ClassificationField.hxx

@@ -74,6 +74,8 @@ public:
         , msIdentifier(std::move(sIdentifier))
     {}
 
+    ~ClassificationField() override;
+
     std::unique_ptr<SvxFieldData> Clone() const override
     {
         return std::make_unique<ClassificationField>(meType, msDescription, msFullClassName, msIdentifier);

include/svx/sxelditm.hxx

@@ -24,12 +24,16 @@
 
 class SAL_DLLPUBLIC_RTTI SdrEdgeLineDeltaCountItem final : public SfxUInt16Item
 {
+    SdrEdgeLineDeltaCountItem(SdrEdgeLineDeltaCountItem const&) = default;
+    void operator=(SdrEdgeLineDeltaCountItem const&) = delete;
+
 public:
     SdrEdgeLineDeltaCountItem(sal_uInt16 nVal = 0)
         : SfxUInt16Item(SDRATTR_EDGELINEDELTACOUNT, nVal,
                         SfxItemType::SdrEdgeLineDeltaCountItemType)
     {
     }
+    virtual ~SdrEdgeLineDeltaCountItem() override;
     virtual SdrEdgeLineDeltaCountItem* Clone(SfxItemPool*) const override
     {
         return new SdrEdgeLineDeltaCountItem(*this);

sfx2/source/appl/app.cxx

@@ -60,6 +60,8 @@
 
 using namespace ::com::sun::star;
 
+SfxLinkItem::~SfxLinkItem() = default;
+
 static SfxApplication* g_pSfxApplication = nullptr;
 
 #if HAVE_FEATURE_XMLHELP

svx/Library_svx.mk

@@ -140,6 +140,7 @@ $(eval $(call gb_Library_add_exception_objects,svx,\
     svx/source/dialog/ClassificationCommon \
     svx/source/dialog/ClassificationDialog \
     svx/source/dialog/ClassificationEditView \
+    svx/source/dialog/ClassificationField \
     svx/source/dialog/databaseregistrationui \
     svx/source/dialog/dialcontrol \
     svx/source/dialog/dlgctl3d \

svx/Library_svxcore.mk

@@ -406,6 +406,7 @@ $(eval $(call gb_Library_add_exception_objects,svxcore,\
     svx/source/svdraw/svdview \
     svx/source/svdraw/svdviter \
     svx/source/svdraw/svdxcgv \
+    svx/source/svdraw/sxelditm \
     svx/source/svdraw/textchain \
     svx/source/svdraw/textchainflow \
     svx/source/svdraw/textchaincursor \

svx/source/dialog/ClassificationField.cxx

@@ -0,0 +1,16 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <sal/config.h>
+
+#include <svx/ClassificationField.hxx>
+
+svx::ClassificationField::~ClassificationField() = default;
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

svx/source/svdraw/sxelditm.cxx

@@ -0,0 +1,16 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <sal/config.h>
+
+#include <svx/sxelditm.hxx>
+
+SdrEdgeLineDeltaCountItem::~SdrEdgeLineDeltaCountItem() = default;
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

sw/inc/ftninfo.hxx

@@ -108,6 +108,8 @@ public:
 
     SwFootnoteInfo();
     SwFootnoteInfo(const SwFootnoteInfo&);
+
+    ~SwFootnoteInfo() override;
 };
 
 #endif

sw/source/core/doc/docftn.cxx

@@ -311,6 +311,8 @@ SwFootnoteInfo::SwFootnoteInfo() :
     m_bEndNote = false;
 }
 
+SwFootnoteInfo::~SwFootnoteInfo() = default;
+
 void SwDoc::SetFootnoteInfo(const SwFootnoteInfo& rInfo)
 {
     SwRootFrame* pTmpRoot = getIDocumentLayoutAccess().GetCurrentLayout();