resurrect and improve loplugin:referencecasting

Improve the plugin to avoid generating false+ with the special case of
querying XInterface (what the code calls normalisation).

Also ignore places where the querying is dealing with ambiguous base
classes.

Change-Id: I23b2b2fa6618328fafc4707b94c26582a462ea87
Reviewed-on: https://gerrit.libreoffice.org/74993
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

1 files changed, 385 insertions, 0 deletions

diff --git a/compilerplugins/clang/referencecasting.cxx b/compilerplugins/clang/referencecasting.cxx
new file mode 100644
index 000000000000..e60177f4effb
--- /dev/null
+++ b/compilerplugins/clang/referencecasting.cxx
@@ -0,0 +1,385 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * Based on LLVM/Clang.
+ *
+ * This file is distributed under the University of Illinois Open Source
+ * License. See LICENSE.TXT for details.
+ *
+ */
+
+#include "plugin.hxx"
+#include "check.hxx"
+#include <iostream>
+
+/*
+This is a compile-time checker.
+
+Check for cases where we have
+ - two IDL interfaces A and B,
+ - B extends A
+ - we are converting a Reference<B> to a Reference<A> using UNO_QUERY
+
+This makes the code simpler and cheaper, because UNO_QUERY can be surprisingly expensive if used a lot.
+
+*/
+
+class ReferenceCasting : public loplugin::FilteringPlugin<ReferenceCasting>
+{
+public:
+    explicit ReferenceCasting(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+    void run() override
+    {
+        std::string fn(handler.getMainFileName());
+        loplugin::normalizeDotDotInFilePath(fn);
+        // macros
+        if (fn == SRCDIR "/dbaccess/source/ui/browser/formadapter.cxx")
+            return;
+        TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    }
+    bool VisitCXXConstructExpr(const CXXConstructExpr* cce);
+    bool VisitCXXMemberCallExpr(const CXXMemberCallExpr* mce);
+
+private:
+    bool CheckForUnnecessaryGet(const Expr*);
+};
+
+static const RecordType* extractTemplateType(const clang::Type*);
+static bool isDerivedFrom(const CXXRecordDecl* subtypeRecord, const CXXRecordDecl* baseRecord);
+
+bool ReferenceCasting::VisitCXXConstructExpr(const CXXConstructExpr* cce)
+{
+    // don't bother processing anything in the Reference.h file. Makes my life easier when debugging this.
+    StringRef aFileName = getFileNameOfSpellingLoc(
+        compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(cce)));
+    if (loplugin::isSamePathname(aFileName, SRCDIR "/include/com/sun/star/uno/Reference.h"))
+        return true;
+    if (loplugin::isSamePathname(aFileName, SRCDIR "/include/com/sun/star/uno/Reference.hxx"))
+        return true;
+
+    // look for calls to the Reference<T>(x, UNO_something) constructor
+    auto constructorClass = cce->getConstructor()->getParent();
+    if (!constructorClass->getIdentifier() || constructorClass->getName() != "Reference")
+        return true;
+
+    if (cce->getNumArgs() != 2)
+        return true;
+
+    if (CheckForUnnecessaryGet(cce->getArg(0)))
+        report(DiagnosticsEngine::Warning, "unnecessary get() call", compat::getBeginLoc(cce))
+            << cce->getSourceRange();
+
+    // ignore the up-casting constructor
+    if (!isa<EnumType>(cce->getConstructor()->getParamDecl(1)->getType()))
+        return true;
+
+    // extract the type parameter passed to the template
+    const RecordType* templateParamType = extractTemplateType(cce->getType().getTypePtr());
+    if (!templateParamType)
+        return true;
+
+    // extract the type of the first parameter passed to the constructor
+    const Expr* constructorArg0 = cce->getArg(0);
+    if (!constructorArg0)
+        return true;
+
+    // drill down the expression tree till we hit the bottom, because at the top, the type is BaseReference
+    const clang::Type* argType;
+    for (;;)
+    {
+        if (auto castExpr = dyn_cast<CastExpr>(constructorArg0))
+        {
+            constructorArg0 = castExpr->getSubExpr();
+            continue;
+        }
+        if (auto matTempExpr = dyn_cast<MaterializeTemporaryExpr>(constructorArg0))
+        {
+            constructorArg0 = matTempExpr->GetTemporaryExpr();
+            continue;
+        }
+        if (auto bindTempExpr = dyn_cast<CXXBindTemporaryExpr>(constructorArg0))
+        {
+            constructorArg0 = bindTempExpr->getSubExpr();
+            continue;
+        }
+        if (auto tempObjExpr = dyn_cast<CXXTemporaryObjectExpr>(constructorArg0))
+        {
+            constructorArg0 = tempObjExpr->getArg(0);
+            continue;
+        }
+        if (auto parenExpr = dyn_cast<ParenExpr>(constructorArg0))
+        {
+            constructorArg0 = parenExpr->getSubExpr();
+            continue;
+        }
+        argType = constructorArg0->getType().getTypePtr();
+        break;
+    }
+
+    const RecordType* argTemplateType = extractTemplateType(argType);
+    if (!argTemplateType)
+        return true;
+
+    CXXRecordDecl* templateParamRD = dyn_cast<CXXRecordDecl>(templateParamType->getDecl());
+    CXXRecordDecl* constructorArgRD = dyn_cast<CXXRecordDecl>(argTemplateType->getDecl());
+
+    // querying for XInterface (instead of doing an upcast) has special semantics,
+    // to check for UNO object equivalence.
+    if (templateParamRD->getName() == "XInterface")
+        return true;
+
+    // XShape is used in UNO aggregates in very "entertaining" ways, which means an UNO_QUERY
+    // can return a completely different object, e.g. see SwXShape::queryInterface
+    if (templateParamRD->getName() == "XShape")
+        return true;
+
+    if (auto declRefExpr = dyn_cast<DeclRefExpr>(cce->getArg(1)))
+    {
+        // no warning expected, used to reject null references
+        if (auto enumConstantDecl = dyn_cast<EnumConstantDecl>(declRefExpr->getDecl()))
+        {
+            if (enumConstantDecl->getName() == "UNO_SET_THROW")
+                return true;
+            if (enumConstantDecl->getName() == "UNO_QUERY_THROW")
+                return true;
+            if (enumConstantDecl->getName() == "SAL_NO_ACQUIRE")
+                return true;
+        }
+    }
+
+    if (constructorArgRD->Equals(templateParamRD)
+        || isDerivedFrom(constructorArgRD, templateParamRD))
+    {
+        report(DiagnosticsEngine::Warning,
+               "the source reference is already a subtype of the destination reference, just use =",
+               compat::getBeginLoc(cce))
+            << cce->getSourceRange();
+    }
+    return true;
+}
+
+bool ReferenceCasting::VisitCXXMemberCallExpr(const CXXMemberCallExpr* mce)
+{
+    // don't bother processing anything in the Reference.h file. Makes my life easier when debugging this.
+    StringRef aFileName = getFileNameOfSpellingLoc(
+        compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(mce)));
+    if (loplugin::isSamePathname(aFileName, SRCDIR "/include/com/sun/star/uno/Reference.h"))
+        return true;
+    if (loplugin::isSamePathname(aFileName, SRCDIR "/include/com/sun/star/uno/Reference.hxx"))
+        return true;
+
+    // look for calls to the Reference<T>.set(x, UNO_QUERY) constructor
+    auto method = mce->getMethodDecl();
+    if (!method || !method->getIdentifier() || method->getName() != "set")
+        return true;
+    if (mce->getNumArgs() != 2)
+        return true;
+
+    auto methodRecordDecl = dyn_cast<ClassTemplateSpecializationDecl>(mce->getRecordDecl());
+    if (!methodRecordDecl || !methodRecordDecl->getIdentifier()
+        || methodRecordDecl->getName() != "Reference")
+        return true;
+
+    if (CheckForUnnecessaryGet(mce->getArg(0)))
+        report(DiagnosticsEngine::Warning, "unnecessary get() call", compat::getBeginLoc(mce))
+            << mce->getSourceRange();
+
+    // extract the type parameter passed to the template
+    const RecordType* templateParamType
+        = dyn_cast<RecordType>(methodRecordDecl->getTemplateArgs()[0].getAsType());
+    if (!templateParamType)
+        return true;
+
+    // extract the type of the first parameter passed to the method
+    const Expr* arg0 = mce->getArg(0);
+    if (!arg0)
+        return true;
+
+    // drill down the expression tree till we hit the bottom, because at the top, the type is BaseReference
+    const clang::Type* argType;
+    for (;;)
+    {
+        if (auto castExpr = dyn_cast<CastExpr>(arg0))
+        {
+            arg0 = castExpr->getSubExpr();
+            continue;
+        }
+        if (auto matTempExpr = dyn_cast<MaterializeTemporaryExpr>(arg0))
+        {
+            arg0 = matTempExpr->GetTemporaryExpr();
+            continue;
+        }
+        if (auto bindTempExpr = dyn_cast<CXXBindTemporaryExpr>(arg0))
+        {
+            arg0 = bindTempExpr->getSubExpr();
+            continue;
+        }
+        if (auto tempObjExpr = dyn_cast<CXXTemporaryObjectExpr>(arg0))
+        {
+            arg0 = tempObjExpr->getArg(0);
+            continue;
+        }
+        if (auto parenExpr = dyn_cast<ParenExpr>(arg0))
+        {
+            arg0 = parenExpr->getSubExpr();
+            continue;
+        }
+        argType = arg0->getType().getTypePtr();
+        break;
+    }
+
+    const RecordType* argTemplateType = extractTemplateType(argType);
+    if (!argTemplateType)
+        return true;
+
+    CXXRecordDecl* templateParamRD = dyn_cast<CXXRecordDecl>(templateParamType->getDecl());
+    CXXRecordDecl* methodArgRD = dyn_cast<CXXRecordDecl>(argTemplateType->getDecl());
+
+    // querying for XInterface (instead of doing an upcast) has special semantics,
+    // to check for UNO object equivalence.
+    if (templateParamRD->getName() == "XInterface")
+        return true;
+
+    // XShape is used in UNO aggregates in very "entertaining" ways, which means an UNO_QUERY
+    // can return a completely different object, e.g. see SwXShape::queryInterface
+    if (templateParamRD->getName() == "XShape")
+        return true;
+
+    if (auto declRefExpr = dyn_cast<DeclRefExpr>(mce->getArg(1)))
+    {
+        // no warning expected, used to reject null references
+        if (auto enumConstantDecl = dyn_cast<EnumConstantDecl>(declRefExpr->getDecl()))
+        {
+            if (enumConstantDecl->getName() == "UNO_SET_THROW")
+                return true;
+            if (enumConstantDecl->getName() == "UNO_QUERY_THROW")
+                return true;
+            if (enumConstantDecl->getName() == "SAL_NO_ACQUIRE")
+                return true;
+        }
+    }
+
+    if (methodArgRD->Equals(templateParamRD) || isDerivedFrom(methodArgRD, templateParamRD))
+    {
+        report(DiagnosticsEngine::Warning,
+               "the source reference is already a subtype of the destination reference, just use =",
+               compat::getBeginLoc(mce))
+            << mce->getSourceRange();
+    }
+    return true;
+}
+
+/**
+    Check for
+        Reference<T>(x.get(), UNO_QUERY)
+    because sometimes simplifying that means the main purpose of this plugin can kick in.
+ */
+bool ReferenceCasting::CheckForUnnecessaryGet(const Expr* expr)
+{
+    expr = expr->IgnoreImplicit();
+    auto cxxMemberCallExpr = dyn_cast<CXXMemberCallExpr>(expr);
+    if (!cxxMemberCallExpr)
+        return false;
+    auto methodDecl = cxxMemberCallExpr->getMethodDecl();
+    if (!methodDecl)
+        return false;
+    if (!methodDecl->getIdentifier() || methodDecl->getName() != "get")
+        return false;
+
+    if (!loplugin::TypeCheck(expr->getType()).Pointer())
+        return false;
+    if (!loplugin::DeclCheck(methodDecl->getParent()).Class("Reference").Namespace("uno"))
+        return false;
+
+    StringRef aFileName = getFileNameOfSpellingLoc(
+        compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(expr)));
+    if (loplugin::isSamePathname(aFileName, SRCDIR "/cppu/qa/test_reference.cxx"))
+        return false;
+
+    return true;
+}
+
+static const RecordType* extractTemplateType(const clang::Type* cceType)
+{
+    // check for passing raw pointer to interface case
+    if (cceType->isPointerType())
+    {
+        auto pointeeType = cceType->getPointeeType();
+        if (auto elaboratedType = dyn_cast<ElaboratedType>(pointeeType))
+            pointeeType = elaboratedType->desugar();
+        if (auto recordType = dyn_cast<RecordType>(pointeeType))
+            return recordType;
+    }
+
+    if (auto elaboratedType = dyn_cast<ElaboratedType>(cceType))
+        cceType = elaboratedType->desugar().getTypePtr();
+    auto cceTST = dyn_cast<TemplateSpecializationType>(cceType);
+    if (!cceTST)
+        return NULL;
+    if (cceTST->getNumArgs() != 1)
+        return NULL;
+    const TemplateArgument& cceTA = cceTST->getArg(0);
+    const clang::Type* templateParamType = cceTA.getAsType().getTypePtr();
+    if (auto elaboratedType = dyn_cast<ElaboratedType>(templateParamType))
+        templateParamType = elaboratedType->desugar().getTypePtr();
+    return dyn_cast<RecordType>(templateParamType);
+}
+
+static int derivedFromCount(QualType qt, const CXXRecordDecl* baseRecord);
+
+static int derivedFromCount(const CXXRecordDecl* subtypeRecord, const CXXRecordDecl* baseRecord)
+{
+    if (!subtypeRecord->hasDefinition())
+        return 0;
+    int derivedCount = 0;
+    for (auto it = subtypeRecord->bases_begin(); it != subtypeRecord->bases_end(); ++it)
+    {
+        derivedCount += derivedFromCount(it->getType(), baseRecord);
+        if (derivedCount > 1)
+            return derivedCount;
+    }
+    for (auto it = subtypeRecord->vbases_begin(); it != subtypeRecord->vbases_end(); ++it)
+    {
+        derivedCount += derivedFromCount(it->getType(), baseRecord);
+        if (derivedCount > 1)
+            return derivedCount;
+    }
+    return derivedCount;
+}
+
+static int derivedFromCount(QualType qt, const CXXRecordDecl* baseRecord)
+{
+    int derivedCount = 0;
+    auto type = qt.getTypePtr();
+    if (auto elaboratedType = dyn_cast<ElaboratedType>(type))
+        type = elaboratedType->desugar().getTypePtr();
+    if (auto recordType = dyn_cast<RecordType>(type))
+    {
+        if (auto cxxRecordDecl = dyn_cast<CXXRecordDecl>(recordType->getDecl()))
+        {
+            if (cxxRecordDecl->Equals(baseRecord))
+                derivedCount++;
+            derivedCount += derivedFromCount(cxxRecordDecl, baseRecord);
+        }
+    }
+    return derivedCount;
+}
+
+/**
+  Implement my own isDerived because we can't always see all the definitions of the classes involved.
+  which will cause an assert with the normal clang isDerivedFrom code.
+*/
+static bool isDerivedFrom(const CXXRecordDecl* subtypeRecord, const CXXRecordDecl* baseRecord)
+{
+    // if there is more than one case, then we have an ambigous conversion, and we can't change the code
+    // to use the upcasting constructor.
+    return derivedFromCount(subtypeRecord, baseRecord) == 1;
+}
+
+loplugin::Plugin::Registration<ReferenceCasting> X2("referencecasting");
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */