2017-07-05 08:32:57 +02:00
|
|
|
/* -*- 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 <cassert>
|
|
|
|
#include <string>
|
|
|
|
#include <iostream>
|
|
|
|
#include <fstream>
|
|
|
|
#include <set>
|
|
|
|
|
|
|
|
#include <clang/AST/CXXInheritance.h>
|
|
|
|
#include "compat.hxx"
|
|
|
|
#include "plugin.hxx"
|
|
|
|
|
|
|
|
/**
|
|
|
|
look for unnecessary parentheses
|
|
|
|
*/
|
|
|
|
|
|
|
|
namespace {
|
|
|
|
|
2017-11-23 08:13:06 +01:00
|
|
|
// Like clang::Stmt::IgnoreImplicit (lib/AST/Stmt.cpp), but also ignoring CXXConstructExpr and
|
|
|
|
// looking through implicit UserDefinedConversion's member function call:
|
2017-10-26 10:27:48 +02:00
|
|
|
Expr const * ignoreAllImplicit(Expr const * expr) {
|
|
|
|
if (auto const e = dyn_cast<ExprWithCleanups>(expr)) {
|
|
|
|
expr = e->getSubExpr();
|
|
|
|
}
|
|
|
|
if (auto const e = dyn_cast<CXXConstructExpr>(expr)) {
|
|
|
|
if (e->getNumArgs() == 1) {
|
|
|
|
expr = e->getArg(0);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
if (auto const e = dyn_cast<MaterializeTemporaryExpr>(expr)) {
|
|
|
|
expr = e->GetTemporaryExpr();
|
|
|
|
}
|
|
|
|
if (auto const e = dyn_cast<CXXBindTemporaryExpr>(expr)) {
|
|
|
|
expr = e->getSubExpr();
|
|
|
|
}
|
2017-11-23 08:13:06 +01:00
|
|
|
while (auto const e = dyn_cast<ImplicitCastExpr>(expr)) {
|
|
|
|
expr = e->getSubExpr();
|
|
|
|
if (e->getCastKind() == CK_UserDefinedConversion) {
|
|
|
|
auto const ce = cast<CXXMemberCallExpr>(expr);
|
|
|
|
assert(ce->getNumArgs() == 0);
|
|
|
|
expr = ce->getImplicitObjectArgument();
|
|
|
|
}
|
|
|
|
}
|
|
|
|
return expr;
|
2017-10-26 10:27:48 +02:00
|
|
|
}
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
class UnnecessaryParen:
|
|
|
|
public RecursiveASTVisitor<UnnecessaryParen>, public loplugin::Plugin
|
|
|
|
{
|
|
|
|
public:
|
2017-11-07 11:50:47 +01:00
|
|
|
explicit UnnecessaryParen(loplugin::InstantiationData const & data):
|
|
|
|
Plugin(data) {}
|
2017-07-05 08:32:57 +02:00
|
|
|
|
|
|
|
virtual void run() override
|
|
|
|
{
|
2017-07-05 14:30:39 +02:00
|
|
|
StringRef fn( compiler.getSourceManager().getFileEntryForID(
|
|
|
|
compiler.getSourceManager().getMainFileID())->getName() );
|
2017-07-07 08:42:54 +02:00
|
|
|
// fixing this, makes the source in the .y files look horrible
|
2017-07-13 11:26:39 +02:00
|
|
|
if (loplugin::isSamePathname(fn, WORKDIR "/YaccTarget/unoidl/source/sourceprovider-parser.cxx"))
|
2017-07-05 14:30:39 +02:00
|
|
|
return;
|
2017-07-13 11:26:39 +02:00
|
|
|
if (loplugin::isSamePathname(fn, WORKDIR "/YaccTarget/idlc/source/parser.cxx"))
|
2017-07-05 14:30:39 +02:00
|
|
|
return;
|
2017-07-05 16:30:06 +02:00
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
|
|
|
|
}
|
|
|
|
|
|
|
|
bool VisitParenExpr(const ParenExpr *);
|
|
|
|
bool VisitIfStmt(const IfStmt *);
|
2017-07-05 16:30:06 +02:00
|
|
|
bool VisitDoStmt(const DoStmt *);
|
|
|
|
bool VisitWhileStmt(const WhileStmt *);
|
|
|
|
bool VisitSwitchStmt(const SwitchStmt *);
|
2017-09-05 08:29:58 +02:00
|
|
|
bool VisitCaseStmt(const CaseStmt *);
|
2017-08-17 17:21:53 +02:00
|
|
|
bool VisitReturnStmt(const ReturnStmt* );
|
2017-07-07 08:42:54 +02:00
|
|
|
bool VisitCallExpr(const CallExpr *);
|
2017-11-20 10:26:01 +02:00
|
|
|
bool VisitVarDecl(const VarDecl *);
|
|
|
|
bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *);
|
2017-07-06 14:49:15 +02:00
|
|
|
bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *);
|
|
|
|
bool TraverseCaseStmt(CaseStmt *);
|
2017-07-05 16:30:06 +02:00
|
|
|
private:
|
2017-11-20 10:26:01 +02:00
|
|
|
void VisitSomeStmt(Stmt const * stmt, const Expr* cond, StringRef stmtName);
|
2017-10-26 10:27:48 +02:00
|
|
|
Expr const * insideSizeof = nullptr;
|
|
|
|
Expr const * insideCaseStmt = nullptr;
|
2017-07-05 08:32:57 +02:00
|
|
|
};
|
|
|
|
|
2017-07-16 20:35:11 +02:00
|
|
|
bool UnnecessaryParen::TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr * expr)
|
2017-07-06 14:49:15 +02:00
|
|
|
{
|
2017-07-16 20:35:11 +02:00
|
|
|
auto old = insideSizeof;
|
|
|
|
if (expr->getKind() == UETT_SizeOf && !expr->isArgumentType()) {
|
2017-10-26 10:27:48 +02:00
|
|
|
insideSizeof = ignoreAllImplicit(expr->getArgumentExpr());
|
2017-07-16 20:35:11 +02:00
|
|
|
}
|
|
|
|
bool ret = RecursiveASTVisitor::TraverseUnaryExprOrTypeTraitExpr(expr);
|
|
|
|
insideSizeof = old;
|
|
|
|
return ret;
|
2017-07-06 14:49:15 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::TraverseCaseStmt(CaseStmt * caseStmt)
|
|
|
|
{
|
|
|
|
auto old = insideCaseStmt;
|
2017-10-26 10:27:48 +02:00
|
|
|
insideCaseStmt = ignoreAllImplicit(caseStmt->getLHS());
|
2017-07-06 14:49:15 +02:00
|
|
|
bool ret = RecursiveASTVisitor::TraverseCaseStmt(caseStmt);
|
|
|
|
insideCaseStmt = old;
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr)
|
|
|
|
{
|
|
|
|
if (ignoreLocation(parenExpr))
|
|
|
|
return true;
|
|
|
|
if (parenExpr->getLocStart().isMacroID())
|
|
|
|
return true;
|
2017-07-16 20:35:11 +02:00
|
|
|
if (insideSizeof && parenExpr == insideSizeof)
|
|
|
|
return true;
|
2017-07-06 14:49:15 +02:00
|
|
|
if (insideCaseStmt && parenExpr == insideCaseStmt)
|
|
|
|
return true;
|
2017-07-05 08:32:57 +02:00
|
|
|
|
2017-10-26 10:27:48 +02:00
|
|
|
auto subExpr = ignoreAllImplicit(parenExpr->getSubExpr());
|
2017-09-04 08:53:38 +02:00
|
|
|
|
|
|
|
if (auto subParenExpr = dyn_cast<ParenExpr>(subExpr))
|
|
|
|
{
|
2017-07-05 08:32:57 +02:00
|
|
|
if (subParenExpr->getLocStart().isMacroID())
|
|
|
|
return true;
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "parentheses around parentheses",
|
|
|
|
parenExpr->getLocStart())
|
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
}
|
2017-07-06 14:49:15 +02:00
|
|
|
|
2017-09-04 08:53:38 +02:00
|
|
|
if (auto declRefExpr = dyn_cast<DeclRefExpr>(subExpr)) {
|
2017-11-22 09:54:10 +01:00
|
|
|
// hack for libxml2's BAD_CAST object-like macro (expanding to "(xmlChar *)"), which is
|
|
|
|
// typically used as if it were a function-like macro, e.g., as "BAD_CAST(pName)" in
|
|
|
|
// SwNode::dumpAsXml (sw/source/core/docnode/node.cxx)
|
|
|
|
if (!declRefExpr->getLocStart().isMacroID()) {
|
|
|
|
SourceManager& SM = compiler.getSourceManager();
|
|
|
|
const char *p1 = SM.getCharacterData( declRefExpr->getLocStart().getLocWithOffset(-10) );
|
|
|
|
const char *p2 = SM.getCharacterData( declRefExpr->getLocStart() );
|
|
|
|
if ( std::string(p1, p2 - p1).find("BAD_CAST") != std::string::npos )
|
|
|
|
return true;
|
|
|
|
}
|
2017-07-06 14:49:15 +02:00
|
|
|
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "unnecessary parentheses around identifier",
|
|
|
|
parenExpr->getLocStart())
|
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
}
|
2017-09-04 08:53:38 +02:00
|
|
|
|
|
|
|
|
2017-11-22 09:54:10 +01:00
|
|
|
if (isa<CXXNamedCastExpr>(subExpr)) {
|
2017-09-04 08:53:38 +02:00
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "unnecessary parentheses around cast",
|
|
|
|
parenExpr->getLocStart())
|
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
}
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::VisitIfStmt(const IfStmt* ifStmt)
|
|
|
|
{
|
2017-07-05 16:30:06 +02:00
|
|
|
VisitSomeStmt(ifStmt, ifStmt->getCond(), "if");
|
|
|
|
return true;
|
|
|
|
}
|
2017-07-05 08:32:57 +02:00
|
|
|
|
2017-07-05 16:30:06 +02:00
|
|
|
bool UnnecessaryParen::VisitDoStmt(const DoStmt* doStmt)
|
|
|
|
{
|
|
|
|
VisitSomeStmt(doStmt, doStmt->getCond(), "do");
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::VisitWhileStmt(const WhileStmt* whileStmt)
|
|
|
|
{
|
|
|
|
VisitSomeStmt(whileStmt, whileStmt->getCond(), "while");
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::VisitSwitchStmt(const SwitchStmt* switchStmt)
|
|
|
|
{
|
|
|
|
VisitSomeStmt(switchStmt, switchStmt->getCond(), "switch");
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2017-09-05 08:29:58 +02:00
|
|
|
bool UnnecessaryParen::VisitCaseStmt(const CaseStmt* caseStmt)
|
|
|
|
{
|
|
|
|
VisitSomeStmt(caseStmt, caseStmt->getLHS(), "case");
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2017-08-17 17:21:53 +02:00
|
|
|
bool UnnecessaryParen::VisitReturnStmt(const ReturnStmt* returnStmt)
|
|
|
|
{
|
|
|
|
if (ignoreLocation(returnStmt))
|
|
|
|
return true;
|
|
|
|
|
|
|
|
if (!returnStmt->getRetValue())
|
|
|
|
return true;
|
2017-10-26 10:27:48 +02:00
|
|
|
auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(returnStmt->getRetValue()));
|
2017-08-17 17:21:53 +02:00
|
|
|
if (!parenExpr)
|
|
|
|
return true;
|
|
|
|
if (parenExpr->getLocStart().isMacroID())
|
|
|
|
return true;
|
|
|
|
// assignments need extra parentheses or they generate a compiler warning
|
|
|
|
auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr());
|
|
|
|
if (binaryOp && binaryOp->getOpcode() == BO_Assign)
|
|
|
|
return true;
|
|
|
|
|
|
|
|
// only non-operator-calls for now
|
|
|
|
auto subExpr = parenExpr->getSubExpr();
|
|
|
|
if (isa<CallExpr>(subExpr) && !isa<CXXOperatorCallExpr>(subExpr))
|
|
|
|
{
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "parentheses immediately inside return statement",
|
|
|
|
parenExpr->getLocStart())
|
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
}
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2017-11-20 10:26:01 +02:00
|
|
|
void UnnecessaryParen::VisitSomeStmt(const Stmt * stmt, const Expr* cond, StringRef stmtName)
|
2017-07-05 16:30:06 +02:00
|
|
|
{
|
2017-11-20 10:26:01 +02:00
|
|
|
if (ignoreLocation(stmt))
|
2017-07-05 16:30:06 +02:00
|
|
|
return;
|
|
|
|
|
2017-10-26 10:27:48 +02:00
|
|
|
auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(cond));
|
2017-07-05 16:30:06 +02:00
|
|
|
if (parenExpr) {
|
2017-07-05 08:32:57 +02:00
|
|
|
if (parenExpr->getLocStart().isMacroID())
|
2017-07-05 16:30:06 +02:00
|
|
|
return;
|
Enable -Wunreachable-code
...motivated by <https://gerrit.libreoffice.org/#/c/41565/2> adding dead code
at the end of a switch statement, after the last case's "break".
-Wunreachable-code appears to work well on Clang, while it appears to have no
effect on GCC.
Most of the affected places are apparently temporary/TODO/FIXME cases of
disabling code via "if (false)", which can be written with an extra set of
parentheses as "if ((false))" to silence -Wunreachable-code on Clang (which thus
needed loplugin:unnecessaryparen to be adapted accordingly). In some cases,
the controlling expression was more complex than just "false" and needed to be
rewritten by taking it out of the if statement to silence Clang.
One noteworthy case where the nature of the disabled code wasn't immediately
apparent:
Sep 12 16:59:58 <sberg> quikee, is that "if (false)" in
ScExponentialSmoothingDialog::ApplyOutput
(sc/source/ui/StatisticsDialogs/ExponentialSmoothingDialog.cxx) some work-in-
progress or dead code?
Sep 12 17:02:03 <quikee> sberg: WIP, but you can remove it
Sep 12 17:04:47 <sberg> quikee, I'll wrap the false in an extra set of
parentheses for now, to silence -Wunreachable-code (I wouldn't want to
remove it, as I have no idea whether I should then also remove the "Initial
value" comment preceding it)
Sep 12 17:07:29 <quikee> sberg: both are different ways to calculate the
"intital value"... so no
Another case where the nature of the dead code, following while (true) loops
without breaks, is unclear is sd/source/ui/remotecontrol/BluetoothServer.cxx,
where I added TODO markers to the workarounds that silence the warnings for now.
basic/source/sbx/sbxvalue.cxx had a variable of type double, of automatic
storage duration, and without an initalizer at the top of a switch statement.
Clang warning about it is arguably a false positive.
Apart from that, this didn't find any cases of genuinely dead code in the
existing code base.
Change-Id: Ib00b822c8efec94278c048783d5997b8ba86a94c
Reviewed-on: https://gerrit.libreoffice.org/42217
Tested-by: Stephan Bergmann <sbergman@redhat.com>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2017-09-12 18:24:46 +02:00
|
|
|
// Used to silence -Wunreachable-code:
|
|
|
|
if (isa<CXXBoolLiteralExpr>(parenExpr->getSubExpr())
|
|
|
|
&& stmtName == "if")
|
|
|
|
{
|
|
|
|
return;
|
|
|
|
}
|
2017-07-05 08:32:57 +02:00
|
|
|
// assignments need extra parentheses or they generate a compiler warning
|
|
|
|
auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr());
|
|
|
|
if (binaryOp && binaryOp->getOpcode() == BO_Assign)
|
2017-07-05 16:30:06 +02:00
|
|
|
return;
|
2017-11-23 08:13:06 +01:00
|
|
|
if (auto const opCall = dyn_cast<CXXOperatorCallExpr>(parenExpr->getSubExpr())) {
|
|
|
|
if (opCall->getOperator() == OO_Equal) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
}
|
2017-07-05 08:32:57 +02:00
|
|
|
report(
|
2017-07-05 16:30:06 +02:00
|
|
|
DiagnosticsEngine::Warning, "parentheses immediately inside %0 statement",
|
|
|
|
parenExpr->getLocStart())
|
|
|
|
<< stmtName
|
|
|
|
<< parenExpr->getSourceRange();
|
2017-07-05 08:32:57 +02:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2017-07-07 08:42:54 +02:00
|
|
|
bool UnnecessaryParen::VisitCallExpr(const CallExpr* callExpr)
|
|
|
|
{
|
|
|
|
if (ignoreLocation(callExpr))
|
|
|
|
return true;
|
|
|
|
if (callExpr->getNumArgs() != 1 || isa<CXXOperatorCallExpr>(callExpr))
|
|
|
|
return true;
|
|
|
|
|
2017-10-26 10:27:48 +02:00
|
|
|
auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(callExpr->getArg(0)));
|
2017-11-20 10:26:01 +02:00
|
|
|
if (!parenExpr)
|
|
|
|
return true;
|
|
|
|
if (parenExpr->getLocStart().isMacroID())
|
|
|
|
return true;
|
|
|
|
// assignments need extra parentheses or they generate a compiler warning
|
|
|
|
auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr());
|
|
|
|
if (binaryOp && binaryOp->getOpcode() == BO_Assign)
|
|
|
|
return true;
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "parentheses immediately inside single-arg call",
|
|
|
|
parenExpr->getLocStart())
|
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr* callExpr)
|
|
|
|
{
|
|
|
|
if (ignoreLocation(callExpr))
|
|
|
|
return true;
|
|
|
|
if (callExpr->getNumArgs() != 2)
|
|
|
|
return true;
|
|
|
|
|
|
|
|
// Same logic as CXXOperatorCallExpr::isAssignmentOp(), which our supported clang
|
|
|
|
// doesn't have yet.
|
|
|
|
auto Opc = callExpr->getOperator();
|
|
|
|
if (Opc != OO_Equal && Opc != OO_StarEqual &&
|
|
|
|
Opc != OO_SlashEqual && Opc != OO_PercentEqual &&
|
|
|
|
Opc != OO_PlusEqual && Opc != OO_MinusEqual &&
|
|
|
|
Opc != OO_LessLessEqual && Opc != OO_GreaterGreaterEqual &&
|
|
|
|
Opc != OO_AmpEqual && Opc != OO_CaretEqual &&
|
|
|
|
Opc != OO_PipeEqual)
|
|
|
|
return true;
|
|
|
|
|
|
|
|
auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(callExpr->getArg(1)));
|
|
|
|
if (!parenExpr)
|
|
|
|
return true;
|
|
|
|
if (parenExpr->getLocStart().isMacroID())
|
|
|
|
return true;
|
|
|
|
// Sometimes parentheses make the RHS of an assignment easier to read by
|
|
|
|
// visually disambiguating the = from a call to ==
|
|
|
|
auto sub = parenExpr->getSubExpr();
|
|
|
|
if (isa<BinaryOperator>(sub)
|
|
|
|
|| isa<CXXOperatorCallExpr>(sub)
|
|
|
|
|| isa<ConditionalOperator>(sub))
|
|
|
|
return true;
|
|
|
|
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "parentheses immediately inside assignment",
|
|
|
|
parenExpr->getLocStart())
|
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::VisitVarDecl(const VarDecl* varDecl)
|
|
|
|
{
|
|
|
|
if (ignoreLocation(varDecl))
|
|
|
|
return true;
|
|
|
|
if (!varDecl->getInit())
|
|
|
|
return true;
|
|
|
|
|
|
|
|
auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(varDecl->getInit()));
|
|
|
|
if (!parenExpr)
|
|
|
|
return true;
|
|
|
|
if (parenExpr->getLocStart().isMacroID())
|
|
|
|
return true;
|
|
|
|
auto sub = parenExpr->getSubExpr();
|
|
|
|
if (isa<BinaryOperator>(sub)
|
|
|
|
|| isa<CXXOperatorCallExpr>(sub)
|
|
|
|
|| isa<ConditionalOperator>(sub)
|
|
|
|
// these two are for "parentheses were disambiguated as a function declaration [-Werror,-Wvexing-parse]"
|
|
|
|
|| isa<CXXBindTemporaryExpr>(sub)
|
|
|
|
|| isa<CXXFunctionalCastExpr>(sub))
|
|
|
|
return true;
|
|
|
|
|
|
|
|
//varDecl->dump();
|
|
|
|
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "parentheses immediately inside vardecl statement",
|
|
|
|
parenExpr->getLocStart())
|
|
|
|
<< parenExpr->getSourceRange();
|
2017-07-07 08:42:54 +02:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
loplugin::Plugin::Registration< UnnecessaryParen > X("unnecessaryparen", true);
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
|