From 72f36adb93d24a3da8868dad128ab2eca0124fda Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Tue, 18 Jul 2017 09:02:03 +0200 Subject: loplugin:constparams in vcl Change-Id: I36afe2107e07ffb9b73c0b76be600e3e999a0fd4 Reviewed-on: https://gerrit.libreoffice.org/40116 Tested-by: Jenkins Reviewed-by: Noel Grandin --- compilerplugins/clang/constparams.cxx | 103 ++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 47 deletions(-) (limited to 'compilerplugins/clang/constparams.cxx') diff --git a/compilerplugins/clang/constparams.cxx b/compilerplugins/clang/constparams.cxx index 95325c57d1a7..024ee4ffd53e 100644 --- a/compilerplugins/clang/constparams.cxx +++ b/compilerplugins/clang/constparams.cxx @@ -36,8 +36,9 @@ public: bool VisitFunctionDecl(FunctionDecl *); bool VisitDeclRefExpr( const DeclRefExpr* ); + private: - bool checkIfCanBeConst(const Stmt*); + bool checkIfCanBeConst(const Stmt*, const ParmVarDecl*); bool isPointerOrReferenceToConst(const QualType& qt); StringRef getFilename(const SourceLocation& loc); @@ -85,11 +86,29 @@ bool ConstParams::VisitFunctionDecl(FunctionDecl * functionDecl) 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 || name == "PDFSigningPKCS7PasswordCallback" || name == "VCLExceptionSignal_impl" || name == "parseXcsFile" + || name == "GraphicsExposePredicate" + || name == "ImplPredicateEvent" + || name == "timestamp_predicate" + || name == "signalScreenSizeChanged" + || name == "signalMonitorsChanged" + || name == "signalButton" + || name == "signalFocus" + || name == "signalDestroy" + || name == "signalSettingsNotify" + || name == "signalStyleSet" + || name == "signalIMCommit" + || name == "compressWheelEvents" + || name == "MenuBarSignalKey" + || name == "signalDragDropReceived" // UNO component entry points - || name.endswith("component_getFactory")) + || name.endswith("component_getFactory") + // in Scheduler::, wants to loop until a reference to a bool becomes true + || name == "ProcessEventsToSignal" + ) return true; } @@ -120,7 +139,7 @@ bool ConstParams::VisitFunctionDecl(FunctionDecl * functionDecl) continue; if (type.LvalueReference().Const()) continue; - // since we can't normally change typedefs, just ignore them + // since we normally can't change typedefs, just ignore them if (isa(pParmVarDecl->getType())) continue; // TODO ignore these for now, has some effects I don't understand @@ -156,7 +175,6 @@ bool ConstParams::VisitFunctionDecl(FunctionDecl * functionDecl) pOther->getLocStart()) << pOther->getSourceRange(); } - //functionDecl->dump(); } } return ret; @@ -174,13 +192,15 @@ bool ConstParams::VisitDeclRefExpr( const DeclRefExpr* declRefExpr ) if (interestingSet.find(parmVarDecl) == interestingSet.end()) { return true; } - if (!checkIfCanBeConst(declRefExpr)) + if (!checkIfCanBeConst(declRefExpr, parmVarDecl)) cannotBeConstSet.insert(parmVarDecl); return true; } -bool ConstParams::checkIfCanBeConst(const Stmt* stmt) +// Walk up from a DeclRefExpr to a ParamVarDecl, checking if the usage means that the +// ParamVarDecl can be const. +bool ConstParams::checkIfCanBeConst(const Stmt* stmt, const ParmVarDecl* parmVarDecl) { const Stmt* parent = parentStmt( stmt ); if (isa(parent)) { @@ -191,7 +211,7 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt) return false; } if (op == UO_Deref) { - return checkIfCanBeConst(parent); + return checkIfCanBeConst(parent, parmVarDecl); } return true; } else if (auto binaryOp = dyn_cast(parent)) { @@ -211,7 +231,7 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt) } // for pointer arithmetic need to check parent if (binaryOp->getType()->isPointerType()) { - return checkIfCanBeConst(parent); + return checkIfCanBeConst(parent, parmVarDecl); } return true; } else if (isa(parent)) { @@ -222,8 +242,6 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt) return isPointerOrReferenceToConst(constructorDecl->getParamDecl(i)->getType()); } } - std::cout << "dump0" << std::endl; - parent->dump(); } else if (isa(parent)) { const CXXOperatorCallExpr* operatorCallExpr = dyn_cast(parent); const CXXMethodDecl* calleeMethodDecl = dyn_cast(operatorCallExpr->getDirectCallee()); @@ -254,9 +272,6 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt) } } } - std::cout << "dump1" << std::endl; - parent->dump(); - stmt->dump(); } else if (isa(parent)) { const CallExpr* callExpr = dyn_cast(parent); QualType functionType = callExpr->getCallee()->getType(); @@ -287,20 +302,6 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt) return calleeMethodDecl->isConst(); } } - if (!calleeFunctionDecl) { - std::cout << "---------------dump2" << std::endl; - std::cout << "parent" << std::endl; - parent->dump(); - std::cout << "child" << std::endl; - stmt->dump(); - std::cout << "callee-type" << std::endl; - callExpr->getCallee()->getType()->dump(); - for (unsigned i = 0; i < callExpr->getNumArgs(); ++i) { - std::cout << " param " << i << std::endl; - callExpr->getArg(i)->dump(); - } - return true; - } // TODO could do better if (calleeFunctionDecl->isVariadic()) { return false; @@ -315,32 +316,18 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt) return isPointerOrReferenceToConst(calleeFunctionDecl->getParamDecl(i)->getType()); } } - std::cout << "------------dump3" << std::endl; - std::cout << "parent" << std::endl; - parent->dump(); - std::cout << "child" << std::endl; - stmt->dump(); - calleeFunctionDecl->dump(); - for (unsigned i = 0; i < callExpr->getNumArgs(); ++i) { - std::cout << " param " << i << std::endl; - callExpr->getArg(i)->dump(); - } - if (isa(parent)) { - std::cout << "implicit" << std::endl; - dyn_cast(parent)->getImplicitObjectArgument()->dump(); - } } else if (isa(parent)) { return false; } else if (isa(parent)) { return false; } else if (isa(parent)) { // all other cast expression subtypes - return checkIfCanBeConst(parent); + return checkIfCanBeConst(parent, parmVarDecl); } else if (isa(parent)) { - return checkIfCanBeConst(parent); + return checkIfCanBeConst(parent, parmVarDecl); } else if (isa(parent)) { - return checkIfCanBeConst(parent); + return checkIfCanBeConst(parent, parmVarDecl); } else if (isa(parent)) { - return checkIfCanBeConst(parent); + return checkIfCanBeConst(parent, parmVarDecl); } else if (isa(parent)) { // TODO could do better here, but would require tracking the target(s) return false; @@ -367,10 +354,32 @@ bool ConstParams::checkIfCanBeConst(const Stmt* stmt) } else if (const ConditionalOperator* conditionalExpr = dyn_cast(parent)) { if (conditionalExpr->getCond() == stmt) return true; - return checkIfCanBeConst(parent); + return checkIfCanBeConst(parent, parmVarDecl); + } else if (isa(parent)) { + return false; // ??? + } else if (isa(parent)) { + return true; // because the ParamVarDecl must be a parameter to the expression, probably an array count + } else if (auto lambdaExpr = dyn_cast(parent)) { + for (auto it = lambdaExpr->capture_begin(); it != lambdaExpr->capture_end(); ++it) + { + if (it->getCapturedVar() == parmVarDecl) + return it->getCaptureKind() != LCK_ByRef; + } + report( + DiagnosticsEngine::Warning, + "cannot handle this lambda", + parent->getLocStart()) + << parent->getSourceRange(); + parent->dump(); + parmVarDecl->dump(); } else { - std::cout << "dump5" << std::endl; + report( + DiagnosticsEngine::Warning, + "oh dear, what can the matter be?", + parent->getLocStart()) + << parent->getSourceRange(); parent->dump(); + parmVarDecl->dump(); } return true; } -- cgit v1.2.3