mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Enable loplugin:unnecessaryparen for integer and Boolean literals

Browse Source 
...taking care not to warn about those cases that are used to silence Clang's
-Wunreachable-code

Change-Id: I3c1da907f51cc786f81c1322fe71d75832cd9191
Reviewed-on: https://gerrit.libreoffice.org/45521
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
This commit is contained in:
Stephan Bergmann 2017-11-29 18:21:29 +01:00
parent ea55492a6e
commit 0025fa723a
2 changed files with 71 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
compilerplugins/clang/test/unnecessaryparen.cxx
View File

@ -10,6 +10,8 @@
#include <string>
#include <rtl/ustring.hxx>
#define MACRO (1)
bool foo(int);
enum class EFoo { Bar };
@ -28,8 +30,8 @@ int main()
int y = (x); // expected-error {{parentheses immediately inside vardecl statement [loplugin:unnecessaryparen]}}
(void)y;
EFoo foo = EFoo::Bar;
switch (foo) {
EFoo efoo = EFoo::Bar;
switch (efoo) {
case (EFoo::Bar): break; // expected-error {{parentheses immediately inside case statement [loplugin:unnecessaryparen]}}
}
@ -45,6 +47,19 @@ int main()
}
x = (true) ? 0 : 1;
// More "no warnings", at least potentially used to silence -Wunreachable-code:
while ((false)) {
return 0;
}
for (; (false);) {
return 0;
}
x = foo(0) && (false) ? 0 : 1;
x = MACRO < (0) ? 0 : 1;
// cf. odd Clang -Wunreachable-code--suppression mechanism when the macro itself contains
// parentheses, causing the issue that lead to c421ac3f9432f2e9468d28447dc4c2e45b6f4da3
// "Revert loplugin:unnecessaryparen warning around integer literals"
int v2 = (1); // expected-error {{parentheses immediately inside vardecl statement [loplugin:unnecessaryparen]}}
(void)v2;

70
compilerplugins/clang/unnecessaryparen.cxx
View File

@ -76,6 +76,7 @@ public:
bool VisitIfStmt(const IfStmt *);
bool VisitDoStmt(const DoStmt *);
bool VisitWhileStmt(const WhileStmt *);
bool VisitForStmt(ForStmt const * stmt);
bool VisitSwitchStmt(const SwitchStmt *);
bool VisitCaseStmt(const CaseStmt *);
bool VisitReturnStmt(const ReturnStmt* );
@ -84,10 +85,13 @@ public:
bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *);
bool VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr const *);
bool VisitConditionalOperator(ConditionalOperator const * expr);
bool VisitBinaryConditionalOperator(BinaryConditionalOperator const * expr);
bool VisitMemberExpr(const MemberExpr *f);
private:
void VisitSomeStmt(Stmt const * stmt, const Expr* cond, StringRef stmtName);
void handleUnreachableCodeConditionParens(Expr const * expr);
// 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):
@ -107,11 +111,12 @@ bool UnnecessaryParen::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr co
}
bool UnnecessaryParen::VisitConditionalOperator(ConditionalOperator const * expr) {
if (auto const e = dyn_cast<ParenExpr>(ignoreAllImplicit(expr->getCond()))) {
if (isa<CXXBoolLiteralExpr>(e->getSubExpr())) {
handled_.insert(e);
}
}
handleUnreachableCodeConditionParens(expr->getCond());
return true;
}
bool UnnecessaryParen::VisitBinaryConditionalOperator(BinaryConditionalOperator const * expr) {
handleUnreachableCodeConditionParens(expr->getCond());
return true;
}
@ -149,10 +154,10 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr)
<< parenExpr->getSourceRange();
handled_.insert(parenExpr);
}
} else if (isa<CharacterLiteral>(subExpr) || isa<FloatingLiteral>(subExpr)
|| isa<ImaginaryLiteral>(subExpr) || isa<CXXNullPtrLiteralExpr>(subExpr))
//TODO: isa<IntegerLiteral>(subExpr)
//TODO: isa<CXXBoolLiteralExpr>(subExpr) || isa<ObjCBoolLiteralExpr>(subExpr)
} else if (isa<IntegerLiteral>(subExpr) || isa<CharacterLiteral>(subExpr)
|| isa<FloatingLiteral>(subExpr) || isa<ImaginaryLiteral>(subExpr)
|| isa<CXXBoolLiteralExpr>(subExpr) || isa<CXXNullPtrLiteralExpr>(subExpr)
|| isa<ObjCBoolLiteralExpr>(subExpr))
{
auto const loc = subExpr->getLocStart();
if (loc.isMacroID() && compiler.getSourceManager().isAtStartOfImmediateMacroExpansion(loc))
@ -188,6 +193,7 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr)
bool UnnecessaryParen::VisitIfStmt(const IfStmt* ifStmt)
{
handleUnreachableCodeConditionParens(ifStmt->getCond());
VisitSomeStmt(ifStmt, ifStmt->getCond(), "if");
return true;
}
@ -200,10 +206,18 @@ bool UnnecessaryParen::VisitDoStmt(const DoStmt* doStmt)
bool UnnecessaryParen::VisitWhileStmt(const WhileStmt* whileStmt)
{
handleUnreachableCodeConditionParens(whileStmt->getCond());
VisitSomeStmt(whileStmt, whileStmt->getCond(), "while");
return true;
}
bool UnnecessaryParen::VisitForStmt(ForStmt const * stmt) {
if (auto const cond = stmt->getCond()) {
handleUnreachableCodeConditionParens(cond);
}
return true;
}
bool UnnecessaryParen::VisitSwitchStmt(const SwitchStmt* switchStmt)
{
VisitSomeStmt(switchStmt, switchStmt->getCond(), "switch");
@ -253,15 +267,11 @@ void UnnecessaryParen::VisitSomeStmt(const Stmt * stmt, const Expr* cond, String
auto parenExpr = dyn_cast<ParenExpr>(ignoreAllImplicit(cond));
if (parenExpr) {
if (parenExpr->getLocStart().isMacroID())
return;
// Used to silence -Wunreachable-code:
if (isa<CXXBoolLiteralExpr>(parenExpr->getSubExpr())
&& stmtName == "if")
{
handled_.insert(parenExpr);
if (handled_.find(parenExpr) != handled_.end()) {
return;
}
if (parenExpr->getLocStart().isMacroID())
return;
// assignments need extra parentheses or they generate a compiler warning
auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr());
if (binaryOp && binaryOp->getOpcode() == BO_Assign)
@ -406,6 +416,34 @@ bool UnnecessaryParen::VisitMemberExpr(const MemberExpr* memberExpr)
return true;
}
// 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());
}
}
}
bool UnnecessaryParen::isPrecededBy_BAD_CAST(Expr const * expr) {
if (expr->getLocStart().isMacroID()) {
return false;