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/.
|
|
|
|
*/
|
|
|
|
|
2019-03-07 14:31:17 +01:00
|
|
|
#ifndef LO_CLANG_SHARED_PLUGINS
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
#include <cassert>
|
|
|
|
#include <string>
|
|
|
|
#include <iostream>
|
|
|
|
#include <fstream>
|
|
|
|
#include <set>
|
2017-11-27 09:46:06 +01:00
|
|
|
#include <unordered_set>
|
2017-07-05 08:32:57 +02:00
|
|
|
|
|
|
|
#include <clang/AST/CXXInheritance.h>
|
2018-11-12 12:01:53 +01:00
|
|
|
|
|
|
|
#include "config_clang.h"
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
#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) {
|
2017-12-29 12:23:33 +02:00
|
|
|
while (true)
|
|
|
|
{
|
|
|
|
auto oldExpr = expr;
|
|
|
|
if (auto const e = dyn_cast<ExprWithCleanups>(expr)) {
|
|
|
|
expr = e->getSubExpr();
|
2017-10-26 10:27:48 +02:00
|
|
|
}
|
2017-12-29 12:23:33 +02:00
|
|
|
else if (auto const e = dyn_cast<CXXConstructExpr>(expr)) {
|
|
|
|
if (e->getNumArgs() == 1) {
|
|
|
|
expr = e->getArg(0);
|
|
|
|
}
|
2017-11-23 08:13:06 +01:00
|
|
|
}
|
2017-12-29 12:23:33 +02:00
|
|
|
else if (auto const e = dyn_cast<MaterializeTemporaryExpr>(expr)) {
|
2019-11-25 13:04:02 +01:00
|
|
|
expr = compat::getSubExpr(e);
|
2017-12-29 12:23:33 +02:00
|
|
|
}
|
|
|
|
else if (auto const e = dyn_cast<CXXBindTemporaryExpr>(expr)) {
|
|
|
|
expr = e->getSubExpr();
|
|
|
|
}
|
|
|
|
else if (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();
|
|
|
|
}
|
|
|
|
}
|
2018-11-12 12:01:53 +01:00
|
|
|
#if CLANG_VERSION >= 80000
|
|
|
|
else if (auto const e = dyn_cast<ConstantExpr>(expr)) {
|
|
|
|
expr = e->getSubExpr();
|
|
|
|
}
|
|
|
|
#endif
|
2017-12-29 12:23:33 +02:00
|
|
|
if (expr == oldExpr)
|
|
|
|
return expr;
|
2017-11-23 08:13:06 +01:00
|
|
|
}
|
|
|
|
return expr;
|
2017-10-26 10:27:48 +02:00
|
|
|
}
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
class UnnecessaryParen:
|
2018-08-23 14:35:15 +02:00
|
|
|
public loplugin::FilteringRewritePlugin<UnnecessaryParen>
|
2017-07-05 08:32:57 +02:00
|
|
|
{
|
|
|
|
public:
|
2017-11-07 11:50:47 +01:00
|
|
|
explicit UnnecessaryParen(loplugin::InstantiationData const & data):
|
2018-08-23 14:35:15 +02:00
|
|
|
FilteringRewritePlugin(data) {}
|
2017-07-05 08:32:57 +02:00
|
|
|
|
2019-03-07 14:31:17 +01:00
|
|
|
virtual bool preRun() override
|
2017-07-05 08:32:57 +02:00
|
|
|
{
|
2018-05-29 11:28:07 +02:00
|
|
|
StringRef fn(handler.getMainFileName());
|
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"))
|
2019-03-07 14:31:17 +01:00
|
|
|
return false;
|
2017-07-13 11:26:39 +02:00
|
|
|
if (loplugin::isSamePathname(fn, WORKDIR "/YaccTarget/idlc/source/parser.cxx"))
|
2019-03-07 14:31:17 +01:00
|
|
|
return false;
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
virtual void run() override
|
|
|
|
{
|
|
|
|
if( preRun())
|
|
|
|
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
|
2017-07-05 08:32:57 +02:00
|
|
|
}
|
|
|
|
|
|
|
|
bool VisitParenExpr(const ParenExpr *);
|
|
|
|
bool VisitIfStmt(const IfStmt *);
|
2017-07-05 16:30:06 +02:00
|
|
|
bool VisitDoStmt(const DoStmt *);
|
|
|
|
bool VisitWhileStmt(const WhileStmt *);
|
2017-11-29 18:21:29 +01:00
|
|
|
bool VisitForStmt(ForStmt const * stmt);
|
2017-07-05 16:30:06 +02:00
|
|
|
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-11-27 09:46:06 +01:00
|
|
|
bool VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr const *);
|
|
|
|
bool VisitConditionalOperator(ConditionalOperator const * expr);
|
2017-11-29 18:21:29 +01:00
|
|
|
bool VisitBinaryConditionalOperator(BinaryConditionalOperator const * expr);
|
2017-11-24 13:08:01 +02:00
|
|
|
bool VisitMemberExpr(const MemberExpr *f);
|
2018-01-17 09:01:09 +02:00
|
|
|
bool VisitCXXDeleteExpr(const CXXDeleteExpr *);
|
2019-11-29 17:42:03 +01:00
|
|
|
|
|
|
|
bool VisitImplicitCastExpr(ImplicitCastExpr const * expr) {
|
|
|
|
if (ignoreLocation(expr)) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
if (expr->getCastKind() != CK_UserDefinedConversion) {
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
// Filter out a MemberExpr (resp. a ParenExpr sub-expr, if any, as would be found by
|
|
|
|
// VisitMemberExpr) that is part of a CXXMemberCallExpr which in turn is part of an
|
|
|
|
// ImplicitCastExpr, so that VisitMemberExpr doesn't erroneously pick it up (and note that
|
|
|
|
// CXXMemberCallExpr's getImplicitObjectArgument() skips past the underlying MemberExpr):
|
|
|
|
if (auto const e1 = dyn_cast<CXXMemberCallExpr>(expr->getSubExpr())) {
|
2019-11-30 11:44:26 +01:00
|
|
|
if (auto const e2 = dyn_cast<ParenExpr>(
|
|
|
|
e1->getImplicitObjectArgument()->IgnoreImpCasts()))
|
|
|
|
{
|
2019-11-29 17:42:03 +01:00
|
|
|
handled_.insert(e2);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
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-11-27 09:46:06 +01:00
|
|
|
|
2017-11-29 18:21:29 +01:00
|
|
|
void handleUnreachableCodeConditionParens(Expr const * expr);
|
|
|
|
|
2017-11-27 09:46:06 +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):
|
|
|
|
bool isPrecededBy_BAD_CAST(Expr const * expr);
|
|
|
|
|
Enable loplugin:cstylecast for some more cases
...mostly of C-style casts among arithmetic types, and automatically rewrite
those into either static_cast or a functional cast (which should have identical
semantics, but where the latter probably looks better for simple cases like
casting a literal to a specific type, as in "sal_Int32(0)" vs.
"static_cast<sal_Int32>(0)").
The main benefit of reducing the amount of C-style casts across the code base
further is so that other plugins (that have not been taught about the complex
semantics of C-style cast) can pick those up (cf. the various recent
"loplugin:redundantcast" commits, which address those findings after this
improved loplugin:cstylecast has been run). Also, I found some places where
a C-style cast has probably been applied only to the first part of a larger
expression in error (because it's easy to forget parentheses in cases like
"(sal_uInt16)VOPT_CLIPMARKS+1"); I'll follow up on those individually.
The improved loplugin:cstylecast is careful to output either "(performs:
static_cast)" or "(performs: functional cast)", so that
compilerplugins/clang/test/cstylecast.cxx can check that the plugin would
automatically rewrite to one or the other form.
To allow fully-automatic rewriting, this also required loplugin:unnecessaryparen
to become a rewriting plugin, at least for the parens-around-cast case (where
"((foo)bar)" first gets rewritten to "(static_cast<foo>(bar))", then to
"static_cast<foo>(bar)". Rewriting could probably be added to other cases of
loplugin:unnecessaryparen in the future, too.
(The final version of this patch would even have been able to cope with
361dd2576a09fbda83f3ce9a26ecb590c38f74e3 "Replace some C-style casts in ugly
macros with static_cast", so that manual change would not have been necessary
after all.)
Change-Id: Icd7e319cc38eb58262fcbf7643d177ac9ea0220a
Reviewed-on: https://gerrit.libreoffice.org/47798
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2018-01-10 15:55:22 +01:00
|
|
|
bool badCombination(SourceLocation loc, int prevOffset, int nextOffset);
|
|
|
|
|
|
|
|
bool removeParens(ParenExpr const * expr);
|
|
|
|
|
2017-11-27 09:46:06 +01:00
|
|
|
std::unordered_set<ParenExpr const *> handled_;
|
2017-07-05 08:32:57 +02:00
|
|
|
};
|
|
|
|
|
2017-11-27 09:46:06 +01:00
|
|
|
bool UnnecessaryParen::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr const * expr)
|
2017-07-06 14:49:15 +02:00
|
|
|
{
|
2017-07-16 20:35:11 +02:00
|
|
|
if (expr->getKind() == UETT_SizeOf && !expr->isArgumentType()) {
|
2017-11-27 09:46:06 +01:00
|
|
|
if (auto const e = dyn_cast<ParenExpr>(ignoreAllImplicit(expr->getArgumentExpr()))) {
|
|
|
|
handled_.insert(e);
|
|
|
|
}
|
2017-07-16 20:35:11 +02:00
|
|
|
}
|
2017-11-27 09:46:06 +01:00
|
|
|
return true;
|
2017-07-06 14:49:15 +02:00
|
|
|
}
|
|
|
|
|
2017-11-27 09:46:06 +01:00
|
|
|
bool UnnecessaryParen::VisitConditionalOperator(ConditionalOperator const * expr) {
|
2017-11-29 18:21:29 +01:00
|
|
|
handleUnreachableCodeConditionParens(expr->getCond());
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::VisitBinaryConditionalOperator(BinaryConditionalOperator const * expr) {
|
|
|
|
handleUnreachableCodeConditionParens(expr->getCond());
|
2017-11-27 09:46:06 +01:00
|
|
|
return true;
|
2017-07-06 14:49:15 +02:00
|
|
|
}
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr)
|
|
|
|
{
|
|
|
|
if (ignoreLocation(parenExpr))
|
|
|
|
return true;
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(parenExpr).isMacroID())
|
2017-07-05 08:32:57 +02:00
|
|
|
return true;
|
2017-11-27 09:46:06 +01:00
|
|
|
if (handled_.find(parenExpr) != handled_.end())
|
2017-07-06 14:49:15 +02:00
|
|
|
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))
|
|
|
|
{
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(subParenExpr).isMacroID())
|
2017-07-05 08:32:57 +02:00
|
|
|
return true;
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "parentheses around parentheses",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-07-05 08:32:57 +02:00
|
|
|
<< parenExpr->getSourceRange();
|
2017-11-27 09:46:06 +01:00
|
|
|
handled_.insert(subParenExpr);
|
2017-07-05 08:32:57 +02:00
|
|
|
}
|
2017-07-06 14:49:15 +02:00
|
|
|
|
2017-11-27 09:46:06 +01:00
|
|
|
// Somewhat redundantly add parenExpr to handled_, so that issues within InitListExpr don't get
|
|
|
|
// reported twice (without having to change TraverseInitListExpr to only either traverse the
|
|
|
|
// syntactic or semantic form, as other plugins do):
|
|
|
|
|
|
|
|
if (isa<DeclRefExpr>(subExpr)) {
|
|
|
|
if (!isPrecededBy_BAD_CAST(parenExpr)) {
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "unnecessary parentheses around identifier",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-11-27 09:46:06 +01:00
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
handled_.insert(parenExpr);
|
|
|
|
}
|
2017-11-29 18:21:29 +01:00
|
|
|
} else if (isa<IntegerLiteral>(subExpr) || isa<CharacterLiteral>(subExpr)
|
|
|
|
|| isa<FloatingLiteral>(subExpr) || isa<ImaginaryLiteral>(subExpr)
|
|
|
|
|| isa<CXXBoolLiteralExpr>(subExpr) || isa<CXXNullPtrLiteralExpr>(subExpr)
|
|
|
|
|| isa<ObjCBoolLiteralExpr>(subExpr))
|
2017-11-27 09:46:06 +01:00
|
|
|
{
|
2018-08-10 12:35:21 +02:00
|
|
|
auto const loc = compat::getBeginLoc(subExpr);
|
2017-11-27 09:46:06 +01:00
|
|
|
if (loc.isMacroID() && compiler.getSourceManager().isAtStartOfImmediateMacroExpansion(loc))
|
|
|
|
{
|
|
|
|
// just in case the macro could also expand to something that /would/ require
|
|
|
|
// parentheses here
|
|
|
|
return true;
|
2017-11-22 09:54:10 +01:00
|
|
|
}
|
2017-07-06 14:49:15 +02:00
|
|
|
report(
|
2017-11-27 09:46:06 +01:00
|
|
|
DiagnosticsEngine::Warning, "unnecessary parentheses around literal",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-07-06 14:49:15 +02:00
|
|
|
<< parenExpr->getSourceRange();
|
2017-11-27 09:46:06 +01:00
|
|
|
handled_.insert(parenExpr);
|
|
|
|
} else if (auto const e = dyn_cast<clang::StringLiteral>(subExpr)) {
|
|
|
|
if (e->getNumConcatenated() == 1 && !isPrecededBy_BAD_CAST(parenExpr)) {
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning,
|
|
|
|
"unnecessary parentheses around single-token string literal",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-11-27 09:46:06 +01:00
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
handled_.insert(parenExpr);
|
|
|
|
}
|
2017-11-30 07:17:53 +01:00
|
|
|
} else if (auto const e = dyn_cast<UnaryOperator>(subExpr)) {
|
|
|
|
auto const op = e->getOpcode();
|
|
|
|
if (op == UO_Plus || op == UO_Minus) {
|
|
|
|
auto const e2 = e->getSubExpr();
|
|
|
|
if (isa<IntegerLiteral>(e2) || isa<FloatingLiteral>(e2) || isa<ImaginaryLiteral>(e2)) {
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning,
|
|
|
|
"unnecessary parentheses around signed numeric literal",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-11-30 07:17:53 +01:00
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
handled_.insert(parenExpr);
|
|
|
|
}
|
|
|
|
}
|
2017-11-27 09:46:06 +01:00
|
|
|
} else if (isa<CXXNamedCastExpr>(subExpr)) {
|
Enable loplugin:cstylecast for some more cases
...mostly of C-style casts among arithmetic types, and automatically rewrite
those into either static_cast or a functional cast (which should have identical
semantics, but where the latter probably looks better for simple cases like
casting a literal to a specific type, as in "sal_Int32(0)" vs.
"static_cast<sal_Int32>(0)").
The main benefit of reducing the amount of C-style casts across the code base
further is so that other plugins (that have not been taught about the complex
semantics of C-style cast) can pick those up (cf. the various recent
"loplugin:redundantcast" commits, which address those findings after this
improved loplugin:cstylecast has been run). Also, I found some places where
a C-style cast has probably been applied only to the first part of a larger
expression in error (because it's easy to forget parentheses in cases like
"(sal_uInt16)VOPT_CLIPMARKS+1"); I'll follow up on those individually.
The improved loplugin:cstylecast is careful to output either "(performs:
static_cast)" or "(performs: functional cast)", so that
compilerplugins/clang/test/cstylecast.cxx can check that the plugin would
automatically rewrite to one or the other form.
To allow fully-automatic rewriting, this also required loplugin:unnecessaryparen
to become a rewriting plugin, at least for the parens-around-cast case (where
"((foo)bar)" first gets rewritten to "(static_cast<foo>(bar))", then to
"static_cast<foo>(bar)". Rewriting could probably be added to other cases of
loplugin:unnecessaryparen in the future, too.
(The final version of this patch would even have been able to cope with
361dd2576a09fbda83f3ce9a26ecb590c38f74e3 "Replace some C-style casts in ugly
macros with static_cast", so that manual change would not have been necessary
after all.)
Change-Id: Icd7e319cc38eb58262fcbf7643d177ac9ea0220a
Reviewed-on: https://gerrit.libreoffice.org/47798
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2018-01-10 15:55:22 +01:00
|
|
|
if (!removeParens(parenExpr)) {
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "unnecessary parentheses around cast",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
Enable loplugin:cstylecast for some more cases
...mostly of C-style casts among arithmetic types, and automatically rewrite
those into either static_cast or a functional cast (which should have identical
semantics, but where the latter probably looks better for simple cases like
casting a literal to a specific type, as in "sal_Int32(0)" vs.
"static_cast<sal_Int32>(0)").
The main benefit of reducing the amount of C-style casts across the code base
further is so that other plugins (that have not been taught about the complex
semantics of C-style cast) can pick those up (cf. the various recent
"loplugin:redundantcast" commits, which address those findings after this
improved loplugin:cstylecast has been run). Also, I found some places where
a C-style cast has probably been applied only to the first part of a larger
expression in error (because it's easy to forget parentheses in cases like
"(sal_uInt16)VOPT_CLIPMARKS+1"); I'll follow up on those individually.
The improved loplugin:cstylecast is careful to output either "(performs:
static_cast)" or "(performs: functional cast)", so that
compilerplugins/clang/test/cstylecast.cxx can check that the plugin would
automatically rewrite to one or the other form.
To allow fully-automatic rewriting, this also required loplugin:unnecessaryparen
to become a rewriting plugin, at least for the parens-around-cast case (where
"((foo)bar)" first gets rewritten to "(static_cast<foo>(bar))", then to
"static_cast<foo>(bar)". Rewriting could probably be added to other cases of
loplugin:unnecessaryparen in the future, too.
(The final version of this patch would even have been able to cope with
361dd2576a09fbda83f3ce9a26ecb590c38f74e3 "Replace some C-style casts in ugly
macros with static_cast", so that manual change would not have been necessary
after all.)
Change-Id: Icd7e319cc38eb58262fcbf7643d177ac9ea0220a
Reviewed-on: https://gerrit.libreoffice.org/47798
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2018-01-10 15:55:22 +01:00
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
}
|
2017-11-27 09:46:06 +01:00
|
|
|
handled_.insert(parenExpr);
|
2019-03-05 13:44:17 +02:00
|
|
|
} else if (auto memberExpr = dyn_cast<MemberExpr>(subExpr)) {
|
|
|
|
if (isa<CXXThisExpr>(ignoreAllImplicit(memberExpr->getBase()))) {
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "unnecessary parentheses around member expr",
|
|
|
|
compat::getBeginLoc(parenExpr))
|
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
handled_.insert(parenExpr);
|
|
|
|
}
|
2017-09-04 08:53:38 +02:00
|
|
|
}
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::VisitIfStmt(const IfStmt* ifStmt)
|
|
|
|
{
|
2017-11-29 18:21:29 +01:00
|
|
|
handleUnreachableCodeConditionParens(ifStmt->getCond());
|
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)
|
|
|
|
{
|
2017-11-29 18:21:29 +01:00
|
|
|
handleUnreachableCodeConditionParens(whileStmt->getCond());
|
2017-07-05 16:30:06 +02:00
|
|
|
VisitSomeStmt(whileStmt, whileStmt->getCond(), "while");
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2017-11-29 18:21:29 +01:00
|
|
|
bool UnnecessaryParen::VisitForStmt(ForStmt const * stmt) {
|
|
|
|
if (auto const cond = stmt->getCond()) {
|
|
|
|
handleUnreachableCodeConditionParens(cond);
|
|
|
|
}
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2017-07-05 16:30:06 +02:00
|
|
|
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;
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(parenExpr).isMacroID())
|
2017-08-17 17:21:53 +02:00
|
|
|
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
|
2017-12-29 12:23:33 +02:00
|
|
|
auto subExpr = ignoreAllImplicit(parenExpr->getSubExpr());
|
2017-08-17 17:21:53 +02:00
|
|
|
if (isa<CallExpr>(subExpr) && !isa<CXXOperatorCallExpr>(subExpr))
|
|
|
|
{
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "parentheses immediately inside return statement",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-08-17 17:21:53 +02:00
|
|
|
<< parenExpr->getSourceRange();
|
2017-11-27 09:46:06 +01:00
|
|
|
handled_.insert(parenExpr);
|
2017-08-17 17:21:53 +02:00
|
|
|
}
|
|
|
|
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-11-29 18:21:29 +01:00
|
|
|
if (handled_.find(parenExpr) != handled_.end()) {
|
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
|
|
|
return;
|
|
|
|
}
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(parenExpr).isMacroID())
|
2017-11-29 18:21:29 +01:00
|
|
|
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",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-07-05 16:30:06 +02:00
|
|
|
<< stmtName
|
|
|
|
<< parenExpr->getSourceRange();
|
2017-11-27 09:46:06 +01:00
|
|
|
handled_.insert(parenExpr);
|
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;
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(parenExpr).isMacroID())
|
2017-11-20 10:26:01 +02:00
|
|
|
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",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-11-20 10:26:01 +02:00
|
|
|
<< parenExpr->getSourceRange();
|
2017-11-27 09:46:06 +01:00
|
|
|
handled_.insert(parenExpr);
|
2017-11-20 10:26:01 +02:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2018-01-17 09:01:09 +02:00
|
|
|
bool UnnecessaryParen::VisitCXXDeleteExpr(const CXXDeleteExpr* deleteExpr)
|
|
|
|
{
|
|
|
|
if (ignoreLocation(deleteExpr))
|
|
|
|
return true;
|
|
|
|
|
|
|
|
auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(deleteExpr->getArgument()));
|
|
|
|
if (!parenExpr)
|
|
|
|
return true;
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(parenExpr).isMacroID())
|
2018-01-17 09:01:09 +02:00
|
|
|
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 delete expr",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2018-01-17 09:01:09 +02:00
|
|
|
<< parenExpr->getSourceRange();
|
|
|
|
handled_.insert(parenExpr);
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2017-11-20 10:26:01 +02:00
|
|
|
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;
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(parenExpr).isMacroID())
|
2017-11-20 10:26:01 +02:00
|
|
|
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();
|
2018-12-04 11:20:03 +02:00
|
|
|
if (auto subBinOp = dyn_cast<BinaryOperator>(sub))
|
|
|
|
{
|
|
|
|
if (!(subBinOp->isMultiplicativeOp() || subBinOp->isAdditiveOp() || subBinOp->isPtrMemOp()))
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
if (auto subOperatorCall = dyn_cast<CXXOperatorCallExpr>(sub))
|
|
|
|
{
|
|
|
|
auto op = subOperatorCall->getOperator();
|
|
|
|
if (!((op >= OO_Plus && op <= OO_Exclaim) || (op >= OO_ArrowStar && op <= OO_Subscript)))
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
if (isa<ConditionalOperator>(sub))
|
2017-11-20 10:26:01 +02:00
|
|
|
return true;
|
|
|
|
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "parentheses immediately inside assignment",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-11-20 10:26:01 +02:00
|
|
|
<< parenExpr->getSourceRange();
|
2017-11-27 09:46:06 +01:00
|
|
|
handled_.insert(parenExpr);
|
2017-11-20 10:26:01 +02:00
|
|
|
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;
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(parenExpr).isMacroID())
|
2017-11-20 10:26:01 +02:00
|
|
|
return true;
|
2018-12-04 11:20:03 +02:00
|
|
|
|
|
|
|
// Sometimes parentheses make the RHS of an assignment easier to read by
|
|
|
|
// visually disambiguating the = from a call to ==
|
2017-11-20 10:26:01 +02:00
|
|
|
auto sub = parenExpr->getSubExpr();
|
2020-04-11 16:55:25 +02:00
|
|
|
#if CLANG_VERSION >= 100000
|
|
|
|
if (auto const e = dyn_cast<CXXRewrittenBinaryOperator>(sub)) {
|
|
|
|
sub = e->getDecomposedForm().InnerBinOp;
|
|
|
|
}
|
|
|
|
#endif
|
2018-12-04 11:20:03 +02:00
|
|
|
if (auto subBinOp = dyn_cast<BinaryOperator>(sub))
|
|
|
|
{
|
|
|
|
if (!(subBinOp->isMultiplicativeOp() || subBinOp->isAdditiveOp() || subBinOp->isPtrMemOp()))
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
if (auto subOperatorCall = dyn_cast<CXXOperatorCallExpr>(sub))
|
|
|
|
{
|
|
|
|
auto op = subOperatorCall->getOperator();
|
|
|
|
if (!((op >= OO_Plus && op <= OO_Exclaim) || (op >= OO_ArrowStar && op <= OO_Subscript)))
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
if (isa<ConditionalOperator>(sub))
|
|
|
|
return true;
|
|
|
|
|
|
|
|
// these two are for "parentheses were disambiguated as a function declaration [-Werror,-Wvexing-parse]"
|
2019-02-19 17:38:24 +01:00
|
|
|
auto const sub2 = sub->IgnoreImplicit();
|
|
|
|
if (isa<CXXTemporaryObjectExpr>(sub2)
|
|
|
|
|| isa<CXXFunctionalCastExpr>(sub2))
|
2017-11-20 10:26:01 +02:00
|
|
|
return true;
|
|
|
|
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "parentheses immediately inside vardecl statement",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-11-20 10:26:01 +02:00
|
|
|
<< parenExpr->getSourceRange();
|
2017-11-27 09:46:06 +01:00
|
|
|
handled_.insert(parenExpr);
|
2017-07-07 08:42:54 +02:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2017-11-24 13:08:01 +02:00
|
|
|
bool UnnecessaryParen::VisitMemberExpr(const MemberExpr* memberExpr)
|
|
|
|
{
|
|
|
|
if (ignoreLocation(memberExpr))
|
|
|
|
return true;
|
|
|
|
|
|
|
|
auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(memberExpr->getBase()));
|
|
|
|
if (!parenExpr)
|
|
|
|
return true;
|
2017-11-27 09:46:06 +01:00
|
|
|
if (handled_.find(parenExpr) != handled_.end())
|
|
|
|
return true;
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(parenExpr).isMacroID())
|
2017-11-24 13:08:01 +02:00
|
|
|
return true;
|
|
|
|
|
|
|
|
auto sub = parenExpr->getSubExpr();
|
|
|
|
if (isa<CallExpr>(sub)) {
|
|
|
|
if (isa<CXXOperatorCallExpr>(sub))
|
|
|
|
return true;
|
|
|
|
} else if (isa<CXXConstructExpr>(sub)) {
|
|
|
|
// warn
|
|
|
|
} else if (isa<MemberExpr>(sub)) {
|
|
|
|
// warn
|
|
|
|
} else if (isa<DeclRefExpr>(sub)) {
|
|
|
|
// warn
|
|
|
|
} else
|
|
|
|
return true;
|
|
|
|
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Warning, "unnecessary parentheses around member expr",
|
2018-08-10 12:35:21 +02:00
|
|
|
compat::getBeginLoc(parenExpr))
|
2017-11-24 13:08:01 +02:00
|
|
|
<< parenExpr->getSourceRange();
|
2017-11-27 09:46:06 +01:00
|
|
|
handled_.insert(parenExpr);
|
2017-11-24 13:08:01 +02:00
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2017-11-29 18:21:29 +01:00
|
|
|
// Conservatively assume any parenthesised integer or Boolean (incl. Objective-C ones) literal in
|
|
|
|
// certain condition expressions (i.e., those for which handleUnreachableCodeConditionParens is
|
|
|
|
// called) to be parenthesised to silence Clang -Wunreachable-code, if that is either the whole
|
|
|
|
// condition expression or appears as a certain sub-expression (looking at what isConfigurationValue
|
|
|
|
// in Clang's lib/Analysis/ReachableCode.cpp looks for, descending into certain unary and binary
|
|
|
|
// operators):
|
|
|
|
void UnnecessaryParen::handleUnreachableCodeConditionParens(Expr const * expr) {
|
|
|
|
// Cf. :
|
|
|
|
auto const e = ignoreAllImplicit(expr);
|
|
|
|
if (auto const e1 = dyn_cast<ParenExpr>(e)) {
|
|
|
|
auto const sub = e1->getSubExpr();
|
|
|
|
if (isa<IntegerLiteral>(sub) || isa<CXXBoolLiteralExpr>(sub)
|
|
|
|
|| isa<ObjCBoolLiteralExpr>(sub))
|
|
|
|
{
|
|
|
|
handled_.insert(e1);
|
|
|
|
}
|
|
|
|
} else if (auto const e1 = dyn_cast<UnaryOperator>(e)) {
|
|
|
|
if (e1->getOpcode() == UO_LNot) {
|
|
|
|
handleUnreachableCodeConditionParens(e1->getSubExpr());
|
|
|
|
}
|
|
|
|
} else if (auto const e1 = dyn_cast<BinaryOperator>(e)) {
|
|
|
|
if (e1->isLogicalOp() || e1->isComparisonOp()) {
|
|
|
|
handleUnreachableCodeConditionParens(e1->getLHS());
|
|
|
|
handleUnreachableCodeConditionParens(e1->getRHS());
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2017-11-27 09:46:06 +01:00
|
|
|
bool UnnecessaryParen::isPrecededBy_BAD_CAST(Expr const * expr) {
|
2018-08-10 12:35:21 +02:00
|
|
|
if (compat::getBeginLoc(expr).isMacroID()) {
|
2017-11-27 09:46:06 +01:00
|
|
|
return false;
|
|
|
|
}
|
|
|
|
SourceManager& SM = compiler.getSourceManager();
|
2018-08-10 12:35:21 +02:00
|
|
|
const char *p1 = SM.getCharacterData( compat::getBeginLoc(expr).getLocWithOffset(-10) );
|
|
|
|
const char *p2 = SM.getCharacterData( compat::getBeginLoc(expr) );
|
2017-11-27 09:46:06 +01:00
|
|
|
return std::string(p1, p2 - p1).find("BAD_CAST") != std::string::npos;
|
|
|
|
}
|
|
|
|
|
Enable loplugin:cstylecast for some more cases
...mostly of C-style casts among arithmetic types, and automatically rewrite
those into either static_cast or a functional cast (which should have identical
semantics, but where the latter probably looks better for simple cases like
casting a literal to a specific type, as in "sal_Int32(0)" vs.
"static_cast<sal_Int32>(0)").
The main benefit of reducing the amount of C-style casts across the code base
further is so that other plugins (that have not been taught about the complex
semantics of C-style cast) can pick those up (cf. the various recent
"loplugin:redundantcast" commits, which address those findings after this
improved loplugin:cstylecast has been run). Also, I found some places where
a C-style cast has probably been applied only to the first part of a larger
expression in error (because it's easy to forget parentheses in cases like
"(sal_uInt16)VOPT_CLIPMARKS+1"); I'll follow up on those individually.
The improved loplugin:cstylecast is careful to output either "(performs:
static_cast)" or "(performs: functional cast)", so that
compilerplugins/clang/test/cstylecast.cxx can check that the plugin would
automatically rewrite to one or the other form.
To allow fully-automatic rewriting, this also required loplugin:unnecessaryparen
to become a rewriting plugin, at least for the parens-around-cast case (where
"((foo)bar)" first gets rewritten to "(static_cast<foo>(bar))", then to
"static_cast<foo>(bar)". Rewriting could probably be added to other cases of
loplugin:unnecessaryparen in the future, too.
(The final version of this patch would even have been able to cope with
361dd2576a09fbda83f3ce9a26ecb590c38f74e3 "Replace some C-style casts in ugly
macros with static_cast", so that manual change would not have been necessary
after all.)
Change-Id: Icd7e319cc38eb58262fcbf7643d177ac9ea0220a
Reviewed-on: https://gerrit.libreoffice.org/47798
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2018-01-10 15:55:22 +01:00
|
|
|
namespace {
|
|
|
|
|
|
|
|
bool badCombinationChar(char c) {
|
|
|
|
return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '_'
|
|
|
|
|| c == '+' || c == '-' || c == '\'' || c == '"';
|
|
|
|
}
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::badCombination(SourceLocation loc, int prevOffset, int nextOffset) {
|
2018-01-15 09:13:49 +01:00
|
|
|
//TODO: check for start/end of file; take backslash-newline line concatenation into account
|
Enable loplugin:cstylecast for some more cases
...mostly of C-style casts among arithmetic types, and automatically rewrite
those into either static_cast or a functional cast (which should have identical
semantics, but where the latter probably looks better for simple cases like
casting a literal to a specific type, as in "sal_Int32(0)" vs.
"static_cast<sal_Int32>(0)").
The main benefit of reducing the amount of C-style casts across the code base
further is so that other plugins (that have not been taught about the complex
semantics of C-style cast) can pick those up (cf. the various recent
"loplugin:redundantcast" commits, which address those findings after this
improved loplugin:cstylecast has been run). Also, I found some places where
a C-style cast has probably been applied only to the first part of a larger
expression in error (because it's easy to forget parentheses in cases like
"(sal_uInt16)VOPT_CLIPMARKS+1"); I'll follow up on those individually.
The improved loplugin:cstylecast is careful to output either "(performs:
static_cast)" or "(performs: functional cast)", so that
compilerplugins/clang/test/cstylecast.cxx can check that the plugin would
automatically rewrite to one or the other form.
To allow fully-automatic rewriting, this also required loplugin:unnecessaryparen
to become a rewriting plugin, at least for the parens-around-cast case (where
"((foo)bar)" first gets rewritten to "(static_cast<foo>(bar))", then to
"static_cast<foo>(bar)". Rewriting could probably be added to other cases of
loplugin:unnecessaryparen in the future, too.
(The final version of this patch would even have been able to cope with
361dd2576a09fbda83f3ce9a26ecb590c38f74e3 "Replace some C-style casts in ugly
macros with static_cast", so that manual change would not have been necessary
after all.)
Change-Id: Icd7e319cc38eb58262fcbf7643d177ac9ea0220a
Reviewed-on: https://gerrit.libreoffice.org/47798
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2018-01-10 15:55:22 +01:00
|
|
|
auto const c1
|
|
|
|
= compiler.getSourceManager().getCharacterData(loc.getLocWithOffset(prevOffset))[0];
|
|
|
|
auto const c2
|
|
|
|
= compiler.getSourceManager().getCharacterData(loc.getLocWithOffset(nextOffset))[0];
|
2018-01-15 09:13:49 +01:00
|
|
|
// An approximation of avoiding whatever combinations that would cause two adjacent tokens to be
|
Enable loplugin:cstylecast for some more cases
...mostly of C-style casts among arithmetic types, and automatically rewrite
those into either static_cast or a functional cast (which should have identical
semantics, but where the latter probably looks better for simple cases like
casting a literal to a specific type, as in "sal_Int32(0)" vs.
"static_cast<sal_Int32>(0)").
The main benefit of reducing the amount of C-style casts across the code base
further is so that other plugins (that have not been taught about the complex
semantics of C-style cast) can pick those up (cf. the various recent
"loplugin:redundantcast" commits, which address those findings after this
improved loplugin:cstylecast has been run). Also, I found some places where
a C-style cast has probably been applied only to the first part of a larger
expression in error (because it's easy to forget parentheses in cases like
"(sal_uInt16)VOPT_CLIPMARKS+1"); I'll follow up on those individually.
The improved loplugin:cstylecast is careful to output either "(performs:
static_cast)" or "(performs: functional cast)", so that
compilerplugins/clang/test/cstylecast.cxx can check that the plugin would
automatically rewrite to one or the other form.
To allow fully-automatic rewriting, this also required loplugin:unnecessaryparen
to become a rewriting plugin, at least for the parens-around-cast case (where
"((foo)bar)" first gets rewritten to "(static_cast<foo>(bar))", then to
"static_cast<foo>(bar)". Rewriting could probably be added to other cases of
loplugin:unnecessaryparen in the future, too.
(The final version of this patch would even have been able to cope with
361dd2576a09fbda83f3ce9a26ecb590c38f74e3 "Replace some C-style casts in ugly
macros with static_cast", so that manual change would not have been necessary
after all.)
Change-Id: Icd7e319cc38eb58262fcbf7643d177ac9ea0220a
Reviewed-on: https://gerrit.libreoffice.org/47798
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2018-01-10 15:55:22 +01:00
|
|
|
// lexed differently, using, for now, letters (TODO: non-ASCII ones) and digits and '_'; '+' and
|
|
|
|
// '-' (to avoid ++, etc.); '\'' and '"' (to avoid u'x' or "foo"bar, etc.):
|
|
|
|
return badCombinationChar(c1) && badCombinationChar(c2);
|
|
|
|
}
|
|
|
|
|
|
|
|
bool UnnecessaryParen::removeParens(ParenExpr const * expr) {
|
|
|
|
if (rewriter == nullptr) {
|
|
|
|
return false;
|
|
|
|
}
|
2018-08-10 12:35:21 +02:00
|
|
|
auto const firstBegin = compat::getBeginLoc(expr);
|
|
|
|
auto secondBegin = compat::getEndLoc(expr);
|
Enable loplugin:cstylecast for some more cases
...mostly of C-style casts among arithmetic types, and automatically rewrite
those into either static_cast or a functional cast (which should have identical
semantics, but where the latter probably looks better for simple cases like
casting a literal to a specific type, as in "sal_Int32(0)" vs.
"static_cast<sal_Int32>(0)").
The main benefit of reducing the amount of C-style casts across the code base
further is so that other plugins (that have not been taught about the complex
semantics of C-style cast) can pick those up (cf. the various recent
"loplugin:redundantcast" commits, which address those findings after this
improved loplugin:cstylecast has been run). Also, I found some places where
a C-style cast has probably been applied only to the first part of a larger
expression in error (because it's easy to forget parentheses in cases like
"(sal_uInt16)VOPT_CLIPMARKS+1"); I'll follow up on those individually.
The improved loplugin:cstylecast is careful to output either "(performs:
static_cast)" or "(performs: functional cast)", so that
compilerplugins/clang/test/cstylecast.cxx can check that the plugin would
automatically rewrite to one or the other form.
To allow fully-automatic rewriting, this also required loplugin:unnecessaryparen
to become a rewriting plugin, at least for the parens-around-cast case (where
"((foo)bar)" first gets rewritten to "(static_cast<foo>(bar))", then to
"static_cast<foo>(bar)". Rewriting could probably be added to other cases of
loplugin:unnecessaryparen in the future, too.
(The final version of this patch would even have been able to cope with
361dd2576a09fbda83f3ce9a26ecb590c38f74e3 "Replace some C-style casts in ugly
macros with static_cast", so that manual change would not have been necessary
after all.)
Change-Id: Icd7e319cc38eb58262fcbf7643d177ac9ea0220a
Reviewed-on: https://gerrit.libreoffice.org/47798
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2018-01-10 15:55:22 +01:00
|
|
|
if (firstBegin.isMacroID() || secondBegin.isMacroID()) {
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
unsigned firstLen = Lexer::MeasureTokenLength(
|
|
|
|
firstBegin, compiler.getSourceManager(), compiler.getLangOpts());
|
|
|
|
for (auto l = firstBegin.getLocWithOffset(std::max<unsigned>(firstLen, 1));;
|
|
|
|
l = l.getLocWithOffset(1))
|
|
|
|
{
|
|
|
|
unsigned n = Lexer::MeasureTokenLength(
|
|
|
|
l, compiler.getSourceManager(), compiler.getLangOpts());
|
|
|
|
if (n != 0) {
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
++firstLen;
|
|
|
|
}
|
|
|
|
unsigned secondLen = Lexer::MeasureTokenLength(
|
|
|
|
secondBegin, compiler.getSourceManager(), compiler.getLangOpts());
|
|
|
|
for (;;) {
|
|
|
|
auto l = secondBegin.getLocWithOffset(-1);
|
|
|
|
auto const c = compiler.getSourceManager().getCharacterData(l)[0];
|
|
|
|
if (c == '\n') {
|
|
|
|
if (compiler.getSourceManager().getCharacterData(l.getLocWithOffset(-1))[0] == '\\') {
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
} else if (!(c == ' ' || c == '\t' || c == '\v' || c == '\f')) {
|
|
|
|
break;
|
|
|
|
}
|
|
|
|
secondBegin = l;
|
|
|
|
++secondLen;
|
|
|
|
}
|
|
|
|
if (!replaceText(firstBegin, firstLen, badCombination(firstBegin, -1, firstLen) ? " " : "")) {
|
|
|
|
if (isDebugMode()) {
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Fatal,
|
|
|
|
"TODO: cannot rewrite opening parenthesis, needs investigation",
|
|
|
|
firstBegin);
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Note, "when removing these parentheses", expr->getExprLoc())
|
|
|
|
<< expr->getSourceRange();
|
|
|
|
}
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
if (!replaceText(secondBegin, secondLen, badCombination(secondBegin, -1, secondLen) ? " " : ""))
|
|
|
|
{
|
|
|
|
//TODO: roll back first change
|
|
|
|
if (isDebugMode()) {
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Fatal,
|
|
|
|
"TODO: cannot rewrite closing parenthesis, needs investigation",
|
|
|
|
secondBegin);
|
|
|
|
report(
|
|
|
|
DiagnosticsEngine::Note, "when removing these parentheses", expr->getExprLoc())
|
|
|
|
<< expr->getSourceRange();
|
|
|
|
}
|
|
|
|
return false;
|
|
|
|
}
|
|
|
|
return true;
|
|
|
|
}
|
|
|
|
|
2019-03-07 14:31:17 +01:00
|
|
|
loplugin::Plugin::Registration< UnnecessaryParen > unnecessaryparen("unnecessaryparen", true);
|
2017-07-05 08:32:57 +02:00
|
|
|
|
|
|
|
}
|
|
|
|
|
2019-03-07 14:31:17 +01:00
|
|
|
#endif // LO_CLANG_SHARED_PLUGINS
|
|
|
|
|
2017-07-05 08:32:57 +02:00
|
|
|
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
|