From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26605 invoked by alias); 2 Jun 2017 17:11:18 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 25151 invoked by uid 89); 2 Jun 2017 17:11:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=21=e2, 31f?= X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 02 Jun 2017 17:11:15 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7F72E15A2; Fri, 2 Jun 2017 10:11:17 -0700 (PDT) Received: from [10.2.207.43] (e104453-lin.cambridge.arm.com [10.2.207.43]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0222D3F58B; Fri, 2 Jun 2017 10:11:16 -0700 (PDT) Subject: Re: [PATCH] add more detail to -Wconversion and -Woverflow (PR 80731) To: Martin Sebor , Gcc Patch List References: <315bf6c8-04e5-01ac-7278-d93c04f83b0e@gmail.com> From: Renlin Li Message-ID: <59319C33.70702@foss.arm.com> Date: Fri, 02 Jun 2017 17:11:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <315bf6c8-04e5-01ac-7278-d93c04f83b0e@gmail.com> Content-Type: multipart/mixed; boundary="------------060203020407090608000405" X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00151.txt.bz2 This is a multi-part message in MIME format. --------------060203020407090608000405 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-length: 3136 Hi Martin, I noticed the following failures after your change r248431. FAIL: c-c++-common/Wfloat-conversion.c -Wc++-compat (test for warnings, line 42) FAIL: c-c++-common/Wfloat-conversion.c -Wc++-compat (test for warnings, line 43) It happens on arm target which is not a large_long_double target. The patch here add the missing target selector. After the change, those test won't checked in arm target. Here I have a simple fix to it. Okay to commit? gcc/testsuite/ChangeLog: 2017-06-02 Renlin Li * c-c++-common/Wfloat-conversion.c: Add large_long_double target selector to related line. And there is another failure: FAIL: gcc.dg/utf16-4.c (test for warnings, line 15) The warning message is slightly different from expected. utf16-4.c:10:15: warning: character constant too long for its type utf16-4.c:15:15: warning: conversion from 'long unsigned int' to 'char16_t {aka short unsigned int}' changes value from '410401' to '17185' On 18/05/17 01:04, Martin Sebor wrote: > While working on a new warning for unsafe conversion I noticed > that the existing warnings that diagnose these kinds of problems > are missing some useful detail. For example, given declarations > of an integer Type and an integer Constant defined in some header, > a C programmer who writes this declaration: > > Type x = Constant; > > might see the following: > > warning: overflow in implicit constant conversion [-Woverflow] > > To help the programmer better understand the problem and its impact > it would be helpful to mention the types of the operands, and if > available, also the values of the expressions. For instance, like > so: > > warning: overflow in conversion from ‘int’ to ‘T {aka signed char}’ changes value from > ‘123456789’ to ‘21’ [-Woverflow] > > The attached simple patch does just that. In making the changes > I tried to make the text of all the warnings follow the same > consistent wording pattern without losing any essential information > (e.g., I dropped "implicit" or "constant" because the implicit part > is evident from the code (no cast) and explicit conversions aren't > diagnosed, and because constant is apparent from the rest of the > diagnostic that includes its value. > > Besides adding more detail and tweaking the wording the patch > makes no functional changes (i.e., doesn't add new or remove > existing warnings). > > Martin > > PS While adjusting the tests (a painstaking process) it occurred > to me that these kinds of changes would be a whole lot easier if > dg-warning directives simply checked for "-Woption-name" rather > than some (often arbitrary) part of the warning text. It might > even be more accurate if the pattern happens to match the text > of two or more warnings controlled by different options. > > It's of course important to also exercise the full text of > the warnings, especially where additional detail is included > (like in this patch), but that can be done in a small subset > of tests. All the others that just verify the presence of > a warning controlled by a given option could use the simpler > approach. --------------060203020407090608000405 Content-Type: text/x-patch; name="tmp.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="tmp.diff" Content-length: 1253 diff --git a/gcc/testsuite/c-c++-common/Wfloat-conversion.c b/gcc/testsuite/c-c++-common/Wfloat-conversion.c index e9899bc..c33a2a6 100644 --- a/gcc/testsuite/c-c++-common/Wfloat-conversion.c +++ b/gcc/testsuite/c-c++-common/Wfloat-conversion.c @@ -39,8 +39,8 @@ void h (void) vfloat = vdouble; /* { dg-warning "conversion from .double. to .float. may change value" } */ ffloat (vlongdouble); /* { dg-warning "conversion from .long double. to .float. may change value" } */ vfloat = vlongdouble; /* { dg-warning "conversion from .long double. to .float. may change value" } */ - fdouble (vlongdouble); /* { dg-warning "conversion from .long double. to .double. may change value" } */ - vdouble = vlongdouble; /* { dg-warning "conversion from .long double. to .double. may change value" } */ + fdouble (vlongdouble); /* { dg-warning "conversion from .long double. to .double. may change value" "" { target large_long_double } } */ + vdouble = vlongdouble; /* { dg-warning "conversion from .long double. to .double. may change value" "" { target large_long_double } } */ fsi (3.1f); /* { dg-warning "conversion from .float. to .int. changes value" } */ si = 3.1f; /* { dg-warning "conversion from .float. to .int. changes value" } */ --------------060203020407090608000405--