From patchwork Fri Apr 18 11:50:13 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 340294 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 92E2D1400E5 for ; Fri, 18 Apr 2014 22:30:58 +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=T/9tRCn+qZ0DzQOvO 7b1OCPntFrTXx5sqhBV1wU/c9Gigys8KW/5/t+A3FQ9Bkg7EVXUWVweEy9Nc1jVy Fw5BrZZjNEytFsKeIwOWyjlPDRIbSKkkQuwOtqGQ197t9zfLomP23+rGjCnAK+eD a5aLO/e7sPhtN1U162DM/NKxZw= 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=wxWG/BLSszlQAhczxDABFgp hmMQ=; b=JPY9MHXmexoRVaVfKPa7Yt2kWZR274TeLll4NeareY/milGk5+6Rq2j hLNkdUEZ/KT3Uwshb/YrpRcJ5VUUWA5AAnkKMxftJz5RcCMPhOWA4K4oYc9ex5ba NYEwySlCDokrb7eNOMs2TcIiCePs2b6V9pbBY/oofgKf4HS0kdR0= Received: (qmail 24400 invoked by alias); 18 Apr 2014 12:30:52 -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 24386 invoked by uid 89); 18 Apr 2014 12:30:51 -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 12:30:50 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s3ICUmxI010482 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 18 Apr 2014 08:30:48 -0400 Received: from redhat.com (ovpn-116-31.ams2.redhat.com [10.36.116.31]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s3IBoDNM003211 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Fri, 18 Apr 2014 07:50:16 -0400 Date: Fri, 18 Apr 2014 13:50:13 +0200 From: Marek Polacek To: Steven Bosscher Cc: GCC Patches , "Joseph S. Myers" Subject: Re: [C PATCH] Warn if switch has boolean value (PR c/60439) Message-ID: <20140418115013.GK20332@redhat.com> References: <20140418053021.GH20332@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) On Fri, Apr 18, 2014 at 01:20:59PM +0200, Steven Bosscher wrote: > On Fri, Apr 18, 2014 at 7:30 AM, Marek Polacek wrote: > > + 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) > > Is there a TREE_CODE_CLASS or a #define for this? Good question. I was looking for something nicer and found nothing, no T_C_C or #define. But now when I'm looking again, I found truth_value_p... Thanks. 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..1b37f83 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -9344,6 +9344,15 @@ c_start_case (location_t switch_loc, else { tree type = TYPE_MAIN_VARIANT (orig_type); + tree e = exp; + + while (TREE_CODE (e) == COMPOUND_EXPR) + e = TREE_OPERAND (e, 1); + + if (TREE_CODE (type) == BOOLEAN_TYPE + || truth_value_p (TREE_CODE (e))) + 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; +}