From fe10d89672d3b106775524852cf612131a7e4ea6 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Fri, 2 Aug 2019 16:31:38 +0000 Subject: [PATCH] [Sema] Disable -Wbitwise-op-parentheses and -Wlogical-op-parentheses by default Summary: The -Wparentheses warnings are enabled by default in clang but they are under -Wall in gcc (gcc/c-family/c.opt). Some of the operator precedence warnings are oftentimes criticized as noise (clang: default; gcc: -Wall). If a warning is very controversial, it is probably not a good idea to enable it by default. This patch disables the rather annoying ones: -Wbitwise-op-parentheses, e.g. i & i | i -Wlogical-op-parentheses, e.g. i && i || i After this change: ``` * = enabled by default -Wall -Wparentheses -Wlogical-op-parentheses -Wlogical-not-parentheses* -Wbitwise-op-parentheses -Wshift-op-parentheses* -Woverloaded-shift-op-parentheses* -Wparentheses-equality* -Wdangling-else* ``` -Woverloaded-shift-op-parentheses is typically followed by overload resolution failure. We can instead improve the error message, and probably delete -Woverloaded-shift-op-parentheses in the future. Keep it for now because it gives some diagnostics. Reviewers: akyrtzi, jyknight, rtrieu, rsmith, aaron.ballman Reviewed By: aaron.ballman Subscribers: dexonsmith, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D65192 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@367690 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 4 +- test/Sema/bitwise-op-parentheses.c | 58 ++++++++++++++++++++++ test/Sema/logical-op-parentheses.c | 41 +++++++++++++++ test/Sema/parentheses.c | 53 +------------------- test/SemaCXX/parentheses.cpp | 2 +- 5 files changed, 103 insertions(+), 55 deletions(-) create mode 100644 test/Sema/bitwise-op-parentheses.c create mode 100644 test/Sema/logical-op-parentheses.c diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 2704a56e43..cf036b5760 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -5642,10 +5642,10 @@ def note_logical_instead_of_bitwise_remove_constant : Note< "remove constant to silence this warning">; def warn_bitwise_op_in_bitwise_op : Warning< - "'%0' within '%1'">, InGroup; + "'%0' within '%1'">, InGroup, DefaultIgnore; def warn_logical_and_in_logical_or : Warning< - "'&&' within '||'">, InGroup; + "'&&' within '||'">, InGroup, DefaultIgnore; def warn_overloaded_shift_in_comparison :Warning< "overloaded operator %select{>>|<<}0 has higher precedence than " diff --git a/test/Sema/bitwise-op-parentheses.c b/test/Sema/bitwise-op-parentheses.c new file mode 100644 index 0000000000..ed270de842 --- /dev/null +++ b/test/Sema/bitwise-op-parentheses.c @@ -0,0 +1,58 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -DSILENCE +// RUN: %clang_cc1 -fsyntax-only -verify %s -Wbitwise-op-parentheses +// RUN: %clang_cc1 -fsyntax-only -verify %s -Wparentheses +// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s -Wbitwise-op-parentheses 2>&1 | FileCheck %s + +#ifdef SILENCE +// expected-no-diagnostics +#endif + +void bitwise_op_parentheses(unsigned i) { + (void)(i & i | i); +#ifndef SILENCE + // expected-warning@-2 {{'&' within '|'}} + // expected-note@-3 {{place parentheses around the '&' expression to silence this warning}} +#endif + // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")" + + (void)(i | i & i); +#ifndef SILENCE + // expected-warning@-2 {{'&' within '|'}} + // expected-note@-3 {{place parentheses around the '&' expression to silence this warning}} +#endif + // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:19-[[@LINE-6]]:19}:")" + + (void)(i ^ i | i); +#ifndef SILENCE + // expected-warning@-2 {{'^' within '|'}} + // expected-note@-3 {{place parentheses around the '^' expression to silence this warning}} +#endif + // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")" + + (void)(i | i ^ i); +#ifndef SILENCE + // expected-warning@-2 {{'^' within '|'}} + // expected-note@-3 {{place parentheses around the '^' expression to silence this warning}} +#endif + // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:19-[[@LINE-6]]:19}:")" + + (void)(i & i ^ i); +#ifndef SILENCE + // expected-warning@-2 {{'&' within '^'}} + // expected-note@-3 {{place parentheses around the '&' expression to silence this warning}} +#endif + // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")" + + (void)(i ^ i & i); +#ifndef SILENCE + // expected-warning@-2 {{'&' within '^'}} + // expected-note@-3 {{place parentheses around the '&' expression to silence this warning}} +#endif + // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:19-[[@LINE-6]]:19}:")" +} diff --git a/test/Sema/logical-op-parentheses.c b/test/Sema/logical-op-parentheses.c new file mode 100644 index 0000000000..84464b4009 --- /dev/null +++ b/test/Sema/logical-op-parentheses.c @@ -0,0 +1,41 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s -DSILENCE +// RUN: %clang_cc1 -fsyntax-only -verify %s -Wlogical-op-parentheses +// RUN: %clang_cc1 -fsyntax-only -verify %s -Wparentheses +// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s -Wlogical-op-parentheses 2>&1 | FileCheck %s + +#ifdef SILENCE +// expected-no-diagnostics +#endif + +void logical_op_parentheses(unsigned i) { + (void)(i || + i && i); +#ifndef SILENCE + // expected-warning@-2 {{'&&' within '||'}} + // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}} +#endif + // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:20-[[@LINE-6]]:20}:")" + + (void)(i || i && "w00t"); + (void)("w00t" && i || i); + + (void)(i || i && "w00t" || i); +#ifndef SILENCE + // expected-warning@-2 {{'&&' within '||'}} + // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}} +#endif + // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")" + + (void)(i || "w00t" && i || i); +#ifndef SILENCE + // expected-warning@-2 {{'&&' within '||'}} + // expected-note@-3 {{place parentheses around the '&&' expression to silence this warning}} +#endif + // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:15-[[@LINE-5]]:15}:"(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:26-[[@LINE-6]]:26}:")" + + (void)(i && i || 0); + (void)(0 || i && i); +} diff --git a/test/Sema/parentheses.c b/test/Sema/parentheses.c index 8c6c4999f7..0dabc11d39 100644 --- a/test/Sema/parentheses.c +++ b/test/Sema/parentheses.c @@ -1,3 +1,4 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s // RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify %s // RUN: %clang_cc1 -Wparentheses -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s @@ -44,58 +45,6 @@ void bitwise_rel(unsigned i) { // Eager logical op (void)(i == 1 | i == 2 | i == 3); (void)(i != 1 & i != 2 & i != 3); - - (void)(i & i | i); // expected-warning {{'&' within '|'}} \ - // expected-note {{place parentheses around the '&' expression to silence this warning}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:"(" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:15}:")" - - (void)(i | i & i); // expected-warning {{'&' within '|'}} \ - // expected-note {{place parentheses around the '&' expression to silence this warning}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:14}:"(" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:19-[[@LINE-3]]:19}:")" - - (void)(i ^ i | i); // expected-warning {{'^' within '|'}} \ - // expected-note {{place parentheses around the '^' expression to silence this warning}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:"(" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:15}:")" - - (void)(i | i ^ i); // expected-warning {{'^' within '|'}} \ - // expected-note {{place parentheses around the '^' expression to silence this warning}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:14}:"(" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:19-[[@LINE-3]]:19}:")" - - (void)(i & i ^ i); // expected-warning {{'&' within '^'}} \ - // expected-note {{place parentheses around the '&' expression to silence this warning}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:"(" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:15}:")" - - (void)(i ^ i & i); // expected-warning {{'&' within '^'}} \ - // expected-note {{place parentheses around the '&' expression to silence this warning}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:14}:"(" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:19-[[@LINE-3]]:19}:")" - - (void)(i || - i && i); // expected-warning {{'&&' within '||'}} \ - // expected-note {{place parentheses around the '&&' expression to silence this warning}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:14}:"(" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:20-[[@LINE-3]]:20}:")" - - (void)(i || i && "w00t"); // no warning. - (void)("w00t" && i || i); // no warning. - - (void)(i || i && "w00t" || i); // expected-warning {{'&&' within '||'}} \ - // expected-note {{place parentheses around the '&&' expression to silence this warning}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"(" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:26-[[@LINE-3]]:26}:")" - - (void)(i || "w00t" && i || i); // expected-warning {{'&&' within '||'}} \ - // expected-note {{place parentheses around the '&&' expression to silence this warning}} - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"(" - // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:26-[[@LINE-3]]:26}:")" - - (void)(i && i || 0); // no warning. - (void)(0 || i && i); // no warning. } _Bool someConditionFunc(); diff --git a/test/SemaCXX/parentheses.cpp b/test/SemaCXX/parentheses.cpp index b430b25e5d..d466bd0138 100644 --- a/test/SemaCXX/parentheses.cpp +++ b/test/SemaCXX/parentheses.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -verify %s +// RUN: %clang_cc1 -verify -Wlogical-op-parentheses %s // PR16930, PR16727: template -- 2.40.0