From patchwork Sun Oct 23 11:31:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 78822 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp2101975qge; Sun, 23 Oct 2016 04:31:48 -0700 (PDT) X-Received: by 10.98.98.68 with SMTP id w65mr18492743pfb.121.1477222308852; Sun, 23 Oct 2016 04:31:48 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id bq3si3044185pab.70.2016.10.23.04.31.48 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 23 Oct 2016 04:31:48 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-439327-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-439327-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-439327-patch=linaro.org@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; q=dns; s=default; b=c/oGuUFpXhcovetR OMjW1S4GOrIqAs7GaCmDJ0VEd0J9Tmp6kC0cL1MmVEU+qaAi2h1D91JGUi7SGXeb Q/87fqY2MkRXMXbz+ThFU8kBsE89f3uNYVNKr3Jh/SVD+bye5N5Y9h0rPdwQ5s6F snEvMQN0kRpxmKPwtPemVY53eiM= 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:from :to:cc:subject:date:message-id:references:in-reply-to :content-type:mime-version; s=default; bh=oMpCl1qdSCKhCbJEInDuz8 rSl4o=; b=Y8C2Cbw9ETdmkZ9OT7hXfUJ/Tb/ITwTdgg5hjFH0TTZ/9BDjoQ+wEg tQVbbRuGkBwr79QFYcVCKH2zewB30VwpY2T9BPsy27f7R2gI+ZpQ1+/9PlqnH7rW a2u1C1liUp2YsTjd9pLrpmDgt3VG4CnCnXQm2UB+zNwisjBXN5AuA= Received: (qmail 11320 invoked by alias); 23 Oct 2016 11:31:35 -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 11290 invoked by uid 89); 23 Oct 2016 11:31:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.3 required=5.0 tests=AWL, BAYES_50, FREEMAIL_FROM, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=no version=3.3.2 spammy=locatio, 187812, UD:Wint-in-bool-context-3.c, 6169, 8 X-HELO: BAY004-OMC1S23.hotmail.com Received: from bay004-omc1s23.hotmail.com (HELO BAY004-OMC1S23.hotmail.com) (65.54.190.34) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 23 Oct 2016 11:31:23 +0000 Received: from EUR03-DB5-obe.outbound.protection.outlook.com ([65.54.190.60]) by BAY004-OMC1S23.hotmail.com over TLS secured channel with Microsoft SMTPSVC(7.5.7601.23008); Sun, 23 Oct 2016 04:31:21 -0700 Received: from AM5EUR03FT025.eop-EUR03.prod.protection.outlook.com (10.152.16.58) by AM5EUR03HT065.eop-EUR03.prod.protection.outlook.com (10.152.17.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.669.7; Sun, 23 Oct 2016 11:31:19 +0000 Received: from AM4PR0701MB2162.eurprd07.prod.outlook.com (10.152.16.59) by AM5EUR03FT025.mail.protection.outlook.com (10.152.16.157) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.669.7 via Frontend Transport; Sun, 23 Oct 2016 11:31:19 +0000 Received: from AM4PR0701MB2162.eurprd07.prod.outlook.com ([10.167.132.147]) by AM4PR0701MB2162.eurprd07.prod.outlook.com ([10.167.132.147]) with mapi id 15.01.0679.012; Sun, 23 Oct 2016 11:31:19 +0000 From: Bernd Edlinger To: Martin Sebor , Joseph Myers CC: "gcc-patches@gcc.gnu.org" , Jeff Law , Jason Merrill Subject: Re: [PATCH] Extend -Wint-in-bool-context to warn for multiplications Date: Sun, 23 Oct 2016 11:31:19 +0000 Message-ID: References: <9adf888a-b07c-2987-bd3d-27458eabaae5@gmail.com> In-Reply-To: authentication-results: gmail.com; dkim=none (message not signed) header.d=none; gmail.com; dmarc=none action=none header.from=hotmail.de; x-ms-exchange-messagesentrepresentingtype: 1 x-eopattributedmessage: 0 x-microsoft-exchange-diagnostics: 1; AM5EUR03HT065; 6:6O2Dr+FL5tFef6omfDxO9p2gPFZEKXbQ2flv8ux2ZW3GEfTf+vnq0mUp6csPsx836X5DawEKrzEKSW8uGniRrM1PjlW3dPuVfXDE2QxJgWNBW0KkdbXgsCfJSfUBgJrskU9kSpa6LwrQZpn/lNKA+70wjj9g2HecaHTR0Ch9reaiBzrDSKGcogsGPBISFQmIo70u3AkrDqDp7rwx7xrUbmwtMecYFCFTCuBM6Z+gh2f2s5PBCFIFoX1mnZnUd3OM2f22lXJ+Ds1hHHZvAqVugRH4JOy9tgbc8VhSgdjZGyl1kDv7UlAed8S/EN0HpjXG; 5:qhpMrMd3HZkdokwk1r5E9yp5tZ00VXvAfjhcL4nMPetMAB/wwjkg10aiu79z7mKTQDxTtswiEpkfg9YCGiy4vH0G4nWcY+YHlTti4YilI7rPp+z7kWJpgnV2TCOKWjljECFa7ARueR7+tJ/TxLZeow==; 24:5Yk9JZ7A11VrvaNEyZSjwwBgljdPQI1cJuOQ1bP1DgPGA5m/HmtjVzYEN+9LYHfT3CN4tDi08RcWz/+l6qWdfe4iS9DSekLzg3KeYOhmI5w=; 7:d07DEPwC2X2sRFzzWdSM5aLRNlLGYh8W8ebS7DzxdQLHqqZ0yKjkAUfTeoIBWJE8FJheUjAc2n76bZ7YnK/iXhHQAriGGhcOtbnNopD21hzfAnuZwoZ0OsYo0ls8e7t+IsCsjfouGrtfN9aZeE3XpH0yUR1JQDlydidxU21nnJTxjUYHhMqhq9ws4mzS85x7DZaxGmNRqO9idTW3MuuZTmvXVW+RZD2F6bw2oIVhV0w1jWqUaGu9DBM/kywXRh5nVyUlXlE13dqDSZaFhAqxjPLCdlZ/C/H3iuhRpx+LFBUBLslE/ETrT0+61LUfOloX9XDzC8WBVxfmeYopJNgIBFeQwovUZNArRSNUgZyJlSQ= x-forefront-antispam-report: EFV:NLI; SFV:NSPM; SFS:(10019020)(98900003); DIR:OUT; SFP:1102; SCL:1; SRVR:AM5EUR03HT065; H:AM4PR0701MB2162.eurprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; x-ms-office365-filtering-correlation-id: 719d60ac-8f32-474e-6b22-08d3fb381b32 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(1601124038)(1603103081)(1601125047); SRVR:AM5EUR03HT065; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(432015012)(102415321)(82015046); SRVR:AM5EUR03HT065; BCL:0; PCL:0; RULEID:; SRVR:AM5EUR03HT065; x-forefront-prvs: 0104247462 spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Oct 2016 11:31:19.3470 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Internet X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5EUR03HT065 On 10/22/16 08:52, Bernd Edlinger wrote: > On 10/22/16 04:17, Martin Sebor wrote: >> On 10/21/2016 04:37 PM, Joseph Myers wrote: >>> The quoting in the diagnostic should be %<&&%>, not '&&'. >> >> Presumably same for '*' (i.e., %<*%>). >> >> But I would actually suggest a somewhat more formal phrasing than >> "better use xxx here" such as "suggest %<&&%> instead" or something >> akin to what's already in place elsewhere in gcc.pot. >> > > Aehm, yes. That would be better then: > > > Index: c-common.c > =================================================================== > --- c-common.c (revision 241400) > +++ c-common.c (working copy) > @@ -3327,6 +3327,11 @@ > return c_common_truthvalue_conversion (location, > TREE_OPERAND (expr, 0)); > > + case MULT_EXPR: > + warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, > + "%<*%> in boolean context, suggest %<&&%> instead"); > + break; > + > case LSHIFT_EXPR: > /* We will only warn on signed shifts here, because the majority of > false positive warnings happen in code where unsigned arithmetic > > > I assume then I should adjust the warning a few lines below as well: > > warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, > "<< in boolean context, did you mean '<' ?"); > > Attached is the updated patch with those quotes fixed. I have now put the << and < in correct quotes, but left the ?: in the next two warnings unquoted: "?: using integer constants in boolean context, " "the expression will always evaluate to %" I copied that style from the warning about omitted middle operand of conditional expressions: "the omitted middle operand in ?: will always be %, suggest explicit " "middle operand" I think that could be explained because ?: is not really a keyword like <<, and is just a shorter phrase than "conditional expression". Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd. c-family: 2016-10-23 Bernd Edlinger * c-common.c (c_common_truthvalue_conversion): Warn for multiplications in boolean context. Fix the quoting of '<<' and '<' in the shift warning. gcc: 2016-10-23 Bernd Edlinger * doc/invoke.text (Wint-in-bool-context): Update documentation. * value-prof.c (stringop_block_profile): Fix a warning. testsuite: 2016-10-23 Bernd Edlinger * c-c++-common/Wint-in-bool-context-3.c: New test. Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 241437) +++ gcc/c-family/c-common.c (working copy) @@ -3327,6 +3327,11 @@ c_common_truthvalue_conversion (location_t locatio return c_common_truthvalue_conversion (location, TREE_OPERAND (expr, 0)); + case MULT_EXPR: + warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, + "%<*%> in boolean context, suggest %<&&%> instead"); + break; + case LSHIFT_EXPR: /* We will only warn on signed shifts here, because the majority of false positive warnings happen in code where unsigned arithmetic @@ -3336,7 +3341,7 @@ c_common_truthvalue_conversion (location_t locatio if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE && !TYPE_UNSIGNED (TREE_TYPE (expr))) warning_at (EXPR_LOCATION (expr), OPT_Wint_in_bool_context, - "<< in boolean context, did you mean '<' ?"); + "%<<<%> in boolean context, did you mean %<<%> ?"); break; case COND_EXPR: Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 241437) +++ gcc/doc/invoke.texi (working copy) @@ -6169,8 +6169,9 @@ of the C++ standard. @opindex Wno-int-in-bool-context Warn for suspicious use of integer values where boolean values are expected, such as conditional expressions (?:) using non-boolean integer constants in -boolean context, like @code{if (a <= b ? 2 : 3)}. Or left shifting in -boolean context, like @code{for (a = 0; 1 << a; a++);}. +boolean context, like @code{if (a <= b ? 2 : 3)}. Or left shifting of signed +integers in boolean context, like @code{for (a = 0; 1 << a; a++);}. Likewise +for all kinds of multiplications regardless of the data type. This warning is enabled by @option{-Wall}. @item -Wno-int-to-pointer-cast Index: gcc/value-prof.c =================================================================== --- gcc/value-prof.c (revision 241437) +++ gcc/value-prof.c (working copy) @@ -1878,12 +1878,12 @@ stringop_block_profile (gimple *stmt, unsigned int else { gcov_type count; - int alignment; + unsigned int alignment; count = histogram->hvalue.counters[0]; alignment = 1; while (!(count & alignment) - && (alignment * 2 * BITS_PER_UNIT)) + && (alignment <= UINT_MAX / 2 / BITS_PER_UNIT)) alignment <<= 1; *expected_align = alignment * BITS_PER_UNIT; gimple_remove_histogram_value (cfun, stmt, histogram); Index: gcc/testsuite/c-c++-common/Wint-in-bool-context-3.c =================================================================== --- gcc/testsuite/c-c++-common/Wint-in-bool-context-3.c (revision 0) +++ gcc/testsuite/c-c++-common/Wint-in-bool-context-3.c (working copy) @@ -0,0 +1,15 @@ +/* { dg-options "-Wint-in-bool-context" } */ +/* { dg-do compile } */ + +#define BITS_PER_UNIT 8 + +int foo (int count) +{ + int alignment; + + alignment = 1; + while (!(count & alignment) + && (alignment * 2 * BITS_PER_UNIT)) /* { dg-warning "boolean context" } */ + alignment <<= 1; + return alignment * BITS_PER_UNIT; +}