mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

loplugin:constparams handle constructors

Browse Source 
had to change the structure of the plugin considerably, was too messy to
structure it to do the calculations on a per-function basis

Change-Id: I4edee7735f726101105c607368124a08dba21086
Reviewed-on: https://gerrit.libreoffice.org/40516
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
This commit is contained in:
Noel Grandin
2017-07-28 11:47:51 +02:00
parent 587d6e13df
commit b09deb0753
82 changed files with 307 additions and 240 deletions
Download Patch File Download Diff File Expand all files Collapse all files

248
compilerplugins/clang/constparams.cxx
View File

@@ -8,7 +8,8 @@
*/
#include <string>
#include <set>
#include <unordered_set>
#include <unordered_map>
#include <iostream>
#include "plugin.hxx"
@@ -24,6 +25,10 @@
namespace
{
static bool startswith(const std::string& rStr, const char* pSubStr) {
return rStr.compare(0, strlen(pSubStr), pSubStr) == 0;
}
class ConstParams:
public RecursiveASTVisitor<ConstParams>, public loplugin::Plugin
{
@@ -31,35 +36,69 @@ public:
explicit ConstParams(InstantiationData const & data): Plugin(data) {}
virtual void run() override {
std::string fn( compiler.getSourceManager().getFileEntryForID(
compiler.getSourceManager().getMainFileID())->getName() );
normalizeDotDotInFilePath(fn);
if (startswith(fn, SRCDIR "/sal/")
|| startswith(fn, SRCDIR "/bridges/")
|| startswith(fn, SRCDIR "/binaryurp/")
|| startswith(fn, SRCDIR "/stoc/")
|| startswith(fn, WORKDIR "/YaccTarget/unoidl/source/sourceprovider-parser.cxx")
// some weird calling through a function pointer
|| startswith(fn, SRCDIR "/svtools/source/table/defaultinputhandler.cxx")
// windows only
|| startswith(fn, SRCDIR "/basic/source/sbx/sbxdec.cxx")
|| startswith(fn, SRCDIR "/sfx2/source/doc/syspath.cxx")
// ignore this for now
|| startswith(fn, SRCDIR "/libreofficekit")
)
return;
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
for (const ParmVarDecl *pParmVarDecl : interestingSet) {
if (cannotBeConstSet.find(pParmVarDecl) == cannotBeConstSet.end()) {
report(
DiagnosticsEngine::Warning,
"this parameter can be const",
pParmVarDecl->getLocStart())
<< pParmVarDecl->getSourceRange();
auto functionDecl = parmToFunction[pParmVarDecl];
if (functionDecl->getCanonicalDecl()->getLocation() != functionDecl->getLocation()) {
unsigned idx = pParmVarDecl->getFunctionScopeIndex();
const ParmVarDecl* pOther = functionDecl->getCanonicalDecl()->getParamDecl(idx);
report(
DiagnosticsEngine::Note,
"canonical parameter declaration here",
pOther->getLocStart())
<< pOther->getSourceRange();
}
}
}
}
bool VisitFunctionDecl(FunctionDecl *);
bool VisitDeclRefExpr( const DeclRefExpr* );
bool VisitFunctionDecl(const FunctionDecl *);
bool VisitDeclRefExpr(const DeclRefExpr *);
private:
bool checkIfCanBeConst(const Stmt*, const ParmVarDecl*);
bool isPointerOrReferenceToConst(const QualType& qt);
StringRef getFilename(const SourceLocation& loc);
bool mbInsideFunction;
std::set<const ParmVarDecl*> interestingSet;
std::set<const ParmVarDecl*> cannotBeConstSet;
std::unordered_set<const ParmVarDecl*> interestingSet;
std::unordered_map<const ParmVarDecl*, const FunctionDecl*> parmToFunction;
std::unordered_set<const ParmVarDecl*> cannotBeConstSet;
};
bool ConstParams::VisitFunctionDecl(FunctionDecl * functionDecl)
bool ConstParams::VisitFunctionDecl(const FunctionDecl * functionDecl)
{
if (ignoreLocation(functionDecl) || !functionDecl->doesThisDeclarationHaveABody()) {
if (ignoreLocation(functionDecl) || !functionDecl->isThisDeclarationADefinition()) {
return true;
}
// ignore stuff that forms part of the stable URE interface
if (isInUnoIncludeFile(functionDecl)) {
return true;
}
// TODO ignore these for now, requires some extra work
if (isa<CXXConstructorDecl>(functionDecl)) {
return true;
}
// TODO ignore template stuff for now
if (functionDecl->getTemplatedKind() != FunctionDecl::TK_NonTemplate) {
return true;
@@ -82,11 +121,12 @@ bool ConstParams::VisitFunctionDecl(FunctionDecl * functionDecl)
if (functionDecl->getLocation().isMacroID())
return true;
if (functionDecl->getIdentifier()) {
if (functionDecl->getIdentifier())
{
StringRef name = functionDecl->getName();
if (name == "yyerror" // ignore lex/yacc callback
// some function callbacks
// TODO should really use a two-pass algorithm to detect and ignore these automatically
// some function callbacks
// TODO should really use a two-pass algorithm to detect and ignore these automatically
|| name == "PDFSigningPKCS7PasswordCallback"
|| name == "VCLExceptionSignal_impl"
|| name == "parseXcsFile"
@@ -114,6 +154,7 @@ bool ConstParams::VisitFunctionDecl(FunctionDecl * functionDecl)
|| name == "ImpGetEscDir"
|| name == "ImpGetPercent"
|| name == "ImpGetAlign"
|| name == "write_function"
// #ifdef win32
|| name == "convert_slashes"
// UNO component entry points
@@ -129,22 +170,7 @@ bool ConstParams::VisitFunctionDecl(FunctionDecl * functionDecl)
return true;
}
StringRef aFileName = getFilename(functionDecl->getLocStart());
if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sal/")
|| loplugin::hasPathnamePrefix(aFileName, SRCDIR "/bridges/")
|| loplugin::hasPathnamePrefix(aFileName, SRCDIR "/binaryurp/")
|| loplugin::hasPathnamePrefix(aFileName, SRCDIR "/stoc/")
|| loplugin::hasPathnamePrefix(aFileName, WORKDIR "/YaccTarget/unoidl/source/sourceprovider-parser.cxx")
// some weird calling through a function pointer
|| loplugin::hasPathnamePrefix(aFileName, SRCDIR "/svtools/source/table/defaultinputhandler.cxx")
// windows only
|| loplugin::hasPathnamePrefix(aFileName, SRCDIR "/basic/source/sbx/sbxdec.cxx")
|| loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sfx2/source/doc/syspath.cxx")) {
return true;
}
// calculate the ones we want to check
interestingSet.clear();
for (const ParmVarDecl *pParmVarDecl : compat::parameters(*functionDecl)) {
// ignore unused params
if (pParmVarDecl->getName().empty())
@@ -165,50 +191,29 @@ bool ConstParams::VisitFunctionDecl(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;
interestingSet.insert(pParmVarDecl);
}
if (interestingSet.empty()) {
return true;
parmToFunction[pParmVarDecl] = functionDecl;
}
mbInsideFunction = true;
cannotBeConstSet.clear();
bool ret = RecursiveASTVisitor::TraverseStmt(functionDecl->getBody());
mbInsideFunction = false;
for (const ParmVarDecl *pParmVarDecl : interestingSet) {
if (cannotBeConstSet.find(pParmVarDecl) == cannotBeConstSet.end()) {
report(
DiagnosticsEngine::Warning,
"this parameter can be const",
pParmVarDecl->getLocStart())
<< pParmVarDecl->getSourceRange();
if (functionDecl->getCanonicalDecl()->getLocation() != functionDecl->getLocation()) {
unsigned idx = pParmVarDecl->getFunctionScopeIndex();
ParmVarDecl* pOther = functionDecl->getCanonicalDecl()->getParamDecl(idx);
report(
DiagnosticsEngine::Note,
"canonical parameter declaration here",
pOther->getLocStart())
<< pOther->getSourceRange();
}
}
}
return ret;
return true;
}
bool ConstParams::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
{
if (!mbInsideFunction) {
if (ignoreLocation(declRefExpr)) {
return true;
}
// ignore stuff that forms part of the stable URE interface
if (isInUnoIncludeFile(declRefExpr->getLocStart())) {
return true;
}
const ParmVarDecl* parmVarDecl = dyn_cast_or_null<ParmVarDecl>(declRefExpr->getDecl());
if (!parmVarDecl) {
return true;
}
if (interestingSet.find(parmVarDecl) == interestingSet.end()) {
return true;
}
// no need to check again if we have already eliminated this one
if (cannotBeConstSet.find(parmVarDecl) != cannotBeConstSet.end())
return true;
@@ -218,13 +223,41 @@ bool ConstParams::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
return true;
}
// Walk up from a DeclRefExpr to a ParamVarDecl, checking if the usage means that the
// ParamVarDecl can be const.
// Walk up from a statement that contains a DeclRefExpr, checking if the usage means that the
// related ParamVarDecl can be const.
bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVarDecl)
{
const Stmt* parent = parentStmt( stmt );
if (isa<UnaryOperator>(parent)) {
const UnaryOperator* unaryOperator = dyn_cast<UnaryOperator>(parent);
if (!parent)
{
// check if we're inside a CXXCtorInitializer
auto parentsRange = compiler.getASTContext().getParents(*stmt);
if ( parentsRange.begin() == parentsRange.end())
return true;
auto cxxConstructorDecl = dyn_cast_or_null<CXXConstructorDecl>(parentsRange.begin()->get<Decl>());
if (!cxxConstructorDecl)
return true;
for ( auto cxxCtorInitializer : cxxConstructorDecl->inits())
{
if (cxxCtorInitializer->isAnyMemberInitializer() && 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();
}
}
parmVarDecl->dump();
stmt->dump();
cxxConstructorDecl->dump();
report(
DiagnosticsEngine::Warning,
"couldn't find the CXXCtorInitializer?",
stmt->getLocStart())
<< stmt->getSourceRange();
return false;
}
if (auto unaryOperator = dyn_cast<UnaryOperator>(parent)) {
UnaryOperator::Opcode op = unaryOperator->getOpcode();
if (op == UO_AddrOf || op == UO_PreInc || op == UO_PostInc
|| op == UO_PreDec || op == UO_PostDec) {
@@ -254,17 +287,15 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVar
return checkIfCanBeConst(parent, parmVarDecl);
}
return true;
} else if (isa<CXXConstructExpr>(parent)) {
const CXXConstructExpr* constructExpr = dyn_cast<CXXConstructExpr>(parent);
} else if (auto constructExpr = dyn_cast<CXXConstructExpr>(parent)) {
const CXXConstructorDecl * constructorDecl = constructExpr->getConstructor();
for (unsigned i = 0; i < constructExpr->getNumArgs(); ++i) {
if (constructExpr->getArg(i) == stmt) {
return isPointerOrReferenceToConst(constructorDecl->getParamDecl(i)->getType());
}
}
} else if (isa<CXXOperatorCallExpr>(parent)) {
const CXXOperatorCallExpr* operatorCallExpr = dyn_cast<CXXOperatorCallExpr>(parent);
const CXXMethodDecl* calleeMethodDecl = dyn_cast<CXXMethodDecl>(operatorCallExpr->getDirectCallee());
} else if (auto operatorCallExpr = dyn_cast<CXXOperatorCallExpr>(parent)) {
const CXXMethodDecl* calleeMethodDecl = dyn_cast_or_null<CXXMethodDecl>(operatorCallExpr->getDirectCallee());
if (calleeMethodDecl) {
// unary operator
if (calleeMethodDecl->getNumParams() == 0) {
@@ -293,8 +324,7 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVar
}
}
}
} else if (isa<CallExpr>(parent)) {
const CallExpr* callExpr = dyn_cast<CallExpr>(parent);
} else if (auto callExpr = dyn_cast<CallExpr>(parent)) {
QualType functionType = callExpr->getCallee()->getType();
if (functionType->isFunctionPointerType()) {
functionType = functionType->getPointeeType();
@@ -314,27 +344,29 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVar
}
}
const FunctionDecl* calleeFunctionDecl = callExpr->getDirectCallee();
if (isa<CXXMemberCallExpr>(parent)) {
const CXXMemberCallExpr* memberCallExpr = dyn_cast<CXXMemberCallExpr>(parent);
const MemberExpr* memberExpr = dyn_cast<MemberExpr>(stmt);
if (memberExpr && memberCallExpr->getImplicitObjectArgument() == memberExpr->getBase())
{
const CXXMethodDecl* calleeMethodDecl = dyn_cast<CXXMethodDecl>(calleeFunctionDecl);
return calleeMethodDecl->isConst();
if (calleeFunctionDecl)
{
if (auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(parent)) {
const MemberExpr* memberExpr = dyn_cast<MemberExpr>(stmt);
if (memberExpr && memberCallExpr->getImplicitObjectArgument() == memberExpr->getBase())
{
const CXXMethodDecl* calleeMethodDecl = dyn_cast<CXXMethodDecl>(calleeFunctionDecl);
return calleeMethodDecl->isConst();
}
}
}
// TODO could do better
if (calleeFunctionDecl->isVariadic()) {
return false;
}
if (callExpr->getCallee() == stmt) {
return true;
}
for (unsigned i = 0; i < callExpr->getNumArgs(); ++i) {
if (i >= calleeFunctionDecl->getNumParams()) // can happen in template code
break;
if (callExpr->getArg(i) == stmt) {
return isPointerOrReferenceToConst(calleeFunctionDecl->getParamDecl(i)->getType());
// TODO could do better
if (calleeFunctionDecl->isVariadic()) {
return false;
}
if (callExpr->getCallee() == stmt) {
return true;
}
for (unsigned i = 0; i < callExpr->getNumArgs(); ++i) {
if (i >= calleeFunctionDecl->getNumParams()) // can happen in template code
break;
if (callExpr->getArg(i) == stmt) {
return isPointerOrReferenceToConst(calleeFunctionDecl->getParamDecl(i)->getType());
}
}
}
} else if (isa<CXXReinterpretCastExpr>(parent)) {
@@ -370,15 +402,20 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVar
return false;
} else if (isa<VAArgExpr>(parent)) {
return false;
} else if (isa<CXXDependentScopeMemberExpr>(parent)) {
return false;
} else if (isa<MaterializeTemporaryExpr>(parent)) {
return true;
} else if (const ConditionalOperator* conditionalExpr = dyn_cast<ConditionalOperator>(parent)) {
} else if (auto conditionalExpr = dyn_cast<ConditionalOperator>(parent)) {
if (conditionalExpr->getCond() == stmt)
return true;
return checkIfCanBeConst(parent, parmVarDecl);
} else if (isa<UnaryExprOrTypeTraitExpr>(parent)) {
return false; // ???
} else if (isa<CXXNewExpr>(parent)) {
} else if (auto cxxNewExpr = dyn_cast<CXXNewExpr>(parent)) {
for (auto pa : cxxNewExpr->placement_arguments())
if (pa == stmt)
return false;
return true; // because the ParamVarDecl must be a parameter to the expression, probably an array length
} else if (auto lambdaExpr = dyn_cast<LambdaExpr>(parent)) {
for (auto it = lambdaExpr->capture_begin(); it != lambdaExpr->capture_end(); ++it)
@@ -386,18 +423,21 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVar
if (it->capturesVariable() && it->getCapturedVar() == parmVarDecl)
return it->getCaptureKind() != LCK_ByRef;
}
/* sigh. just running this message will cause clang to crash (in sdext)
report(
DiagnosticsEngine::Warning,
"cannot handle this lambda",
parent->getLocStart())
<< parent->getSourceRange();
parent->dump();
parmVarDecl->dump();
*/
return false;
return false;
} else if (isa<CXXTypeidExpr>(parent)) {
return true;
} else if (isa<ParenListExpr>(parent)) {
return true;
} else if (isa<CXXUnresolvedConstructExpr>(parent)) {
return false;
} else if (isa<UnresolvedMemberExpr>(parent)) {
return false;
} else if (isa<PackExpansionExpr>(parent)) {
return false;
} else if (isa<ExprWithCleanups>(parent)) {
return checkIfCanBeConst(parent, parmVarDecl);
} else if (isa<CaseStmt>(parent)) {
return true;
} else {
parent->dump();
parmVarDecl->dump();