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>

4 files changed, 535 insertions, 228 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: */
diff --git a/compilerplugins/clang/store/referencecasting.cxx b/compilerplugins/clang/store/referencecasting.cxx
deleted file mode 100644
index ed74b0b6e14a..000000000000
--- a/compilerplugins/clang/store/referencecasting.cxx
+++ /dev/null
@@ -1,195 +0,0 @@
-/* -*- 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 "referencecasting.hxx"
-
-#include <clang/AST/Attr.h>
-#include <iostream>
-
-namespace loplugin
-{
-
-/*
-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>
-
-Note that it generates the occasional false positive.
-
-Also, it makes clang3.2 crash on about 4 files in the LO codebase.
-I have logged a bug here:
-  http://llvm.org/bugs/show_bug.cgi?id=15902
-
-*/
-ReferenceCasting::ReferenceCasting( CompilerInstance& compiler )
-    : FilteringPlugin( compiler )
-{
-}
-
-void ReferenceCasting::run()
-{
-    TraverseDecl( compiler.getASTContext().getTranslationUnitDecl());
-}
-
-// This:
-// static void example_method()
-// {
-//     css::uno::Reference<B> b;
-//     css::uno::Reference<A>(b, css::uno::UNO_QUERY);
-// }
-// Compiles to this AST:
-// (CompoundStmt 0x205d430 </noel-extra1/libo-clang/compilerplugins/clang/noel1.cxx:17:1, line:20:1>
-//   (DeclStmt 0x20580a8 <line:18:5, col:32>
-//     (0x20530e0 "css::uno::Reference<B> refB =
-//       (CXXConstructExpr 0x2058078 <col:28> 'css::uno::Reference<B>':'class com::sun::star::uno::Reference<class B>''void(void)')"))
-//   (DeclStmt 0x205d418 <line:19:5, col:59>
-//     (0x2058310 "css::uno::Reference<A> refA =
-//       (CXXConstructExpr 0x205d3d8 <col:28, col:58> 'css::uno::Reference<A>':'class com::sun::star::uno::Reference<class A>''void (const class com::sun::star::uno::BaseReference &, enum com::sun::star::uno::UnoReference_Query)'
-//         (ImplicitCastExpr 0x205d3c0 <col:33> 'const class com::sun::star::uno::BaseReference' lvalue <NoOp>
-//           (ImplicitCastExpr 0x205d3a0 <col:33> 'class com::sun::star::uno::BaseReference' lvalue <DerivedToBase (BaseReference)>
-//             (DeclRefExpr 0x20582a0 <col:33> 'css::uno::Reference<B>':'class com::sun::star::uno::Reference<class B>' lvalue Var 0x20530e0 'refB' 'css::uno::Reference<B>':'class com::sun::star::uno::Reference<class B>')))
-//         (DeclRefExpr 0x2058398 <col:39, col:49> 'enum com::sun::star::uno::UnoReference_Query' EnumConstant 0x1831de0 'UNO_QUERY' 'enum com::sun::star::uno::UnoReference_Query'))")))
-//
-//
-// This:
-// static void example_method1(css::uno::Reference<A>)
-// {
-// }
-// static void example_method2()
-// {
-//     css::uno::Reference<B> refB;
-//     example_method1(css::uno::Reference<A>(refB, css::uno::UNO_QUERY));
-// }
-//  Compiles to this AST:
-// static void example_method1(css::uno::Reference<A>) (CompoundStmt 0x2a74ee8 </noel-extra1/libo-clang/compilerplugins/clang/noel1.cxx:17:1, line:18:1>)
-// static void example_method2() (CompoundStmt 0x2a7a650 </noel-extra1/libo-clang/compilerplugins/clang/noel1.cxx:21:1, line:24:1>
-//  (DeclStmt 0x2a7a1a8 <line:22:5, col:32>
-//    (0x2a751e0 "css::uno::Reference<B> refB =
-//      (CXXConstructExpr 0x2a7a178 <col:28> 'css::uno::Reference<B>':'class com::sun::star::uno::Reference<class B>''void(void)')"))
-//  (ExprWithCleanups 0x2a7a638 <line:23:5, col:70> 'void'
-//    (CallExpr 0x2a7a570 <col:5, col:70> 'void'
-//      (ImplicitCastExpr 0x2a7a558 <col:5> 'void (*)(css::uno::Reference<A>)' <FunctionToPointerDecay>
-//        (DeclRefExpr 0x2a7a4d8 <col:5> 'void (css::uno::Reference<A>)' lvalue Function 0x2a6ff00 'example_method1' 'void (css::uno::Reference<A>)'))
-//      (CXXBindTemporaryExpr 0x2a7a618 <col:21, col:69> 'css::uno::Reference<A>':'class com::sun::star::uno::Reference<class A>' (CXXTemporary 0x2a7a610)
-//        (CXXConstructExpr 0x2a7a5d8 <col:21, col:69> 'css::uno::Reference<A>':'class com::sun::star::uno::Reference<class A>''void (const Reference<class A> &)' elidable
-//          (MaterializeTemporaryExpr 0x2a7a5c0 <col:21, col:69> 'const Reference<class A>':'const class com::sun::star::uno::Reference<class A>' lvalue
-//            (ImplicitCastExpr 0x2a7a5a8 <col:21, col:69> 'const Reference<class A>':'const class com::sun::star::uno::Reference<class A>' <NoOp>
-//              (CXXBindTemporaryExpr 0x2a7a4b8 <col:21, col:69> 'css::uno::Reference<A>':'class com::sun::star::uno::Reference<class A>' (CXXTemporary 0x2a7a4b0)
-//                (CXXTemporaryObjectExpr 0x2a7a460 <col:21, col:69> 'css::uno::Reference<A>':'class com::sun::star::uno::Reference<class A>''void (const class com::sun::star::uno::BaseReference &, enum com::sun::star::uno::UnoReference_Query)'
-//                  (ImplicitCastExpr 0x2a7a448 <col:44> 'const class com::sun::star::uno::BaseReference' lvalue <NoOp>
-//                    (ImplicitCastExpr 0x2a7a428 <col:44> 'class com::sun::star::uno::BaseReference' lvalue <DerivedToBase (BaseReference)>
-//                      (DeclRefExpr 0x2a7a340 <col:44> 'css::uno::Reference<B>':'class com::sun::star::uno::Reference<class B>' lvalue Var 0x2a751e0 'refB' 'css::uno::Reference<B>':'class com::sun::star::uno::Reference<class B>')))
-//                  (DeclRefExpr 0x2a7a398 <col:50, col:60> 'enum com::sun::star::uno::UnoReference_Query' EnumConstant 0x224ee20 'UNO_QUERY' 'enum com::sun::star::uno::UnoReference_Query'))))))))))
-
-static const Type* extractTemplateType(Expr* cce);
-
-bool ReferenceCasting::VisitCXXConstructExpr( CXXConstructExpr* cce )
-{
-    // don't bother processing anything in the Reference.h file. Makes my life easier when debugging this.
-    if( StringRef(compiler.getSourceManager().getPresumedLoc( cce->getSourceRange().getBegin() )).find( "Reference.h" ) != StringRef::npos )
-        return true;
-
-    // look for calls to the Reference<T>(x,UNO_something) constructor
-    if( cce->getConstructor()->getNameInfo().getName().getAsString() != "Reference" )
-        return true;
-
-    if( cce->getNumArgs() != 2 )
-        return true;
-
-    // extract the type parameter passed to the template
-    const Type * templateParamType = extractTemplateType(cce);
-    if ( !templateParamType )
-        return true;
-
-    // extract the type of the first parameter passed to the constructor
-    Expr* constructorArg0 = cce->getArg(0);
-    if( !constructorArg0 )
-       return true;
-
-    // ignore the Reference(XInterface*,...) constructor
-    if( constructorArg0->getType()->isPointerType() )
-       return true;
-
-    // drill down the expression tree till we hit the bottom
-    DeclRefExpr* constructorSubArg2;
-    Expr* constructorArg0SubExpr = constructorArg0;
-    for(;;)
-    {
-        // if we've hit the member expression we want, break
-        constructorSubArg2 = dyn_cast<DeclRefExpr>( constructorArg0SubExpr );
-        if( constructorSubArg2 )
-            break;
-        CastExpr* tmp1 = dyn_cast<CastExpr>( constructorArg0SubExpr );
-        if( tmp1 ) {
-            constructorArg0SubExpr = tmp1->getSubExpr();
-            continue;
-        }
-        MaterializeTemporaryExpr* tmp2 = dyn_cast<MaterializeTemporaryExpr>( constructorArg0SubExpr );
-        if( tmp2 ) {
-            constructorArg0SubExpr = tmp2->GetTemporaryExpr();
-            continue;
-        }
-        CXXBindTemporaryExpr* tmp3 = dyn_cast<CXXBindTemporaryExpr>( constructorArg0SubExpr );
-        if( tmp3 ) {
-            constructorArg0SubExpr = tmp3->getSubExpr();
-            continue;
-        }
-        CXXTemporaryObjectExpr* tmp4 = dyn_cast<CXXTemporaryObjectExpr>( constructorArg0SubExpr );
-        if( tmp4 ) {
-            constructorArg0SubExpr = tmp4->getArg(0);
-            continue;
-        }
-        return true;
-    }
-
-    const Type * tmp3 = extractTemplateType( constructorSubArg2 );
-    if ( !tmp3 )
-        return true;
-
-    const RecordType* templateParamRT = dyn_cast<RecordType>( templateParamType );
-    const RecordType* constructorArgRT = dyn_cast<RecordType>( tmp3 );
-    if( !templateParamRT || !constructorArgRT )
-       return true;
-
-    CXXRecordDecl* templateParamRD = dyn_cast<CXXRecordDecl>( templateParamRT->getDecl() );
-    CXXRecordDecl* constructorArgRD = dyn_cast<CXXRecordDecl>( constructorArgRT->getDecl() );
-
-    if (constructorArgRD->Equals(templateParamRD) || constructorArgRD->isDerivedFrom(templateParamRD))
-        report( DiagnosticsEngine::Warning,
-                "the source reference is already a subtype of the destination reference",
-                cce->getLocStart()) // and the exact position where the message should point
-                    << cce->getSourceRange(); // and the full return statement to highlight (optional)
-
-    return true;
-}
-
-static const Type* extractTemplateType(Expr* cce)
-{
-    QualType cceQT = cce->getType();
-    const Type* cceType = cceQT.getTypePtr();
-    const TemplateSpecializationType* cceTST = dyn_cast<TemplateSpecializationType>( cceType );
-    if( !cceTST )
-        return NULL;
-    if( cceTST->getNumArgs() != 1 )
-        return NULL;
-    const TemplateArgument & cceTA = cceTST->getArg(0);
-    QualType templateParamQT = cceTA.getAsType();
-    return templateParamQT.getTypePtr();
-}
-
-static Plugin::Registration< ReferenceCasting > X( "referencecasting" );
-
-} // namespace
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/store/referencecasting.hxx b/compilerplugins/clang/store/referencecasting.hxx
deleted file mode 100644
index 3c41b773e0f0..000000000000
--- a/compilerplugins/clang/store/referencecasting.hxx
+++ /dev/null
@@ -1,33 +0,0 @@
-/* -*- 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.
- *
- */
-
-#ifndef REFERENCECASTING_H
-#define REFERENCECASTING_H
-
-#include "plugin.hxx"
-
-namespace loplugin
-{
-
-class ReferenceCasting
-    : public FilteringPlugin< ReferenceCasting >
-    {
-    public:
-        explicit ReferenceCasting( CompilerInstance& compiler );
-        virtual void run() override;
-        bool VisitCXXConstructExpr( CXXConstructExpr* cce );
-    };
-
-} // namespace
-
-#endif // REFERENCECASTING_H
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/referencecasting.cxx b/compilerplugins/clang/test/referencecasting.cxx
new file mode 100644
index 000000000000..87324bb86fd6
--- /dev/null
+++ b/compilerplugins/clang/test/referencecasting.cxx
@@ -0,0 +1,150 @@
+/* -*- 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 "com/sun/star/uno/XInterface.hpp"
+#include "com/sun/star/io/XStreamListener.hpp"
+#include "com/sun/star/lang/XTypeProvider.hpp"
+#include "com/sun/star/lang/XComponent.hpp"
+#include "cppuhelper/weak.hxx"
+
+void test1(const css::uno::Reference<css::io::XStreamListener>& a)
+{
+    // expected-error@+1 {{the source reference is already a subtype of the destination reference, just use = [loplugin:referencecasting]}}
+    css::uno::Reference<css::lang::XEventListener> b(a, css::uno::UNO_QUERY);
+}
+
+namespace test2
+{
+css::uno::Reference<css::io::XStreamListener> getListener();
+
+void test()
+{
+    // expected-error@+1 {{the source reference is already a subtype of the destination reference, just use = [loplugin:referencecasting]}}
+    css::uno::Reference<css::lang::XEventListener> b(getListener(), css::uno::UNO_QUERY);
+}
+}
+
+namespace test3
+{
+void callListener(css::uno::Reference<css::uno::XInterface> const&);
+
+void test(css::uno::Reference<css::io::XStreamListener> const& l)
+{
+    // expected-error@+1 {{the source reference is already a subtype of the destination reference, just use = [loplugin:referencecasting]}}
+    callListener(css::uno::Reference<css::lang::XEventListener>(l, css::uno::UNO_QUERY));
+}
+}
+
+void test4(const css::uno::Reference<css::io::XStreamListener>& a)
+{
+    // no warning expected, used to reject null references
+    css::uno::Reference<css::lang::XEventListener> b(a, css::uno::UNO_SET_THROW);
+}
+
+// no warning expected
+namespace test5
+{
+void test(css::uno::Reference<css::io::XStreamListener> l)
+{
+    css::uno::Reference<css::uno::XInterface> a = l;
+}
+}
+
+namespace test6
+{
+void test(css::uno::Reference<css::io::XStreamListener> l)
+{
+    css::uno::Reference<css::lang::XEventListener> a;
+    // expected-error@+1 {{the source reference is already a subtype of the destination reference, just use = [loplugin:referencecasting]}}
+    a.set(l, css::uno::UNO_QUERY);
+}
+}
+
+namespace test7
+{
+void test(css::uno::Reference<css::io::XStreamListener> l)
+{
+    // expected-error@+1 {{unnecessary get() call [loplugin:referencecasting]}}
+    css::uno::Reference<css::lang::XEventListener> a(l.get(), css::uno::UNO_QUERY);
+    // expected-error@+1 {{unnecessary get() call [loplugin:referencecasting]}}
+    a.set(l.get(), css::uno::UNO_QUERY);
+}
+}
+
+namespace test8
+{
+void test(css::io::XStreamListener* l)
+{
+    // expected-error@+1 {{the source reference is already a subtype of the destination reference, just use = [loplugin:referencecasting]}}
+    css::uno::Reference<css::lang::XEventListener> a(l, css::uno::UNO_QUERY);
+    // expected-error@+1 {{the source reference is already a subtype of the destination reference, just use = [loplugin:referencecasting]}}
+    a.set(l, css::uno::UNO_QUERY);
+}
+}
+
+// check for looking through casts
+namespace test9
+{
+class StatusbarController : public css::io::XStreamListener, public ::cppu::OWeakObject
+{
+};
+
+void test(StatusbarController* pController)
+{
+    css::uno::Reference<css::io::XStreamListener> xController;
+    // expected-error@+1 {{the source reference is already a subtype of the destination reference, just use = [loplugin:referencecasting]}}
+    xController.set(static_cast<::cppu::OWeakObject*>(pController), css::uno::UNO_QUERY);
+}
+}
+
+// no warning expected when we have an ambiguous base
+namespace test10
+{
+class Foo : public css::lang::XTypeProvider, public css::lang::XComponent
+{
+    virtual ~Foo();
+    void bar()
+    {
+        css::uno::Reference<css::lang::XEventListener> xSource(
+            static_cast<css::lang::XTypeProvider*>(this), css::uno::UNO_QUERY);
+    }
+};
+}
+
+// no warning expected for SAL_NO_ACQUIRE
+namespace test11
+{
+void test(css::io::XStreamListener* l)
+{
+    css::uno::Reference<css::lang::XEventListener> a(l, SAL_NO_ACQUIRE);
+    a.set(l, SAL_NO_ACQUIRE);
+}
+}
+
+// no warning expected: querying for XInterface (instead of doing an upcast) has special semantics,
+// to check for UNO object equivalence.
+void test12(const css::uno::Reference<css::io::XStreamListener>& a)
+{
+    css::uno::Reference<css::uno::XInterface> b(a, css::uno::UNO_QUERY);
+}
+
+// no warning expected: querying for XInterface (instead of doing an upcast) has special semantics,
+// to check for UNO object equivalence.
+struct Test13
+{
+    css::uno::Reference<css::uno::XInterface> m_xNormalizedIFace;
+    void newObject(const css::uno::Reference<css::uno::XInterface>& _rxIFace)
+    {
+        m_xNormalizedIFace.set(_rxIFace, css::uno::UNO_QUERY);
+    }
+};
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */