summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--compilerplugins/clang/salcall.cxx135
-rw-r--r--compilerplugins/clang/test/salcall.cxx25
2 files changed, 59 insertions, 101 deletions
diff --git a/compilerplugins/clang/salcall.cxx b/compilerplugins/clang/salcall.cxx
index e70e6477f2b0..a3f5678a3918 100644
--- a/compilerplugins/clang/salcall.cxx
+++ b/compilerplugins/clang/salcall.cxx
@@ -10,6 +10,7 @@
#include "plugin.hxx"
#include "check.hxx"
#include "compat.hxx"
+#include "functionaddress.hxx"
#include <algorithm>
#include <cassert>
@@ -51,113 +52,41 @@ CXXMethodDecl const* getTemplateInstantiationPattern(CXXMethodDecl const* decl)
return p == nullptr ? decl : cast<CXXMethodDecl>(p);
}
-class SalCall final : public loplugin::FilteringRewritePlugin<SalCall>
+class SalCall final : public loplugin::FunctionAddress<loplugin::FilteringRewritePlugin<SalCall>>
{
public:
explicit SalCall(loplugin::InstantiationData const& data)
- : FilteringRewritePlugin(data)
+ : FunctionAddress(data)
{
}
virtual void run() override
{
- m_phase = PluginPhase::FindAddressOf;
- TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
- m_phase = PluginPhase::Warning;
- TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+ if (TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()))
+ {
+ auto const& addressOfSet = getFunctionsWithAddressTaken();
+ for (auto const decl : m_decls)
+ {
+ if (addressOfSet.find(decl->getCanonicalDecl()) == addressOfSet.end())
+ {
+ handleFunctionDecl(decl);
+ }
+ }
+ }
}
bool VisitFunctionDecl(FunctionDecl const*);
- bool VisitUnaryOperator(UnaryOperator const*);
- bool VisitInitListExpr(InitListExpr const*);
- bool VisitCallExpr(CallExpr const*);
- bool VisitBinaryOperator(BinaryOperator const*);
- bool VisitCXXConstructExpr(CXXConstructExpr const*);
private:
- void checkForFunctionDecl(Expr const*, bool bCheckOnly = false);
+ void handleFunctionDecl(FunctionDecl const* decl);
bool rewrite(SourceLocation);
bool isSalCallFunction(FunctionDecl const* functionDecl, SourceLocation* pLoc = nullptr);
- std::set<FunctionDecl const*> m_addressOfSet;
- enum class PluginPhase
- {
- FindAddressOf,
- Warning
- };
- PluginPhase m_phase;
+ std::set<FunctionDecl const*> m_decls;
};
-bool SalCall::VisitUnaryOperator(UnaryOperator const* op)
-{
- if (op->getOpcode() != UO_AddrOf)
- {
- return true;
- }
- if (m_phase != PluginPhase::FindAddressOf)
- return true;
- checkForFunctionDecl(op->getSubExpr());
- return true;
-}
-
-bool SalCall::VisitBinaryOperator(BinaryOperator const* binaryOperator)
-{
- if (binaryOperator->getOpcode() != BO_Assign)
- {
- return true;
- }
- if (m_phase != PluginPhase::FindAddressOf)
- return true;
- checkForFunctionDecl(binaryOperator->getRHS());
- return true;
-}
-
-bool SalCall::VisitCallExpr(CallExpr const* callExpr)
-{
- if (m_phase != PluginPhase::FindAddressOf)
- return true;
- for (auto arg : callExpr->arguments())
- checkForFunctionDecl(arg);
- return true;
-}
-
-bool SalCall::VisitCXXConstructExpr(CXXConstructExpr const* constructExpr)
-{
- if (m_phase != PluginPhase::FindAddressOf)
- return true;
- for (auto arg : constructExpr->arguments())
- checkForFunctionDecl(arg);
- return true;
-}
-
-bool SalCall::VisitInitListExpr(InitListExpr const* initListExpr)
-{
- if (m_phase != PluginPhase::FindAddressOf)
- return true;
- for (auto subStmt : *initListExpr)
- checkForFunctionDecl(dyn_cast<Expr>(subStmt));
- return true;
-}
-
-void SalCall::checkForFunctionDecl(Expr const* expr, bool bCheckOnly)
-{
- auto e1 = expr->IgnoreParenCasts();
- auto declRef = dyn_cast<DeclRefExpr>(e1);
- if (!declRef)
- return;
- auto functionDecl = dyn_cast<FunctionDecl>(declRef->getDecl());
- if (!functionDecl)
- return;
- if (bCheckOnly)
- getParentStmt(expr)->dump();
- else
- m_addressOfSet.insert(functionDecl->getCanonicalDecl());
-}
-
bool SalCall::VisitFunctionDecl(FunctionDecl const* decl)
{
- if (m_phase != PluginPhase::Warning)
- return true;
if (ignoreLocation(decl))
return true;
@@ -225,16 +154,15 @@ bool SalCall::VisitFunctionDecl(FunctionDecl const* decl)
if (!decl->isThisDeclarationADefinition() && !(methodDecl && methodDecl->isPure()))
return true;
- // can only check when we have a definition since this is the most likely time
- // when the address of the method will be taken
- if (methodDecl)
- {
- if (m_addressOfSet.find(decl->getCanonicalDecl()) != m_addressOfSet.end())
- return true;
- }
+ m_decls.insert(decl);
+ return true;
+}
+
+void SalCall::handleFunctionDecl(FunctionDecl const* decl)
+{
// some base classes are overridden by sub-classes which override both the base-class and a UNO class
- if (recordDecl)
+ if (auto recordDecl = dyn_cast<CXXRecordDecl>(decl->getDeclContext()))
{
auto dc = loplugin::DeclCheck(recordDecl);
if (dc.Class("OProxyAggregation").Namespace("comphelper").GlobalNamespace()
@@ -266,11 +194,13 @@ bool SalCall::VisitFunctionDecl(FunctionDecl const* decl)
|| dc.Class("ScVbaControl").GlobalNamespace()
)
- return true;
+ return;
}
+ auto canonicalDecl = decl->getCanonicalDecl();
+
// if any of the overridden methods are SAL_CALL, we should be too
- if (methodDecl)
+ if (auto methodDecl = dyn_cast<CXXMethodDecl>(canonicalDecl))
{
for (auto iter = methodDecl->begin_overridden_methods();
iter != methodDecl->end_overridden_methods(); ++iter)
@@ -278,17 +208,22 @@ bool SalCall::VisitFunctionDecl(FunctionDecl const* decl)
const CXXMethodDecl* overriddenMethod
= getTemplateInstantiationPattern(*iter)->getCanonicalDecl();
if (isSalCallFunction(overriddenMethod))
- return true;
+ return;
}
}
+ SourceLocation rewriteLoc;
+ SourceLocation rewriteCanonicalLoc;
+ bool bDeclIsSalCall = isSalCallFunction(decl, &rewriteLoc);
+ isSalCallFunction(canonicalDecl, &rewriteCanonicalLoc);
+
bool bOK = rewrite(rewriteLoc);
if (bOK && canonicalDecl != decl)
{
bOK = rewrite(rewriteCanonicalLoc);
}
if (bOK)
- return true;
+ return;
if (bDeclIsSalCall)
{
@@ -307,8 +242,6 @@ bool SalCall::VisitFunctionDecl(FunctionDecl const* decl)
<< decl->getSourceRange();
}
}
-
- return true;
}
//TODO: This doesn't handle all possible cases of macro usage (and possibly never will be able to),
diff --git a/compilerplugins/clang/test/salcall.cxx b/compilerplugins/clang/test/salcall.cxx
index e424432c4036..a4464cee6064 100644
--- a/compilerplugins/clang/test/salcall.cxx
+++ b/compilerplugins/clang/test/salcall.cxx
@@ -134,6 +134,31 @@ void SAL_CALL Class9::method1() // expected-error {{SAL_CALL inconsistency [lopl
#define M2(T) T SAL_CALL
M2(void) Class9::method2() {} // expected-error {{SAL_CALL inconsistency [loplugin:salcall]}}
+void SAL_CALL f0() {} // expected-error {{SAL_CALL unnecessary here [loplugin:salcall]}}
+
+void SAL_CALL f1() {}
+
+void SAL_CALL f2() {}
+
+void SAL_CALL f3() {}
+
+void SAL_CALL f4() {}
+
+typedef void SAL_CALL (*Ptr)();
+
+void takePtr(Ptr);
+
+void usePtr()
+{
+ f0();
+ takePtr(f1);
+ takePtr(&f2);
+ Ptr p = f3;
+ takePtr(p);
+ p = f4;
+ takePtr(p);
+}
+
#if 0 // see TODO in SalCall::isSalCallFunction
class Class10
{