[Libreoffice-commits] core.git: compilerplugins/clang
Stephan Bergmann
sbergman at redhat.com
Thu Nov 30 06:19:46 UTC 2017
compilerplugins/clang/test/unnecessaryparen.cxx | 19 +++++-
compilerplugins/clang/unnecessaryparen.cxx | 70 ++++++++++++++++++------
2 files changed, 71 insertions(+), 18 deletions(-)
New commits:
commit 0025fa723afb9f6a0d94b9b3185ea14da18f1bd5
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Wed Nov 29 18:21:29 2017 +0100
Enable loplugin:unnecessaryparen for integer and Boolean literals
...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 at libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
diff --git a/compilerplugins/clang/test/unnecessaryparen.cxx b/compilerplugins/clang/test/unnecessaryparen.cxx
index 7c5f525a1857..78e2096abf9e 100644
--- a/compilerplugins/clang/test/unnecessaryparen.cxx
+++ b/compilerplugins/clang/test/unnecessaryparen.cxx
@@ -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;
diff --git a/compilerplugins/clang/unnecessaryparen.cxx b/compilerplugins/clang/unnecessaryparen.cxx
index 903ce4e611ba..692e23706cc9 100644
--- a/compilerplugins/clang/unnecessaryparen.cxx
+++ b/compilerplugins/clang/unnecessaryparen.cxx
@@ -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;
More information about the Libreoffice-commits
mailing list