diff options
Diffstat (limited to 'compilerplugins/clang/refcounting.cxx')
-rw-r--r-- | compilerplugins/clang/refcounting.cxx | 158 |
1 files changed, 123 insertions, 35 deletions
diff --git a/compilerplugins/clang/refcounting.cxx b/compilerplugins/clang/refcounting.cxx index 9157a1910add..801173ce6488 100644 --- a/compilerplugins/clang/refcounting.cxx +++ b/compilerplugins/clang/refcounting.cxx @@ -12,6 +12,7 @@ #include "check.hxx" #include "plugin.hxx" +#include "config_clang.h" #include "clang/AST/CXXInheritance.h" /** @@ -74,6 +75,7 @@ private: bool visitTemporaryObjectExpr(Expr const * expr); bool isCastingReference(const Expr* expr); + bool isCallingGetOnWeakRef(const Expr* expr); }; bool containsXInterfaceSubclass(const clang::Type* pType0); @@ -376,24 +378,30 @@ static bool containsStaticTypeMethod(const CXXRecordDecl* x) void RefCounting::checkUnoReference(QualType qt, const Decl* decl, const RecordDecl* parent, const std::string& rDeclName) { - if (loplugin::TypeCheck(qt).Class("Reference").Namespace("uno").Namespace("star").Namespace("sun").Namespace("com").GlobalNamespace()) { - const CXXRecordDecl* pRecordDecl = qt->getAsCXXRecordDecl(); - const ClassTemplateSpecializationDecl* pTemplate = dyn_cast<ClassTemplateSpecializationDecl>(pRecordDecl); - const TemplateArgument& rArg = pTemplate->getTemplateArgs()[0]; - const CXXRecordDecl* templateParam = rArg.getAsType()->getAsCXXRecordDecl()->getDefinition(); - if (templateParam && !containsStaticTypeMethod(templateParam)) { - report( - DiagnosticsEngine::Warning, - ("uno::Reference %0 with template parameter that does not" - " contain ::static_type() %1%select{|, parent is %3,}2 should" - " probably be using rtl::Reference instead"), - decl->getLocation()) - << rDeclName << qt << (parent != nullptr) - << (parent != nullptr - ? parent->getQualifiedNameAsString() : std::string()) - << decl->getSourceRange(); - } - } + if (!loplugin::TypeCheck(qt).Class("Reference").Namespace("uno").Namespace("star").Namespace("sun").Namespace("com").GlobalNamespace()) + return; + const CXXRecordDecl* pRecordDecl = qt->getAsCXXRecordDecl(); + const ClassTemplateSpecializationDecl* pTemplate = dyn_cast<ClassTemplateSpecializationDecl>(pRecordDecl); + const TemplateArgument& rArg = pTemplate->getTemplateArgs()[0]; + const CXXRecordDecl* templateParam = rArg.getAsType()->getAsCXXRecordDecl()->getDefinition(); + if (!templateParam) + return; + // SwXText is a special case. It is a mixin class that does not inherit from OWeakObject, so + // we cannot use rtl::Reference. + if (loplugin::DeclCheck(templateParam).Class("SwXText")) + return; + if (containsStaticTypeMethod(templateParam)) + return; + report( + DiagnosticsEngine::Warning, + ("uno::Reference %0 with template parameter that does not" + " contain ::static_type() %1%select{|, parent is %3,}2 should" + " probably be using rtl::Reference instead"), + decl->getLocation()) + << rDeclName << qt << (parent != nullptr) + << (parent != nullptr + ? parent->getQualifiedNameAsString() : std::string()) + << decl->getSourceRange(); } bool RefCounting::visitTemporaryObjectExpr(Expr const * expr) { @@ -406,7 +414,7 @@ bool RefCounting::visitTemporaryObjectExpr(Expr const * expr) { DiagnosticsEngine::Warning, ("Temporary object of SvRefBase subclass %0 being directly stack" " managed, should be managed via tools::SvRef"), - compat::getBeginLoc(expr)) + expr->getBeginLoc()) << t.getUnqualifiedType() << expr->getSourceRange(); } else if (containsSalhelperReferenceObjectSubclass(t.getTypePtr())) { report( @@ -414,7 +422,7 @@ bool RefCounting::visitTemporaryObjectExpr(Expr const * expr) { ("Temporary object of salhelper::SimpleReferenceObject subclass %0" " being directly stack managed, should be managed via" " rtl::Reference"), - compat::getBeginLoc(expr)) + expr->getBeginLoc()) << t.getUnqualifiedType() << expr->getSourceRange(); } else if (containsXInterfaceSubclass(t)) { report( @@ -422,7 +430,7 @@ bool RefCounting::visitTemporaryObjectExpr(Expr const * expr) { ("Temporary object of css::uno::XInterface subclass %0 being" " directly stack managed, should be managed via" " css::uno::Reference"), - compat::getBeginLoc(expr)) + expr->getBeginLoc()) << t.getUnqualifiedType() << expr->getSourceRange(); } else if (containsOWeakObjectSubclass(t)) { report( @@ -430,7 +438,7 @@ bool RefCounting::visitTemporaryObjectExpr(Expr const * expr) { ("Temporary object of cppu::OWeakObject subclass %0 being" " directly stack managed, should be managed via" " css::uno::Reference"), - compat::getBeginLoc(expr)) + expr->getBeginLoc()) << t.getUnqualifiedType() << expr->getSourceRange(); } return true; @@ -501,9 +509,13 @@ bool RefCounting::VisitCXXDeleteExpr(const CXXDeleteExpr * cxxDeleteExpr) if (ignoreLocation(cxxDeleteExpr)) return true; StringRef aFileName = getFilenameOfLocation( - compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(cxxDeleteExpr))); + compiler.getSourceManager().getSpellingLoc(cxxDeleteExpr->getBeginLoc())); if (loplugin::isSamePathname(aFileName, SRCDIR "/cppuhelper/source/weak.cxx")) return true; + if (loplugin::isSamePathname(aFileName, SRCDIR "/include/svx/svdobj.hxx")) + return true; + if (loplugin::isSamePathname(aFileName, SRCDIR "/svx/source/svdraw/svdobj.cxx")) + return true; if (!cxxDeleteExpr->getArgument()) return true; @@ -516,7 +528,7 @@ bool RefCounting::VisitCXXDeleteExpr(const CXXDeleteExpr * cxxDeleteExpr) report( DiagnosticsEngine::Warning, "cppu::OWeakObject subclass %0 being deleted via delete, should be managed via rtl::Reference", - compat::getBeginLoc(cxxDeleteExpr)) + cxxDeleteExpr->getBeginLoc()) << pointeeType << cxxDeleteExpr->getSourceRange(); } @@ -609,7 +621,7 @@ bool RefCounting::VisitReturnStmt(const ReturnStmt * returnStmt) { if (!returnStmt->getRetValue()) return true; - auto cxxNewExpr = dyn_cast<CXXNewExpr>(compat::IgnoreImplicit(returnStmt->getRetValue())); + auto cxxNewExpr = dyn_cast<CXXNewExpr>(returnStmt->getRetValue()->IgnoreImplicit()); if (!cxxNewExpr) return true; @@ -622,7 +634,7 @@ bool RefCounting::VisitReturnStmt(const ReturnStmt * returnStmt) { report( DiagnosticsEngine::Warning, "new object of cppu::OWeakObject subclass %0 being returned via raw pointer, should be returned by via rtl::Reference", - compat::getBeginLoc(returnStmt)) + returnStmt->getBeginLoc()) << qt << returnStmt->getSourceRange(); } @@ -679,10 +691,10 @@ bool RefCounting::VisitVarDecl(const VarDecl * varDecl) { if (varDecl->getType()->isPointerType() && varDecl->getInit()) { - auto newExpr = dyn_cast<CXXNewExpr>(compat::IgnoreImplicit(varDecl->getInit())); + auto newExpr = dyn_cast<CXXNewExpr>(varDecl->getInit()->IgnoreImplicit()); if (newExpr) { - StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(varDecl))); + StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(varDecl->getBeginLoc())); if (loplugin::isSamePathname(fileName, SRCDIR "/cppuhelper/source/component_context.cxx")) return true; auto pointeeType = varDecl->getType()->getPointeeType(); @@ -697,7 +709,7 @@ bool RefCounting::VisitVarDecl(const VarDecl * varDecl) { if (isCastingReference(varDecl->getInit())) { // TODO false+ code - StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(varDecl))); + StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(varDecl->getBeginLoc())); if (loplugin::isSamePathname(fileName, SRCDIR "/sw/source/core/unocore/unotbl.cxx")) return true; auto pointeeType = varDecl->getType()->getPointeeType(); @@ -709,6 +721,17 @@ bool RefCounting::VisitVarDecl(const VarDecl * varDecl) { << pointeeType << varDecl->getSourceRange(); } + if (isCallingGetOnWeakRef(varDecl->getInit())) + { + auto pointeeType = varDecl->getType()->getPointeeType(); + if (containsOWeakObjectSubclass(pointeeType)) + report( + DiagnosticsEngine::Warning, + "weak object being converted to strong, and then the reference dropped, and managed via raw pointer, should be managed via rtl::Reference", + varDecl->getLocation()) + << pointeeType + << varDecl->getSourceRange(); + } } return true; } @@ -720,7 +743,7 @@ bool RefCounting::VisitVarDecl(const VarDecl * varDecl) { */ bool RefCounting::isCastingReference(const Expr* expr) { - expr = compat::IgnoreImplicit(expr); + expr = expr->IgnoreImplicit(); auto castExpr = dyn_cast<CastExpr>(expr); if (!castExpr) return false; @@ -733,7 +756,7 @@ bool RefCounting::isCastingReference(const Expr* expr) if (!loplugin::TypeCheck(objectType).Class("Reference")) return false; // ignore "x.get()" where x is a var - auto obj = compat::IgnoreImplicit(memberCallExpr->getImplicitObjectArgument()); + auto obj = memberCallExpr->getImplicitObjectArgument()->IgnoreImplicit(); if (isa<DeclRefExpr>(obj) || isa<MemberExpr>(obj)) return false; // if the foo in foo().get() returns "rtl::Reference<T>&" then the variable @@ -745,9 +768,59 @@ bool RefCounting::isCastingReference(const Expr* expr) if (callMethod->getReturnType()->isReferenceType()) return false; } + // Ignore + // WeakReference x; + // if (x.get.get()) + // and similar stuff + if (auto memberCall2 = dyn_cast<CXXMemberCallExpr>(obj)) + { + if (loplugin::TypeCheck(memberCall2->getImplicitObjectArgument()->getType()).Class("WeakReference")) + return false; + } return true; } +/** + Look for code like + makeFoo().get(); + or + cast<T*>(makeFoo().get().get()); + or + foo.get(); + where makeFoo() returns a unotools::WeakReference<Foo> + and foo is a unotools::WeakReference<Foo> var. +*/ +bool RefCounting::isCallingGetOnWeakRef(const Expr* expr) +{ + expr = expr->IgnoreImplicit(); + // unwrap the cast (if any) + if (auto castExpr = dyn_cast<CastExpr>(expr)) + expr = castExpr->getSubExpr()->IgnoreImplicit(); + // unwrap outer get (if any) + if (auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(expr)) + { + auto methodDecl = memberCallExpr->getMethodDecl(); + if (methodDecl && methodDecl->getIdentifier() && methodDecl->getName() == "get") + { + QualType objectType = memberCallExpr->getImplicitObjectArgument()->getType(); + if (loplugin::TypeCheck(objectType).Class("Reference").Namespace("rtl")) + expr = memberCallExpr->getImplicitObjectArgument()->IgnoreImplicit(); + } + } + // check for converting a WeakReference to a strong reference via get() + if (auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(expr)) + { + auto methodDecl = memberCallExpr->getMethodDecl(); + if (methodDecl && methodDecl->getIdentifier() && methodDecl->getName() == "get") + { + QualType objectType = memberCallExpr->getImplicitObjectArgument()->getType(); + if (loplugin::TypeCheck(objectType).Class("WeakReference").Namespace("unotools")) + return true; + } + } + return false; +} + bool RefCounting::VisitBinaryOperator(const BinaryOperator * binaryOperator) { if (ignoreLocation(binaryOperator)) @@ -757,11 +830,11 @@ bool RefCounting::VisitBinaryOperator(const BinaryOperator * binaryOperator) if (!binaryOperator->getLHS()->getType()->isPointerType()) return true; - auto newExpr = dyn_cast<CXXNewExpr>(compat::IgnoreImplicit(binaryOperator->getRHS())); + auto newExpr = dyn_cast<CXXNewExpr>(binaryOperator->getRHS()->IgnoreImplicit()); if (newExpr) { // deliberately does not want to keep track at the allocation site - StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(binaryOperator))); + StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(binaryOperator->getBeginLoc())); if (loplugin::isSamePathname(fileName, SRCDIR "/vcl/unx/generic/dtrans/X11_selection.cxx")) return true; @@ -771,7 +844,7 @@ bool RefCounting::VisitBinaryOperator(const BinaryOperator * binaryOperator) report( DiagnosticsEngine::Warning, "cppu::OWeakObject subclass %0 being managed via raw pointer, should be managed via rtl::Reference", - compat::getBeginLoc(binaryOperator)) + binaryOperator->getBeginLoc()) << pointeeType << binaryOperator->getSourceRange(); } @@ -783,7 +856,22 @@ bool RefCounting::VisitBinaryOperator(const BinaryOperator * binaryOperator) report( DiagnosticsEngine::Warning, "cppu::OWeakObject subclass %0 being managed via raw pointer, should be managed via rtl::Reference", - compat::getBeginLoc(binaryOperator)) + binaryOperator->getBeginLoc()) + << pointeeType + << binaryOperator->getSourceRange(); + } + if (isCallingGetOnWeakRef(binaryOperator->getRHS())) + { + // TODO Very dodgy code, but I see no simple way of fixing it + StringRef fileName = getFilenameOfLocation(compiler.getSourceManager().getSpellingLoc(binaryOperator->getBeginLoc())); + if (loplugin::isSamePathname(fileName, SRCDIR "/sd/source/ui/view/Outliner.cxx")) + return true; + auto pointeeType = binaryOperator->getLHS()->getType()->getPointeeType(); + if (containsOWeakObjectSubclass(pointeeType)) + report( + DiagnosticsEngine::Warning, + "weak object being converted to strong, and then the reference dropped, and managed via raw pointer, should be managed via rtl::Reference", + binaryOperator->getBeginLoc()) << pointeeType << binaryOperator->getSourceRange(); } |