diff options
author | Noel Grandin <noel.grandin@collabora.co.uk> | 2023-01-20 12:55:25 +0200 |
---|---|---|
committer | Noel Grandin <noel.grandin@collabora.co.uk> | 2023-01-20 12:58:40 +0000 |
commit | 32c8b03f6172acf3a19c5d1938b9935369f536f1 (patch) | |
tree | 87c994cbbb77a1817ad7675ee712f75a6f4320b7 /compilerplugins/clang/refcounting.cxx | |
parent | a5a1ea2f7d784c5c6c33f332ba61aceb7af3eca4 (diff) |
improve loplugin:refcounting
to catch places where we are converting a weak reference to a strong
reference, and then using a pointer to store the result
Change-Id: I69b132907b574e5c6974fadf18bd9658107d3a0d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145877
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Diffstat (limited to 'compilerplugins/clang/refcounting.cxx')
-rw-r--r-- | compilerplugins/clang/refcounting.cxx | 68 |
1 files changed, 68 insertions, 0 deletions
diff --git a/compilerplugins/clang/refcounting.cxx b/compilerplugins/clang/refcounting.cxx index e65772f71e7d..ca6c0d97d9f0 100644 --- a/compilerplugins/clang/refcounting.cxx +++ b/compilerplugins/clang/refcounting.cxx @@ -75,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); @@ -714,6 +715,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; } @@ -762,6 +774,47 @@ bool RefCounting::isCastingReference(const Expr* expr) 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)) @@ -801,6 +854,21 @@ bool RefCounting::VisitBinaryOperator(const BinaryOperator * binaryOperator) << 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(); + } return true; } |