From patchwork Fri Jun 2 17:11:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Renlin Li X-Patchwork-Id: 770526 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 3wfW2r3qkPz9s3T for ; Sat, 3 Jun 2017 03:11: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="RvCuywAY"; 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 :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=u0OWOa7LtgQWsA0Hl //EHs4GDC1yiilMe1oeg4FgrfrnLcgDxEOMwCGeKnrN0E96wybWE/1xx9FmyAYM8 M7SXucP1iYKhTdhMIXLDxJHLuLDkrM1M1Ou/42BiFnwQsrkg83vIiH55XlSkgLCb 0U2RZ0RYh3F/cye/SKT53UbITI= 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 :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=6lzWks/00sBHsWH2TtGl1fa 3wEY=; b=RvCuywAYzKqSy8HiSPECtsO42EY7ZMyIeLHLQNd9nRKkzICHOEWO7k1 2Nib4v88aMsITOsJ2yhsMeUVlMoQngZqKm6HdcBXgYq5Is8z8d5WD3MGJ+UJWri9 uzI8TpCu0CcblDrDs4sKF9mlz2tAfco9CD4a8WpIgb5aVQcnVkxs= 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-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 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, 2 Jun 2017 18:11:15 +0100 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> X-IsSubscribed: yes 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. 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" } */