From patchwork Tue Sep 6 22:07:08 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 666758 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 3sTLLY4RH2z9sC7 for ; Wed, 7 Sep 2016 08:07:31 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=vFHCS0GR; dkim-atps=neutral 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:subject:date:message-id:references:in-reply-to:content-type :mime-version; q=dns; s=default; b=yoo8k72FvtLpS8GXp+kNPimstCaaC Is5fGx2WLqCjWI0pkyBezzrQGzfQng/7OieXcj2NB+WTVZBpUaRKdE33ita3CULg XFNku4stUfR2ggBAzzzQc2TDNQqM1Q7o5Ic+UWvTWOAhsJw9NOgnhJPqz8m+MATe Qmkalktk++5oOM= 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:subject:date:message-id:references:in-reply-to:content-type :mime-version; s=default; bh=IqCyg8FXpMLn6ydK2Bw2YVFtv9A=; b=vFH CS0GRNDwMc+PWAYoqJgCrX3jwtVjN47HOcEMD5gXJL+JITakfVmrqd3F24iTiZ6W 1PIEjLsAmlcXFxKloJRwlXObO0WhGari1gNz08FStXS6md1eSbTzWuSHd3nJp6WU d++CDj9awA22mU+BCYouo8R5tEnDOaSVza8KFl5g= Received: (qmail 23270 invoked by alias); 6 Sep 2016 22:07:24 -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 23255 invoked by uid 89); 6 Sep 2016 22:07:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 spammy=oliva, component_ref, LHS, COMPONENT_REF X-HELO: COL004-OMC2S6.hotmail.com Received: from col004-omc2s6.hotmail.com (HELO COL004-OMC2S6.hotmail.com) (65.55.34.80) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 06 Sep 2016 22:07:13 +0000 Received: from EUR01-HE1-obe.outbound.protection.outlook.com ([65.55.34.71]) by COL004-OMC2S6.hotmail.com over TLS secured channel with Microsoft SMTPSVC(7.5.7601.23008); Tue, 6 Sep 2016 15:07:12 -0700 Received: from HE1EUR01FT013.eop-EUR01.prod.protection.outlook.com (10.152.0.54) by HE1EUR01HT172.eop-EUR01.prod.protection.outlook.com (10.152.1.173) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA_P384) id 15.1.587.6; Tue, 6 Sep 2016 22:07:09 +0000 Received: from AM4PR0701MB2162.eurprd07.prod.outlook.com (10.152.0.56) by HE1EUR01FT013.mail.protection.outlook.com (10.152.0.160) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA_P384) id 15.1.587.6 via Frontend Transport; Tue, 6 Sep 2016 22:07:09 +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.0599.016; Tue, 6 Sep 2016 22:07:08 +0000 From: Bernd Edlinger To: "gcc-patches@gcc.gnu.org" , Jakub Jelinek , Jason Merrill , Joseph Myers Subject: [PATCHv2] Fix a few simple cases where -Wparentheses does not warn for omitted middle value Date: Tue, 6 Sep 2016 22:07:08 +0000 Message-ID: References: In-Reply-To: authentication-results: spf=softfail (sender IP is 10.152.0.56) smtp.mailfrom=hotmail.de; gcc.gnu.org; dkim=none (message not signed) header.d=none; gcc.gnu.org; dmarc=none action=none header.from=hotmail.de; received-spf: SoftFail (protection.outlook.com: domain of transitioning hotmail.de discourages use of 10.152.0.56 as permitted sender) x-ms-exchange-messagesentrepresentingtype: 1 x-eopattributedmessage: 0 x-forefront-antispam-report: CIP:10.152.0.56; IPV:NLI; CTRY:; EFV:NLI; SFV:NSPM; SFS:(10019020)(98900003); DIR:OUT; SFP:1102; SCL:1; SRVR:HE1EUR01HT172; H:AM4PR0701MB2162.eurprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; x-microsoft-exchange-diagnostics: 1; HE1EUR01HT172; 6:wSXJeiHGOjQq52PpdNipNK58Mse0fZ02VqnCY/Bu+OeQviFVFUUJIe0tM4u+fRMGSQzuLlduaPTT636W/s6f312tg8jOKO/YnrCh5Eqwjl/+JY8wbagjLhGW25ozFXRyze3B+QxwvlPM2UCDL5sighAWYuTI/5RhNYzVPQ9Khk1e91IZsTCk935jgHqWkJJRHFdbMDXU7eWE2/jXAZaBjVYVzEMzqRTEKP9B3P1OsXDbW2MKy3+HXxbSNtJbzN81O36pUAS7cVZdukTNsv6gXT+ezYNSBIaPNfPusn51os4GLCiqLvr7dfTQRXS8Iw+r; 5:dGwtRuzZ3D990dhlc+DbagSnNPuPQtASOpeeUQg3hv7g5tfW75dZ3tCkTZpVu0n+oTmd4zPvc1volwJr3Kcj8F9HUj2W2XUfNok1oCUumDl26XqT8eq2h9o0fYNOONocvsKvoHkaB4R4DagWeCwQ+g==; 24:x1/8blkdyZ/aLNgxdkiyJfzDgnHoMsNljybWr61z+bvoden+vExp4hrsxQXsUUfycZlncPLXyWaOdX1Yuc/RicvniN/MQwcsYaJbdIAh6YY=; 7:xdYHAqn/b80qijcHranOWADEw9uXS1wDV9FABZQ6ipAbvgH6XdCIAOO/XJxvggCo9DxSROkZK9RS2iWF6Fzg1tJ8WfMjydLPHIR5RH5K9DGFQ3KmFsXVCgxkE9vE0lxKbsf4sO7qlKxPit83k012Tqz/+dJAnJ6bSgLCeUdRVTDJ4Wg37XNeAcEwcDrvBJ8iOZkRYNJHgnxcBsYgKeW+ZaqFxKivUkVL/TDTvaZNnh+rre+RnC71tYbMsjCObV/Q x-ms-office365-filtering-correlation-id: ebe74927-7589-4998-1254-08d3d6a224be x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(1601124038)(1603103081)(1601125047); SRVR:HE1EUR01HT172; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(432015012)(102415321)(82015046); SRVR:HE1EUR01HT172; BCL:0; PCL:0; RULEID:; SRVR:HE1EUR01HT172; x-forefront-prvs: 0057EE387C spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Sep 2016 22:07:08.8031 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Internet X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1EUR01HT172 On 09/05/16 23:50, Bernd Edlinger wrote: > Hi, > > I've noticed that there is already a -Wparentheses warning for code like > > int y = x == 2 ?: 1 > > => warning: the omitted middle operand in ?: will always be 'true', > suggest explicit middle operand [-Wparentheses] > > But it is not emitted for code that uses bool, like: > > void foo(bool x) > { > int y = x ?: 1; > > and under C it is not emitted for compound expressions > that end with a comparison, like: > > int y = (i++,i==1) ?: 1; > > C++ is OK, but does only miss to warn on the bool data type. > > The attached patch should fix these warnings. > Well, reg-testing showed few test cases were broken, that made me aware of an issue with templates when the LHS of ?: is dependent. In that case the type is not available at the template declaration, and the warning cannot be generated at the declaration but only when the template is instantiated. The new patch fixes this, and a pre-existing issue, entered as PR 77496, when the type can not be implicitly converted to boolean. Boot-strapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd. gcc/c-family: 2016-09-06 Bernd Edlinger PR c++/77496 * c-common.c (warn_for_omitted_condop): Also warn for boolean data. gcc/c: 2016-09-06 Bernd Edlinger PR c++/77496 * c-parser.c (c_parser_conditional_expression): Pass the rightmost COMPOUND_EXPR to warn_for_omitted_condop. gcc/cp: 2016-09-06 Bernd Edlinger PR c++/77496 * call.c (build_conditional_expr_1): Call warn_for_omitted_condop. * class.c (instantiate_type): Look through the SAVE_EXPR. gcc/testsuite: 2016-09-06 Bernd Edlinger PR c++/77496 * c-c++-common/warn-ommitted-condop.c: Add more test cases. * g++.dg/ext/pr77496.C: New test. * g++.dg/warn/pr77496.C: New test. Index: gcc/c/c-parser.c =================================================================== --- gcc/c/c-parser.c (revision 240001) +++ gcc/c/c-parser.c (working copy) @@ -6423,16 +6423,20 @@ c_parser_conditional_expression (c_parser *parser, if (c_parser_next_token_is (parser, CPP_COLON)) { tree eptype = NULL_TREE; + tree e; middle_loc = c_parser_peek_token (parser)->location; - pedwarn (middle_loc, OPT_Wpedantic, + pedwarn (middle_loc, OPT_Wpedantic, "ISO C forbids omitting the middle term of a ?: expression"); - warn_for_omitted_condop (middle_loc, cond.value); if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR) { eptype = TREE_TYPE (cond.value); cond.value = TREE_OPERAND (cond.value, 0); } + e = cond.value; + while (TREE_CODE (e) == COMPOUND_EXPR) + e = TREE_OPERAND (e, 1); + warn_for_omitted_condop (middle_loc, e); /* Make sure first operand is calculated only once. */ exp1.value = c_save_expr (default_conversion (cond.value)); if (eptype) Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 240001) +++ gcc/c-family/c-common.c (working copy) @@ -10613,17 +10613,21 @@ fold_offsetof (tree expr) return convert (size_type_node, fold_offsetof_1 (expr)); } -/* Warn for A ?: C expressions (with B omitted) where A is a boolean +/* Warn for A ?: C expressions (with B omitted) where A is a boolean expression, because B will always be true. */ void -warn_for_omitted_condop (location_t location, tree cond) -{ - if (truth_value_p (TREE_CODE (cond))) - warning_at (location, OPT_Wparentheses, +warn_for_omitted_condop (location_t location, tree cond) +{ + /* In C++ template declarations it can happen that the type is dependent + and not yet known, thus TREE_TYPE (cond) == NULL_TREE. */ + if (truth_value_p (TREE_CODE (cond)) + || (TREE_TYPE (cond) != NULL_TREE && + TREE_CODE (TREE_TYPE (cond)) == BOOLEAN_TYPE)) + warning_at (location, OPT_Wparentheses, "the omitted middle operand in ?: will always be %, " "suggest explicit middle operand"); -} +} /* Give an error for storing into ARG, which is 'const'. USE indicates how ARG was being used. */ Index: gcc/cp/call.c =================================================================== --- gcc/cp/call.c (revision 240001) +++ gcc/cp/call.c (working copy) @@ -4653,9 +4653,12 @@ build_conditional_expr_1 (location_t loc, tree arg if (!arg2) { if (complain & tf_error) - pedwarn (loc, OPT_Wpedantic, + pedwarn (loc, OPT_Wpedantic, "ISO C++ forbids omitting the middle term of a ?: expression"); + if ((complain & tf_warning) && !truth_value_p (TREE_CODE (arg1))) + warn_for_omitted_condop (loc, arg1); + /* Make sure that lvalues remain lvalues. See g++.oliva/ext1.C. */ if (lvalue_p (arg1)) arg2 = arg1 = cp_stabilize_reference (arg1); Index: gcc/cp/class.c =================================================================== --- gcc/cp/class.c (revision 240001) +++ gcc/cp/class.c (working copy) @@ -8262,7 +8262,12 @@ instantiate_type (tree lhstype, tree rhs, tsubst_f return error_mark_node; } - /* There only a few kinds of expressions that may have a type + /* If we instantiate a template, and it is a A ?: C expression + with omitted B, look through the SAVE_EXPR. */ + if (TREE_CODE (rhs) == SAVE_EXPR) + rhs = TREE_OPERAND (rhs, 0); + + /* There are only a few kinds of expressions that may have a type dependent on overload resolution. */ gcc_assert (TREE_CODE (rhs) == ADDR_EXPR || TREE_CODE (rhs) == COMPONENT_REF Index: gcc/testsuite/c-c++-common/warn-ommitted-condop.c =================================================================== --- gcc/testsuite/c-c++-common/warn-ommitted-condop.c (revision 240001) +++ gcc/testsuite/c-c++-common/warn-ommitted-condop.c (working copy) @@ -1,11 +1,15 @@ /* { dg-options "-Wparentheses -ftrack-macro-expansion=0" } */ +#ifndef __cplusplus +#define bool _Bool +#endif + extern void f2 (int); -void bar (int x, int y, int z) +void bar (int x, int y, int z, bool b) { -#define T(op) f2 (x op y ? : 1) -#define T2(op) f2 (x op y ? 2 : 1) +#define T(op) f2 (x op y ? : 1) +#define T2(op) f2 (x op y ? 2 : 1) T(<); /* { dg-warning "omitted middle operand" } */ T(>); /* { dg-warning "omitted middle operand" } */ @@ -16,6 +20,8 @@ extern void f2 (int); T(||); /* { dg-warning "omitted middle operand" } */ T(&&); /* { dg-warning "omitted middle operand" } */ f2 (!x ? : 1); /* { dg-warning "omitted middle operand" } */ + f2 ((x,!x) ? : 1); /* { dg-warning "omitted middle operand" } */ + f2 ((x,y,!x) ? : 1); /* { dg-warning "omitted middle operand" } */ T2(<); /* { dg-bogus "omitted middle operand" } */ T2(>); /* { dg-bogus "omitted middle operand" } */ T2(==); /* { dg-bogus "omitted middle operand" } */ @@ -26,4 +32,5 @@ extern void f2 (int); T(*); /* { dg-bogus "omitted middle operand" } */ T(/); /* { dg-bogus "omitted middle operand" } */ T(^); /* { dg-bogus "omitted middle operand" } */ + f2 (b ? : 1); /* { dg-warning "omitted middle operand" } */ } Index: gcc/testsuite/g++.dg/ext/pr77496.C =================================================================== --- gcc/testsuite/g++.dg/ext/pr77496.C (revision 0) +++ gcc/testsuite/g++.dg/ext/pr77496.C (working copy) @@ -0,0 +1,21 @@ +// { dg-do compile } +// { dg-options "" } + +template +class z : x +{ +public: + bool zz () { return false; } + int f () { return zz ? : 1; } // { dg-error "cannot convert" } +}; + +class t +{ +}; + +int +main () +{ + z x; + return x.f (); +} Index: gcc/testsuite/g++.dg/warn/pr77496.C =================================================================== --- gcc/testsuite/g++.dg/warn/pr77496.C (revision 0) +++ gcc/testsuite/g++.dg/warn/pr77496.C (working copy) @@ -0,0 +1,21 @@ +// { dg-do compile } +// { dg-options "-Wparentheses" } + +template +class z : x +{ +public: + bool zz () { return false; } + int f () { return zz () ? : 1; } // { dg-warning "omitted middle operand" } +}; + +class t +{ +}; + +int +main () +{ + z x; + return x.f (); +}