d1a2b80b9d
...as discussed in the mail thread starting at <https://lists.freedesktop.org/archives/libreoffice/2020-November/086234.html> "Bump --enable-compiler-plugins Clang baseline?" (and now picked up again at <https://lists.freedesktop.org/archives/libreoffice/2022-February/088459.html> "Re: Bump --enable-compiler-plugins Clang baseline?"), and clean up compilerplugins/clang/ accordingly Change-Id: I5e81c6fdcc363aeefd6227606225b526fdf7ac16 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129989 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
444 lines
15 KiB
C++
444 lines
15 KiB
C++
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
|
|
/*
|
|
* This file is part of the LibreOffice project.
|
|
*
|
|
* This Source Code Form is subject to the terms of the Mozilla Public
|
|
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
|
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
|
*/
|
|
|
|
#include <algorithm>
|
|
#include <cassert>
|
|
#include <deque>
|
|
#include <string>
|
|
#include <iostream>
|
|
#include <fstream>
|
|
#include <set>
|
|
|
|
#include <clang/AST/CXXInheritance.h>
|
|
|
|
#include "plugin.hxx"
|
|
#include "check.hxx"
|
|
|
|
/**
|
|
Simplify boolean expressions involving smart pointers e.g.
|
|
if (x.get())
|
|
can be
|
|
if (x)
|
|
*/
|
|
//TODO: Make this a shared plugin for Clang 12 (and possibly even for older Clang) again.
|
|
|
|
namespace
|
|
{
|
|
class SimplifyPointerToBool : public loplugin::FilteringRewritePlugin<SimplifyPointerToBool>
|
|
{
|
|
public:
|
|
explicit SimplifyPointerToBool(loplugin::InstantiationData const& data)
|
|
: FilteringRewritePlugin(data)
|
|
{
|
|
}
|
|
|
|
virtual void run() override
|
|
{
|
|
if (preRun())
|
|
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
|
|
}
|
|
|
|
bool VisitImplicitCastExpr(ImplicitCastExpr const*);
|
|
bool VisitBinaryOperator(BinaryOperator const*);
|
|
|
|
bool PreTraverseUnaryOperator(UnaryOperator* expr)
|
|
{
|
|
if (expr->getOpcode() == UO_LNot)
|
|
{
|
|
contextuallyConvertedExprs_.push_back(expr->getSubExpr()->IgnoreParenImpCasts());
|
|
}
|
|
return true;
|
|
}
|
|
|
|
bool PostTraverseUnaryOperator(UnaryOperator* expr, bool)
|
|
{
|
|
if (expr->getOpcode() == UO_LNot)
|
|
{
|
|
assert(!contextuallyConvertedExprs_.empty());
|
|
contextuallyConvertedExprs_.pop_back();
|
|
}
|
|
return true;
|
|
}
|
|
|
|
bool TraverseUnaryOperator(UnaryOperator* expr)
|
|
{
|
|
auto res = PreTraverseUnaryOperator(expr);
|
|
assert(res);
|
|
res = FilteringRewritePlugin::TraverseUnaryOperator(expr);
|
|
PostTraverseUnaryOperator(expr, res);
|
|
return res;
|
|
}
|
|
|
|
bool PreTraverseBinaryOperator(BinaryOperator* expr)
|
|
{
|
|
auto const op = expr->getOpcode();
|
|
if (op == BO_LAnd || op == BO_LOr)
|
|
{
|
|
contextuallyConvertedExprs_.push_back(expr->getLHS()->IgnoreParenImpCasts());
|
|
contextuallyConvertedExprs_.push_back(expr->getRHS()->IgnoreParenImpCasts());
|
|
}
|
|
return true;
|
|
}
|
|
|
|
bool PostTraverseBinaryOperator(BinaryOperator* expr, bool)
|
|
{
|
|
auto const op = expr->getOpcode();
|
|
if (op == BO_LAnd || op == BO_LOr)
|
|
{
|
|
assert(contextuallyConvertedExprs_.size() >= 2);
|
|
contextuallyConvertedExprs_.pop_back();
|
|
contextuallyConvertedExprs_.pop_back();
|
|
}
|
|
return true;
|
|
}
|
|
|
|
bool TraverseBinaryOperator(BinaryOperator* expr)
|
|
{
|
|
auto res = PreTraverseBinaryOperator(expr);
|
|
assert(res);
|
|
res = FilteringRewritePlugin::TraverseBinaryOperator(expr);
|
|
PostTraverseBinaryOperator(expr, res);
|
|
return res;
|
|
}
|
|
|
|
bool PreTraverseConditionalOperator(ConditionalOperator* expr)
|
|
{
|
|
contextuallyConvertedExprs_.push_back(expr->getCond()->IgnoreParenImpCasts());
|
|
return true;
|
|
}
|
|
|
|
bool PostTraverseConditionalOperator(ConditionalOperator*, bool)
|
|
{
|
|
assert(!contextuallyConvertedExprs_.empty());
|
|
contextuallyConvertedExprs_.pop_back();
|
|
return true;
|
|
}
|
|
|
|
bool TraverseConditionalOperator(ConditionalOperator* expr)
|
|
{
|
|
auto res = PreTraverseConditionalOperator(expr);
|
|
assert(res);
|
|
res = FilteringRewritePlugin::TraverseConditionalOperator(expr);
|
|
PostTraverseConditionalOperator(expr, res);
|
|
return res;
|
|
}
|
|
|
|
bool PreTraverseIfStmt(IfStmt* stmt)
|
|
{
|
|
contextuallyConvertedExprs_.push_back(stmt->getCond()->IgnoreParenImpCasts());
|
|
return true;
|
|
}
|
|
|
|
bool PostTraverseIfStmt(IfStmt*, bool)
|
|
{
|
|
assert(!contextuallyConvertedExprs_.empty());
|
|
contextuallyConvertedExprs_.pop_back();
|
|
return true;
|
|
}
|
|
|
|
bool TraverseIfStmt(IfStmt* stmt)
|
|
{
|
|
auto res = PreTraverseIfStmt(stmt);
|
|
assert(res);
|
|
res = FilteringRewritePlugin::TraverseIfStmt(stmt);
|
|
PostTraverseIfStmt(stmt, res);
|
|
return res;
|
|
}
|
|
|
|
bool PreTraverseWhileStmt(WhileStmt* stmt)
|
|
{
|
|
contextuallyConvertedExprs_.push_back(stmt->getCond()->IgnoreParenImpCasts());
|
|
return true;
|
|
}
|
|
|
|
bool PostTraverseWhileStmt(WhileStmt*, bool)
|
|
{
|
|
assert(!contextuallyConvertedExprs_.empty());
|
|
contextuallyConvertedExprs_.pop_back();
|
|
return true;
|
|
}
|
|
|
|
bool TraverseWhileStmt(WhileStmt* stmt)
|
|
{
|
|
auto res = PreTraverseWhileStmt(stmt);
|
|
assert(res);
|
|
res = FilteringRewritePlugin::TraverseWhileStmt(stmt);
|
|
PostTraverseWhileStmt(stmt, res);
|
|
return res;
|
|
}
|
|
|
|
bool PreTraverseDoStmt(DoStmt* stmt)
|
|
{
|
|
contextuallyConvertedExprs_.push_back(stmt->getCond()->IgnoreParenImpCasts());
|
|
return true;
|
|
}
|
|
|
|
bool PostTraverseDoStmt(DoStmt*, bool)
|
|
{
|
|
assert(!contextuallyConvertedExprs_.empty());
|
|
contextuallyConvertedExprs_.pop_back();
|
|
return true;
|
|
}
|
|
|
|
bool TraverseDoStmt(DoStmt* stmt)
|
|
{
|
|
auto res = PreTraverseDoStmt(stmt);
|
|
assert(res);
|
|
res = FilteringRewritePlugin::TraverseDoStmt(stmt);
|
|
PostTraverseDoStmt(stmt, res);
|
|
return res;
|
|
}
|
|
|
|
bool PreTraverseForStmt(ForStmt* stmt)
|
|
{
|
|
auto const e = stmt->getCond();
|
|
if (e != nullptr)
|
|
{
|
|
contextuallyConvertedExprs_.push_back(e->IgnoreParenImpCasts());
|
|
}
|
|
return true;
|
|
}
|
|
|
|
bool PostTraverseForStmt(ForStmt* stmt, bool)
|
|
{
|
|
if (stmt->getCond() != nullptr)
|
|
{
|
|
assert(!contextuallyConvertedExprs_.empty());
|
|
contextuallyConvertedExprs_.pop_back();
|
|
}
|
|
return true;
|
|
}
|
|
|
|
bool TraverseForStmt(ForStmt* stmt)
|
|
{
|
|
auto res = PreTraverseForStmt(stmt);
|
|
assert(res);
|
|
res = FilteringRewritePlugin::TraverseForStmt(stmt);
|
|
PostTraverseForStmt(stmt, res);
|
|
return res;
|
|
}
|
|
|
|
private:
|
|
bool isContextuallyConverted(Expr const* expr) const
|
|
{
|
|
return std::find(contextuallyConvertedExprs_.begin(), contextuallyConvertedExprs_.end(),
|
|
expr)
|
|
!= contextuallyConvertedExprs_.end();
|
|
}
|
|
|
|
// Get the source range starting at the "."or "->" (plus any preceding non-comment white space):
|
|
SourceRange getCallSourceRange(CXXMemberCallExpr const* expr) const
|
|
{
|
|
if (expr->getImplicitObjectArgument() == nullptr)
|
|
{
|
|
//TODO: Arguably, such a call of a `get` member function from within some member
|
|
// function (so that syntactically no caller is mentioned) should already be handled
|
|
// differently when reporting it (just "drop the get()" does not make sense), instead of
|
|
// being filtered here:
|
|
return {};
|
|
}
|
|
// CXXMemberCallExpr::getExprLoc happens to return the location following the "." or "->":
|
|
auto start = compiler.getSourceManager().getSpellingLoc(expr->getExprLoc());
|
|
if (!start.isValid())
|
|
{
|
|
return {};
|
|
}
|
|
for (;;)
|
|
{
|
|
start = Lexer::GetBeginningOfToken(start.getLocWithOffset(-1),
|
|
compiler.getSourceManager(), compiler.getLangOpts());
|
|
auto const s = StringRef(compiler.getSourceManager().getCharacterData(start),
|
|
Lexer::MeasureTokenLength(start, compiler.getSourceManager(),
|
|
compiler.getLangOpts()));
|
|
if (s.empty() || s.startswith("\\\n"))
|
|
{
|
|
continue;
|
|
}
|
|
if (s != "." && s != "->")
|
|
{
|
|
return {};
|
|
}
|
|
break;
|
|
}
|
|
for (;;)
|
|
{
|
|
auto start1 = Lexer::GetBeginningOfToken(
|
|
start.getLocWithOffset(-1), compiler.getSourceManager(), compiler.getLangOpts());
|
|
auto const s = StringRef(compiler.getSourceManager().getCharacterData(start1),
|
|
Lexer::MeasureTokenLength(start1, compiler.getSourceManager(),
|
|
compiler.getLangOpts()));
|
|
if (!(s.empty() || s.startswith("\\\n")))
|
|
{
|
|
break;
|
|
}
|
|
start = start1;
|
|
}
|
|
return SourceRange(start, compiler.getSourceManager().getSpellingLoc(expr->getEndLoc()));
|
|
}
|
|
|
|
//TODO: There are some more places where an expression is contextually converted to bool, but
|
|
// those are probably not relevant for our needs here.
|
|
std::deque<Expr const*> contextuallyConvertedExprs_;
|
|
};
|
|
|
|
bool SimplifyPointerToBool::VisitImplicitCastExpr(ImplicitCastExpr const* castExpr)
|
|
{
|
|
if (ignoreLocation(castExpr))
|
|
return true;
|
|
if (castExpr->getCastKind() != CK_PointerToBoolean)
|
|
return true;
|
|
auto memberCallExpr
|
|
= dyn_cast<CXXMemberCallExpr>(castExpr->getSubExpr()->IgnoreParenImpCasts());
|
|
if (!memberCallExpr)
|
|
return true;
|
|
auto methodDecl = memberCallExpr->getMethodDecl();
|
|
if (!methodDecl || !methodDecl->getIdentifier() || methodDecl->getName() != "get")
|
|
return true;
|
|
// castExpr->dump();
|
|
// methodDecl->getParent()->getTypeForDecl()->dump();
|
|
if (!loplugin::isSmartPointerType(memberCallExpr->getImplicitObjectArgument()))
|
|
return true;
|
|
// if (isa<CXXOperatorCallExpr>(callExpr))
|
|
// return true;
|
|
// const FunctionDecl* functionDecl;
|
|
// if (isa<CXXMemberCallExpr>(callExpr))
|
|
// {
|
|
// functionDecl = dyn_cast<CXXMemberCallExpr>(callExpr)->getMethodDecl();
|
|
// }
|
|
// else
|
|
// {
|
|
// functionDecl = callExpr->getDirectCallee();
|
|
// }
|
|
// if (!functionDecl)
|
|
// return true;
|
|
//
|
|
// unsigned len = std::min(callExpr->getNumArgs(), functionDecl->getNumParams());
|
|
// for (unsigned i = 0; i < len; ++i)
|
|
// {
|
|
// auto param = functionDecl->getParamDecl(i);
|
|
// auto paramTC = loplugin::TypeCheck(param->getType());
|
|
// if (!paramTC.AnyBoolean())
|
|
// continue;
|
|
// auto arg = callExpr->getArg(i)->IgnoreImpCasts();
|
|
// auto argTC = loplugin::TypeCheck(arg->getType());
|
|
// if (argTC.AnyBoolean())
|
|
// continue;
|
|
// // sal_Bool is sometimes disguised
|
|
// if (isa<SubstTemplateTypeParmType>(arg->getType()))
|
|
// if (arg->getType()->getUnqualifiedDesugaredType()->isSpecificBuiltinType(
|
|
// clang::BuiltinType::UChar))
|
|
// continue;
|
|
// if (arg->getType()->isDependentType())
|
|
// continue;
|
|
// if (arg->getType()->isIntegerType())
|
|
// {
|
|
// auto ret = getCallValue(arg);
|
|
// if (ret.hasValue() && (ret.getValue() == 1 || ret.getValue() == 0))
|
|
// continue;
|
|
// // something like: priv->m_nLOKFeatures & LOK_FEATURE_DOCUMENT_PASSWORD
|
|
// if (isa<BinaryOperator>(arg->IgnoreParenImpCasts()))
|
|
// continue;
|
|
// // something like: pbEmbolden ? FcTrue : FcFalse
|
|
// if (isa<ConditionalOperator>(arg->IgnoreParenImpCasts()))
|
|
// continue;
|
|
// }
|
|
if (isContextuallyConverted(memberCallExpr))
|
|
{
|
|
if (rewriter)
|
|
{
|
|
auto const range = getCallSourceRange(memberCallExpr);
|
|
if (range.isValid() && removeText(range))
|
|
{
|
|
return true;
|
|
}
|
|
}
|
|
report(DiagnosticsEngine::Warning, "simplify, drop the get()", memberCallExpr->getExprLoc())
|
|
<< memberCallExpr->getSourceRange();
|
|
}
|
|
else if (isa<ParenExpr>(castExpr->getSubExpr()))
|
|
{
|
|
if (rewriter)
|
|
{
|
|
auto const loc
|
|
= compiler.getSourceManager().getSpellingLoc(memberCallExpr->getBeginLoc());
|
|
auto const range = getCallSourceRange(memberCallExpr);
|
|
if (loc.isValid() && range.isValid() && insertText(loc, "bool") && removeText(range))
|
|
{
|
|
//TODO: atomically only change both or neither
|
|
return true;
|
|
}
|
|
}
|
|
report(DiagnosticsEngine::Warning,
|
|
"simplify, drop the get() and turn the surrounding parentheses into a functional "
|
|
"cast to bool",
|
|
memberCallExpr->getExprLoc())
|
|
<< memberCallExpr->getSourceRange();
|
|
report(DiagnosticsEngine::Note, "surrounding parentheses here",
|
|
castExpr->getSubExpr()->getExprLoc())
|
|
<< castExpr->getSubExpr()->getSourceRange();
|
|
}
|
|
else
|
|
{
|
|
if (rewriter)
|
|
{
|
|
auto const loc
|
|
= compiler.getSourceManager().getSpellingLoc(memberCallExpr->getBeginLoc());
|
|
auto const range = getCallSourceRange(memberCallExpr);
|
|
if (loc.isValid() && range.isValid() && insertText(loc, "bool(")
|
|
&& replaceText(range, ")"))
|
|
{
|
|
//TODO: atomically only change both or neither
|
|
return true;
|
|
}
|
|
}
|
|
report(DiagnosticsEngine::Warning,
|
|
"simplify, drop the get() and wrap the expression in a functional cast to bool",
|
|
memberCallExpr->getExprLoc())
|
|
<< memberCallExpr->getSourceRange();
|
|
}
|
|
// report(DiagnosticsEngine::Note, "method here", param->getLocation())
|
|
// << param->getSourceRange();
|
|
return true;
|
|
}
|
|
|
|
bool SimplifyPointerToBool::VisitBinaryOperator(BinaryOperator const* binOp)
|
|
{
|
|
if (ignoreLocation(binOp))
|
|
return true;
|
|
auto opCode = binOp->getOpcode();
|
|
if (opCode != BO_EQ && opCode != BO_NE)
|
|
return true;
|
|
const Expr* possibleMemberCall = nullptr;
|
|
if (isa<CXXNullPtrLiteralExpr>(binOp->getLHS()->IgnoreParenImpCasts()))
|
|
possibleMemberCall = binOp->getRHS();
|
|
else if (isa<CXXNullPtrLiteralExpr>(binOp->getRHS()->IgnoreParenImpCasts()))
|
|
possibleMemberCall = binOp->getLHS();
|
|
else
|
|
return true;
|
|
auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(possibleMemberCall);
|
|
if (!memberCallExpr)
|
|
return true;
|
|
auto methodDecl = memberCallExpr->getMethodDecl();
|
|
if (!methodDecl || !methodDecl->getIdentifier() || methodDecl->getName() != "get")
|
|
return true;
|
|
if (!loplugin::isSmartPointerType(memberCallExpr->getImplicitObjectArgument()))
|
|
return true;
|
|
report(DiagnosticsEngine::Warning,
|
|
std::string("simplify, convert to ") + (opCode == BO_EQ ? "'!x'" : "'x'"),
|
|
binOp->getExprLoc())
|
|
<< binOp->getSourceRange();
|
|
return true;
|
|
}
|
|
|
|
loplugin::Plugin::Registration<SimplifyPointerToBool> simplifypointertobool("simplifypointertobool",
|
|
true);
|
|
|
|
} // namespace
|
|
|
|
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
|