summaryrefslogtreecommitdiffstats
path: root/compilerplugins/clang/refcounting.cxx
diff options
context:
space:
mode:
Diffstat (limited to 'compilerplugins/clang/refcounting.cxx')
-rw-r--r--compilerplugins/clang/refcounting.cxx158
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();
}