loplugin:constparams: Ignore functions whose address is taken
(unless as the callee of a function call). In response to
<https://gerrit.libreoffice.org/#/c/42912/> "DO NOT MERGE - error in clang
static plugin". Many of the whitelisted functions can now be taken off the
list.
Change-Id: I04c2ee445e7973a288f42fd663d8b2d78cd3c5aa
Reviewed-on: https://gerrit.libreoffice.org/42958
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Tested-by: Stephan Bergmann <sbergman@redhat.com>
diff --git a/compilerplugins/clang/constparams.cxx b/compilerplugins/clang/constparams.cxx
index 62300f4..5ce9df2 100644
--- a/compilerplugins/clang/constparams.cxx
+++ b/compilerplugins/clang/constparams.cxx
@@ -8,6 +8,7 @@
*/
#include <algorithm>
#include <set>
#include <string>
#include <unordered_set>
#include <unordered_map>
@@ -61,15 +62,21 @@ public:
for (const ParmVarDecl *pParmVarDecl : interestingSet) {
if (cannotBeConstSet.find(pParmVarDecl) == cannotBeConstSet.end()) {
auto functionDecl = parmToFunction[pParmVarDecl];
auto canonicalDecl = functionDecl->getCanonicalDecl();
if (ignoredFunctions_.find(canonicalDecl)
!= ignoredFunctions_.end())
{
continue;
}
report(
DiagnosticsEngine::Warning,
"this parameter can be const",
pParmVarDecl->getLocStart())
<< pParmVarDecl->getSourceRange();
auto functionDecl = parmToFunction[pParmVarDecl];
if (functionDecl->getCanonicalDecl()->getLocation() != functionDecl->getLocation()) {
if (canonicalDecl->getLocation() != functionDecl->getLocation()) {
unsigned idx = pParmVarDecl->getFunctionScopeIndex();
const ParmVarDecl* pOther = functionDecl->getCanonicalDecl()->getParamDecl(idx);
const ParmVarDecl* pOther = canonicalDecl->getParamDecl(idx);
report(
DiagnosticsEngine::Note,
"canonical parameter declaration here",
@@ -80,6 +87,86 @@ public:
}
}
bool TraverseCallExpr(CallExpr * expr) {
auto const saved = callee_;
callee_ = expr->getCallee();
auto const ret = RecursiveASTVisitor::TraverseCallExpr(expr);
callee_ = saved;
return ret;
}
bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr * expr) {
auto const saved = callee_;
callee_ = expr->getCallee();
auto const ret = RecursiveASTVisitor::TraverseCXXOperatorCallExpr(expr);
callee_ = saved;
return ret;
}
bool TraverseCXXMemberCallExpr(CXXMemberCallExpr * expr) {
auto const saved = callee_;
callee_ = expr->getCallee();
auto const ret = RecursiveASTVisitor::TraverseCXXMemberCallExpr(expr);
callee_ = saved;
return ret;
}
bool TraverseCUDAKernelCallExpr(CUDAKernelCallExpr * expr) {
auto const saved = callee_;
callee_ = expr->getCallee();
auto const ret = RecursiveASTVisitor::TraverseCUDAKernelCallExpr(expr);
callee_ = saved;
return ret;
}
bool TraverseUserDefinedLiteral(UserDefinedLiteral * expr) {
auto const saved = callee_;
callee_ = expr->getCallee();
auto const ret = RecursiveASTVisitor::TraverseUserDefinedLiteral(expr);
callee_ = saved;
return ret;
}
bool VisitImplicitCastExpr(ImplicitCastExpr const * expr) {
if (expr == callee_) {
return true;
}
if (ignoreLocation(expr)) {
return true;
}
if (expr->getCastKind() != CK_FunctionToPointerDecay) {
return true;
}
auto const dre = dyn_cast<DeclRefExpr>(
expr->getSubExpr()->IgnoreParens());
if (dre == nullptr) {
return true;
}
auto const fd = dyn_cast<FunctionDecl>(dre->getDecl());
if (fd == nullptr) {
return true;
}
ignoredFunctions_.insert(fd->getCanonicalDecl());
return true;
}
bool VisitUnaryAddrOf(UnaryOperator const * expr) {
if (ignoreLocation(expr)) {
return true;
}
auto const dre = dyn_cast<DeclRefExpr>(
expr->getSubExpr()->IgnoreParenImpCasts());
if (dre == nullptr) {
return true;
}
auto const fd = dyn_cast<FunctionDecl>(dre->getDecl());
if (fd == nullptr) {
return true;
}
ignoredFunctions_.insert(fd->getCanonicalDecl());
return true;
}
bool VisitFunctionDecl(const FunctionDecl *);
bool VisitDeclRefExpr(const DeclRefExpr *);
@@ -90,6 +177,8 @@ private:
std::unordered_set<const ParmVarDecl*> interestingSet;
std::unordered_map<const ParmVarDecl*, const FunctionDecl*> parmToFunction;
std::unordered_set<const ParmVarDecl*> cannotBeConstSet;
std::set<FunctionDecl const *> ignoredFunctions_;
Expr const * callee_ = nullptr;
};
bool ConstParams::VisitFunctionDecl(const FunctionDecl * functionDecl)
@@ -128,65 +217,17 @@ bool ConstParams::VisitFunctionDecl(const FunctionDecl * functionDecl)
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
|| 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"
|| name == "memory_write"
|| name == "file_write"
if ( name == "file_write"
|| name == "SalMainPipeExchangeSignal_impl"
|| name.startswith("SbRtl_")
|| name == "my_if_errors"
|| name == "my_eval_defined"
|| name == "my_eval_variable"
|| name == "ImpGetEscDir"
|| name == "ImpGetPercent"
|| name == "ImpGetAlign"
|| name == "write_function"
|| name == "PyUNO_getattr"
|| name == "PyUNO_setattr"
|| name == "PyUNOStruct_setattr"
|| name == "PyUNOStruct_getattr"
|| name == "GoNext"
|| name == "GoPrevious"
|| name == "lcl_SetOtherLineHeight"
|| name == "BoxNmsToPtr"
|| name == "PtrToBoxNms"
|| name == "RelNmsToBoxNms"
|| name == "RelBoxNmsToPtr"
|| name == "BoxNmsToRelNm"
|| name == "MakeFormula_"
|| name == "GetFormulaBoxes"
|| name == "HasValidBoxes_"
|| name == "SplitMergeBoxNm_"
|| name.startswith("Read_F_")
|| name == "UpdateFieldInformation"
// #ifdef win32
|| name == "convert_slashes"
// UNO component entry points
|| name.endswith("component_getFactory")
// external API
|| name == "Java_com_sun_star_sdbcx_comp_hsqldb_StorageNativeOutputStream_flush"
|| name == "egiGraphicExport"
|| name == "etiGraphicExport"
|| name == "epsGraphicExport"
|| name == "releasePool" // vcl/osx/saldata.cxx
)
return true;
}
diff --git a/compilerplugins/clang/test/constparams.cxx b/compilerplugins/clang/test/constparams.cxx
index fd4ee2c..feb07f1 100644
--- a/compilerplugins/clang/test/constparams.cxx
+++ b/compilerplugins/clang/test/constparams.cxx
@@ -24,4 +24,26 @@ struct Class3
Class3(void * f2) : m_f2(static_cast<int*>(f2)) {}
};
int const * f1(int *); // expected-note {{canonical parameter declaration here [loplugin:constparams]}}
int const * f2(int *);
int const * f3(int *);
void g() {
int const * (*p1)(int *);
int n = 0;
f1(&n);
p1 = f2;
typedef void (*P2)();
P2 p2;
p2 = (P2) (f3);
}
int const * f1(int * p) { // expected-error {{this parameter can be const [loplugin:constparams]}}
return p;
}
int const * f2(int * p) {
return p;
}
int const * f3(int * p) {
return p;
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
