From patchwork Fri Apr 18 06:45:25 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 340232 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 559BA14007A for ; Fri, 18 Apr 2014 16:45:40 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=P/e4zSjYxfPbNV6Wr O/cN+ewxO/I0DBHdCyUyaTCfnlSQo9HpDY1zBhImZrBlvbaa0xh775eF2VCsvIT8 Tj5AKX6zsWcPOALVn7ufSCvsyL+a3xN/ZFvYNegQWtrGbAqWn7b5EP8YyC3tRXa7 EFucfO4LLqsTtga7DAHEJu841I= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=aImDGQWVH+GVt6AkLmE05pP D+PE=; b=DhV1d1edezyt2RnrrqEZLvOGsvJoIKyOFZENmYNdLTMogT+BTavjeVB 26+Pu+uAheEyGaYdH7w1L6XsR4oJphu/GXB7uTBcYS+coH0EGUBRSRj1D+AiJHW6 +Gggx3V8bCQtUn+uaymT/H1AV+6DhmpcuFLyHJg6IuwBCNKlcAg4= Received: (qmail 25943 invoked by alias); 18 Apr 2014 06:45:33 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 25933 invoked by uid 89); 18 Apr 2014 06:45:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 18 Apr 2014 06:45:32 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s3I6jTkw025902 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 18 Apr 2014 02:45:29 -0400 Received: from redhat.com (ovpn-116-31.ams2.redhat.com [10.36.116.31]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s3I6jPTW008099 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Fri, 18 Apr 2014 02:45:28 -0400 Date: Fri, 18 Apr 2014 08:45:25 +0200 From: Marek Polacek To: Marc Glisse Cc: GCC Patches , "Joseph S. Myers" Subject: Re: [C PATCH] Warn if switch has boolean value (PR c/60439) Message-ID: <20140418064525.GJ20332@redhat.com> References: <20140418053021.GH20332@redhat.com> <20140418060045.GI20332@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140418060045.GI20332@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) On Fri, Apr 18, 2014 at 08:00:45AM +0200, Marek Polacek wrote: > On Fri, Apr 18, 2014 at 07:49:22AM +0200, Marc Glisse wrote: > > On Fri, 18 Apr 2014, Marek Polacek wrote: > > > > >This patch implements a new warning that warns when controlling > > >expression of a switch has boolean value. (Intentionally I don't > > >warn if the controlling expression is (un)signed:1 bit-field.) > > >I guess the question is if this should be enabled by default or > > >deserves some new warning option. Since clang does the former, > > >I did it too and currently this warning is enabled by default. > > > > It can be enabled by -Wsome-name which is itself enabled by default but > > at least gives the possibility to use -Wno-some-name, -Werror=some-name, > > etc. No? I believe Manuel insists regularly that no new warning should > > use 0 (and old ones should progressively lose it). > > Yes, that's the other possibility and exactly what I wanted to > discuss. I think I'll prepare another version with -Wswitch-bool (and > documentation). Here. 2014-04-18 Marek Polacek PR c/60439 * doc/invoke.texi: Document -Wswitch-bool. c/ * c-typeck.c (c_start_case): Warn if switch condition has boolean value. c-family/ * c.opt (Wswitch-bool): New option. testsuite/ * gcc.dg/pr60439.c: New test. Marek diff --git gcc/c-family/c.opt gcc/c-family/c.opt index 390c056..9089496 100644 --- gcc/c-family/c.opt +++ gcc/c-family/c.opt @@ -529,6 +529,10 @@ Wswitch-enum C ObjC C++ ObjC++ Var(warn_switch_enum) Warning Warn about all enumerated switches missing a specific case +Wswitch-bool +C ObjC Warning Init(1) +Warn about switches with boolean controlling expression + Wmissing-format-attribute C ObjC C++ ObjC++ Alias(Wsuggest-attribute=format) ; diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index 65aad45..44982d3 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -9344,6 +9344,28 @@ c_start_case (location_t switch_loc, else { tree type = TYPE_MAIN_VARIANT (orig_type); + tree e = exp; + enum tree_code exp_code; + + while (TREE_CODE (e) == COMPOUND_EXPR) + e = TREE_OPERAND (e, 1); + exp_code = TREE_CODE (e); + + if (TREE_CODE (type) == BOOLEAN_TYPE + || exp_code == TRUTH_ANDIF_EXPR + || exp_code == TRUTH_AND_EXPR + || exp_code == TRUTH_ORIF_EXPR + || exp_code == TRUTH_OR_EXPR + || exp_code == TRUTH_XOR_EXPR + || exp_code == TRUTH_NOT_EXPR + || exp_code == EQ_EXPR + || exp_code == NE_EXPR + || exp_code == LE_EXPR + || exp_code == GE_EXPR + || exp_code == LT_EXPR + || exp_code == GT_EXPR) + warning_at (switch_cond_loc, OPT_Wswitch_bool, + "switch condition has boolean value"); if (!in_system_header_at (input_location) && (type == long_integer_type_node diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi index 8004da8..04e1c41 100644 --- gcc/doc/invoke.texi +++ gcc/doc/invoke.texi @@ -268,7 +268,7 @@ Objective-C and Objective-C++ Dialects}. -Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol -Wmissing-format-attribute @gol --Wswitch -Wswitch-default -Wswitch-enum -Wsync-nand @gol +-Wswitch -Wswitch-default -Wswitch-enum -Wswitch-bool -Wsync-nand @gol -Wsystem-headers -Wtrampolines -Wtrigraphs -Wtype-limits -Wundef @gol -Wuninitialized -Wunknown-pragmas -Wno-pragmas @gol -Wunsuffixed-float-constants -Wunused -Wunused-function @gol @@ -3822,6 +3822,12 @@ between @option{-Wswitch} and this option is that this option gives a warning about an omitted enumeration code even if there is a @code{default} label. +@item -Wswitch-bool +@opindex Wswitch-bool +@opindex Wno-switch-bool +Warn whenever a @code{switch} statement has an index of boolean type. +This warning is enabled by default for C programs. + @item -Wsync-nand @r{(C and C++ only)} @opindex Wsync-nand @opindex Wno-sync-nand diff --git gcc/testsuite/gcc.dg/pr60439.c gcc/testsuite/gcc.dg/pr60439.c index e69de29..26e7c25 100644 --- gcc/testsuite/gcc.dg/pr60439.c +++ gcc/testsuite/gcc.dg/pr60439.c @@ -0,0 +1,112 @@ +/* PR c/60439 */ +/* { dg-do compile } */ + +typedef _Bool bool; +extern _Bool foo (void); + +void +f1 (const _Bool b) +{ + switch (b) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; +} + +void +f2 (int a, int b) +{ + switch (a && b) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch ((bool) (a && b)) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch ((a && b) || a) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; +} + +void +f3 (int a) +{ + switch (!!a) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (!a) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; +} + +void +f4 (void) +{ + switch (foo ()) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; +} + +void +f5 (int a) +{ + switch (a == 3) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (a != 3) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (a > 3) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (a < 3) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (a <= 3) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (a >= 3) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (foo (), foo (), a >= 42) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (a == 3, a & 4, a ^ 5, a) + case 1: + break; +} + +void +f6 (bool b) +{ + switch (b) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (!b) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; + switch (b++) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; +} + +void +f7 (void) +{ + bool b; + switch (b = 1) /* { dg-warning "switch condition has boolean value" } */ + case 1: + break; +} + +void +f8 (int i) +{ + switch (i) + case 0: + break; + switch ((unsigned int) i) + case 0: + break; + switch ((bool) i) /* { dg-warning "switch condition has boolean value" } */ + case 0: + break; +}