improve loplugin constparams
Change-Id: Ic1833dbd030044011e7ee5f89dc35737e5469f05
Reviewed-on: https://gerrit.libreoffice.org/66586
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
diff --git a/compilerplugins/clang/constparams.cxx b/compilerplugins/clang/constparams.cxx
index cccc454..3f1aad8 100644
--- a/compilerplugins/clang/constparams.cxx
+++ b/compilerplugins/clang/constparams.cxx
@@ -76,11 +76,12 @@ public:
{
continue;
}
std::string fname = functionDecl->getQualifiedNameAsString();
report(
DiagnosticsEngine::Warning,
"this parameter can be const",
"this parameter can be const %0",
compat::getBeginLoc(pParmVarDecl))
<< pParmVarDecl->getSourceRange();
<< fname << pParmVarDecl->getSourceRange();
if (canonicalDecl->getLocation() != functionDecl->getLocation()) {
unsigned idx = pParmVarDecl->getFunctionScopeIndex();
const ParmVarDecl* pOther = canonicalDecl->getParamDecl(idx);
@@ -150,16 +151,8 @@ bool ConstParams::CheckTraverseFunctionDecl(FunctionDecl * functionDecl)
if (isInUnoIncludeFile(functionDecl)) {
return false;
}
// TODO ignore template stuff for now
if (functionDecl->getTemplatedKind() != FunctionDecl::TK_NonTemplate) {
return false;
}
if (functionDecl->isDeleted())
return false;
if (isa<CXXMethodDecl>(functionDecl)
&& dyn_cast<CXXMethodDecl>(functionDecl)->getParent()->getDescribedClassTemplate() != nullptr ) {
return false;
}
// ignore virtual methods
if (isa<CXXMethodDecl>(functionDecl)
&& dyn_cast<CXXMethodDecl>(functionDecl)->isVirtual() ) {
@@ -210,10 +203,19 @@ bool ConstParams::CheckTraverseFunctionDecl(FunctionDecl * functionDecl)
|| name == "Read_And"
// passed as a LINK<> to another method
|| name == "GlobalBasicErrorHdl_Impl"
// template
|| name == "extract_throw" || name == "readProp"
)
return false;
}
std::string fqn = functionDecl->getQualifiedNameAsString();
if ( fqn == "connectivity::jdbc::GlobalRef::set"
|| fqn == "(anonymous namespace)::ReorderNotifier::operator()"
|| fqn == "static_txtattr_cast")
return false;
// calculate the ones we want to check
bool foundInterestingParam = false;
for (const ParmVarDecl *pParmVarDecl : functionDecl->parameters()) {
@@ -222,11 +224,8 @@ bool ConstParams::CheckTraverseFunctionDecl(FunctionDecl * functionDecl)
|| pParmVarDecl->hasAttr<UnusedAttr>())
continue;
auto const type = loplugin::TypeCheck(pParmVarDecl->getType());
if (!type.Pointer() && !type.LvalueReference())
continue;
if (type.Pointer().Const())
continue;
if (type.LvalueReference().Const())
if (!( type.Pointer().NonConst()
|| type.LvalueReference().NonConst()))
continue;
// since we normally can't change typedefs, just ignore them
if (isa<TypedefType>(pParmVarDecl->getType()))
@@ -240,11 +239,6 @@ bool ConstParams::CheckTraverseFunctionDecl(FunctionDecl * functionDecl)
// const is meaningless when applied to function pointer types
if (pParmVarDecl->getType()->isFunctionPointerType())
continue;
// ignore things with template params
if (pParmVarDecl->getType()->isInstantiationDependentType())
continue;
if (functionDecl->getIdentifier() && functionDecl->getName() == "WW8TransCol")
pParmVarDecl->getType()->dump();
interestingParamSet.insert(pParmVarDecl);
parmToFunction[pParmVarDecl] = functionDecl;
foundInterestingParam = true;
@@ -281,12 +275,23 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVar
{
for ( auto cxxCtorInitializer : cxxConstructorDecl->inits())
{
if (cxxCtorInitializer->isAnyMemberInitializer() && cxxCtorInitializer->getInit() == stmt)
if ( cxxCtorInitializer->getInit() == stmt)
{
// if the member is not pointer or ref to-const, we cannot make the param const
auto fieldDecl = cxxCtorInitializer->getAnyMember();
auto tc = loplugin::TypeCheck(fieldDecl->getType());
return tc.Pointer().Const() || tc.LvalueReference().Const();
if (cxxCtorInitializer->isAnyMemberInitializer())
{
// if the member is not pointer-to-const or ref-to-const or value, we cannot make the param const
auto fieldDecl = cxxCtorInitializer->getAnyMember();
auto tc = loplugin::TypeCheck(fieldDecl->getType());
if (tc.Pointer() || tc.LvalueReference())
return tc.Pointer().Const() || tc.LvalueReference().Const();
else
return true;
}
else
{
// probably base initialiser, but no simple way to look up the relevant constructor decl
return false;
}
}
}
}
@@ -392,6 +397,7 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVar
}
}
}
return false;
} else if (auto callExpr = dyn_cast<CallExpr>(parent)) {
QualType functionType = callExpr->getCallee()->getType();
if (functionType->isFunctionPointerType()) {
@@ -437,6 +443,7 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVar
}
}
}
return false;
} else if (auto callExpr = dyn_cast<ObjCMessageExpr>(parent)) {
if (callExpr->getInstanceReceiver() == stmt) {
return true;
@@ -526,7 +533,7 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVar
} else if (isa<CXXTypeidExpr>(parent)) {
return true;
} else if (isa<ParenListExpr>(parent)) {
return true;
return false; // could be improved, seen in constructors when calling base class constructor
} else if (isa<CXXUnresolvedConstructExpr>(parent)) {
return false;
} else if (isa<UnresolvedMemberExpr>(parent)) {
diff --git a/compilerplugins/clang/test/constparams.cxx b/compilerplugins/clang/test/constparams.cxx
index 9390ce2..2cffd87 100644
--- a/compilerplugins/clang/test/constparams.cxx
+++ b/compilerplugins/clang/test/constparams.cxx
@@ -12,7 +12,7 @@
struct Class1
{
int const * m_f1;
Class1(int * f1) : m_f1(f1) {} // expected-error {{this parameter can be const [loplugin:constparams]}}
Class1(int * f1) : m_f1(f1) {} // expected-error {{this parameter can be const Class1::Class1 [loplugin:constparams]}}
};
struct Class2
@@ -38,7 +38,7 @@ void g() {
P2 p2;
p2 = (P2) (f3);
}
int const * f1(int * p) { // expected-error {{this parameter can be const [loplugin:constparams]}}
int const * f1(int * p) { // expected-error {{this parameter can be const f1 [loplugin:constparams]}}
return p;
}
void f4(std::string * p) {
