From patchwork Fri Nov 15 13:15:50 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joshua J Cogliati X-Patchwork-Id: 291570 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)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 756782C00A4 for ; Sat, 16 Nov 2013 00:16:20 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=efZ7VBUoVBNswGw1d rxwc9mijc35mdTBaWtI9eBM5Wu8km03m+qlUcCKbyZAjD23h/btP5IYz2CW818a9 Njx7Bx0XtXCA+o5KJlBxy5m2PTIOTbFMRvcN9x2lPMIkbYVk2hDxfqY2Xq0id+Xt 7RmU5IvoPMkFZlM95Qzdy9vsTk= 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=pGsS6L+VBa4JNLbal/nSpws ZHPY=; b=hM/ew9IHkcmVraUwMFrOzREhBFUm3CyjUvvQs+B9VVTFdsIPDPZZFMW FGSHpPHKPqj/uV7VDriWJ8B6VdsPr4QWPkxSLymGG/gtoXR2YW5suAi56io4NrsH XpFi9y/3ii30nvRKWvQto46C6lhp4+N2z0xntYAeJTyYImcRhAjg= Received: (qmail 557 invoked by alias); 15 Nov 2013 13:16:10 -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 479 invoked by uid 89); 15 Nov 2013 13:16:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=AWL, BAYES_50, FORGED_YAHOO_RCVD, FREEMAIL_FROM, RDNS_NONE, URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: nm23-vm6.bullet.mail.ne1.yahoo.com Received: from Unknown (HELO nm23-vm6.bullet.mail.ne1.yahoo.com) (98.138.91.116) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 15 Nov 2013 13:16:07 +0000 Received: from [98.138.226.180] by nm23.bullet.mail.ne1.yahoo.com with NNFMP; 15 Nov 2013 13:15:59 -0000 Received: from [98.138.226.60] by tm15.bullet.mail.ne1.yahoo.com with NNFMP; 15 Nov 2013 13:15:58 -0000 Received: from [127.0.0.1] by smtp211.mail.ne1.yahoo.com with NNFMP; 15 Nov 2013 13:15:58 -0000 X-Yahoo-SMTP: lppKq0OswBAEjZTK6IsLbGZHxiqn X-Rocket-Received: from localhost.localdomain (jrincayc@174.27.41.224 with ) by smtp211.mail.ne1.yahoo.com with SMTP; 15 Nov 2013 05:15:58 -0800 PST Message-ID: <52861E86.1000904@yahoo.com> Date: Fri, 15 Nov 2013 06:15:50 -0700 From: Joshua J Cogliati User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: Dodji Seketeli CC: "Joseph S. Myers" , gcc-patches@gcc.gnu.org, jason@redhat.com, manu@gcc.gnu.org Subject: Re: [PATCH, PR 53001] Re: Patch to split out new warning flag for floating point conversion References: <52554AA2.50706@yahoo.com> <525AA8E5.807@yahoo.com> <87zjqcp6jx.fsf@redhat.com> <52613001.9030606@yahoo.com> <52673B3E.3070006@yahoo.com> <526D1E17.80001@yahoo.com> <87hac1rd8d.fsf@redhat.com> <5273D0E3.1020503@yahoo.com> <878ux3nj2w.fsf@redhat.com> In-Reply-To: <878ux3nj2w.fsf@redhat.com> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 The patch is the same as before (but for newer revision 204343 so line numbers are a bit different), but the copyright assignment has been sorted out and the changelog has been modified as discussed by Dodji Seketeli. I think it is ready to be applied. Thanks for all the comments and suggestions. For gcc/c-family/ChangeLog: PR c/53001 Splitting out a -Wfloat-conversion from -Wconversion for conversions that lower floating point number precision or conversion from floating point numbers to integers * c-common.c (unsafe_conversion_p): Make this function return an enumeration with more detail. (conversion_warning): Use the new return type of unsafe_conversion_p to separately warn either about conversions that lower floating point number precision or about the other kinds of conversions. * c-common.h (enum conversion_safety): New enumeration. (unsafe_conversion_p): switching return type to conversion_safety enumeration. * c.opt: Adding new warning float-conversion and enabling it -Wconversion For gcc/ChangeLog: PR c/53001 Splitting out a -Wfloat-conversion from -Wconversion for conversions that lower floating point number precision or conversion from floating point numbers to integers * doc/invoke.texi: Adding documentation about -Wfloat-conversion For gcc/testsuite/ChangeLog PR c/53001 Splitting out a -Wfloat-conversion from -Wconversion for conversions that lower floating point number precision or conversion from floating point numbers to integers * c-c++-common/Wfloat-conversion.c: Copies relevant tests from c-c++-common/Wconversion-real.c, gcc.dg/Wconversion-real-integer.c and gcc.dg/pr35635.c into new testcase for conversions that are warned about by -Wfloat-conversion On 11/05/2013 05:12 AM, Dodji Seketeli wrote: > Sorry for my late reply on this. > > I just have one very small nit for one ChangeLog entry, expressed > below. If nobody objects in the next 48 hours, I'd say this is OK > to commit with the nit fixed. > > I am not seeing your name in the MAINTAINERS file. Have you filed > copyright assignment to the FSF and do you have it all sorted? > > Joshua J Cogliati writes: > >> Since part of it was to go into c-family (as per Joseph S. >> Myers's email) and parts are not in c-family, I split the >> changelog into three parts. I also changed the formatting of >> the changelog and patch as per Dodji's comments. The attached >> patch is the same as before, but the diff for Wfloat-conversion.c >> is as a new file, not as a copy of Wconversion-real.c. Thanks >> for reviewing it. If there is anything else that needs changing, >> please tell me. >> >> >> For gcc/c-family/ChangeLog: >> >> PR c/53001 Splitting out a -Wfloat-conversion from -Wconversion >> for conversions that lower floating point number precision or >> conversion from floating point numbers to integers * c-common.c >> (unsafe_conversion_p): Make this function return an enumeration >> with more detail. > > This is nicely formatted now, thank you. > >> (conversion_warning): Use the new return type of >> unsafe_conversion_p to separately warn either about conversions >> that lower floating point number precision or about the other >> kinds of conversions. * c-common.h: Adding conversion_safety >> enumeration. (unsafe_conversion_p): switching return type to >> conversion_safety enumeration. > > Here, this should be: > > * c-common.h (enum conversion_safety): New enumeration. > (unsafe_conversion_p): switching return type to conversion_safety > enumeration. > > OK to commit with the above changes, if you have your copyright > properly assigned to the FSF. > > Thank for working on this. It is appreciated. > -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.15 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJShh6BAAoJEFeOW05LP1faRt8H/iDxkB+KafIoVoBVuL/XinOc WhMvD+jV/fZFHey/O3WBCBPWeKP/5Gyvf+Z9YnioAnc3xAkil/SqMYDRKW+4obnJ x6XqeOHaq5TX0LLIhUDXX3TQWHYEzHTRmowqsedONUVz2nIsPoF52Q09wijsjXKq isllMKYj8TEVYTVN0NrlbhkkmG/6w+0NmSUuoDViHIdl/fX728CxpzgVzJLxjMQb wPNidbmeFomoTblM+imjc2vdqn+wiTwWY74gxV8oywv/J6otMfEodVSQcAQCSAyJ MvEhUFPHG+Ptt2KKPJVnAusUnm8TBdnrPV78uvtexGOW2mLMXR2YkdP/iAhmmgs= =M4aL -----END PGP SIGNATURE----- Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 204343) +++ gcc/c-family/c-common.c (working copy) @@ -2525,7 +2525,7 @@ shorten_binary_op (tree result_type, tre } /* Checks if expression EXPR of real/integer type cannot be converted - to the real/integer type TYPE. Function returns true when: + to the real/integer type TYPE. Function returns non-zero when: * EXPR is a constant which cannot be exactly converted to TYPE * EXPR is not a constant and size of EXPR's type > than size of TYPE, for EXPR type and TYPE being both integers or both real. @@ -2533,12 +2533,12 @@ shorten_binary_op (tree result_type, tre * EXPR is not a constant of integer type which cannot be exactly converted to real type. Function allows conversions between types of different signedness and - does not return true in that case. Function can produce signedness - warnings if PRODUCE_WARNS is true. */ -bool + can return SAFE_CONVERSION (zero) in that case. Function can produce + signedness warnings if PRODUCE_WARNS is true. */ +enum conversion_safety unsafe_conversion_p (tree type, tree expr, bool produce_warns) { - bool give_warning = false; + enum conversion_safety give_warning = SAFE_CONVERSION; /* is 0 or false */ tree expr_type = TREE_TYPE (expr); location_t loc = EXPR_LOC_OR_HERE (expr); @@ -2550,7 +2550,7 @@ unsafe_conversion_p (tree type, tree exp && TREE_CODE (type) == INTEGER_TYPE) { if (!real_isinteger (TREE_REAL_CST_PTR (expr), TYPE_MODE (expr_type))) - give_warning = true; + give_warning = UNSAFE_REAL; } /* Warn for an integer constant that does not fit into integer type. */ else if (TREE_CODE (expr_type) == INTEGER_TYPE @@ -2571,7 +2571,7 @@ unsafe_conversion_p (tree type, tree exp " constant value to negative integer"); } else - give_warning = true; + give_warning = UNSAFE_OTHER; } else if (TREE_CODE (type) == REAL_TYPE) { @@ -2580,7 +2580,7 @@ unsafe_conversion_p (tree type, tree exp { REAL_VALUE_TYPE a = real_value_from_int_cst (0, expr); if (!exact_real_truncate (TYPE_MODE (type), &a)) - give_warning = true; + give_warning = UNSAFE_REAL; } /* Warn for a real constant that does not fit into a smaller real type. */ @@ -2589,7 +2589,7 @@ unsafe_conversion_p (tree type, tree exp { REAL_VALUE_TYPE a = TREE_REAL_CST (expr); if (!exact_real_truncate (TYPE_MODE (type), &a)) - give_warning = true; + give_warning = UNSAFE_REAL; } } } @@ -2598,7 +2598,7 @@ unsafe_conversion_p (tree type, tree exp /* Warn for real types converted to integer types. */ if (TREE_CODE (expr_type) == REAL_TYPE && TREE_CODE (type) == INTEGER_TYPE) - give_warning = true; + give_warning = UNSAFE_REAL; else if (TREE_CODE (expr_type) == INTEGER_TYPE && TREE_CODE (type) == INTEGER_TYPE) @@ -2636,7 +2636,7 @@ unsafe_conversion_p (tree type, tree exp && int_fits_type_p (op1, c_common_signed_type (type)) && int_fits_type_p (op1, c_common_unsigned_type (type)))) - return false; + return SAFE_CONVERSION; /* If constant is unsigned and fits in the target type, then the result will also fit. */ else if ((TREE_CODE (op0) == INTEGER_CST @@ -2645,12 +2645,12 @@ unsafe_conversion_p (tree type, tree exp || (TREE_CODE (op1) == INTEGER_CST && unsigned1 && int_fits_type_p (op1, type))) - return false; + return SAFE_CONVERSION; } } /* Warn for integer types converted to smaller integer types. */ if (TYPE_PRECISION (type) < TYPE_PRECISION (expr_type)) - give_warning = true; + give_warning = UNSAFE_OTHER; /* When they are the same width but different signedness, then the value may change. */ @@ -2686,14 +2686,14 @@ unsafe_conversion_p (tree type, tree exp if (!exact_real_truncate (TYPE_MODE (type), &real_low_bound) || !exact_real_truncate (TYPE_MODE (type), &real_high_bound)) - give_warning = true; + give_warning = UNSAFE_OTHER; } /* Warn for real types converted to smaller real types. */ else if (TREE_CODE (expr_type) == REAL_TYPE && TREE_CODE (type) == REAL_TYPE && TYPE_PRECISION (type) < TYPE_PRECISION (expr_type)) - give_warning = true; + give_warning = UNSAFE_REAL; } return give_warning; @@ -2707,8 +2707,9 @@ conversion_warning (tree type, tree expr { tree expr_type = TREE_TYPE (expr); location_t loc = EXPR_LOC_OR_HERE (expr); + enum conversion_safety conversion_kind; - if (!warn_conversion && !warn_sign_conversion) + if (!warn_conversion && !warn_sign_conversion && !warn_float_conversion) return; switch (TREE_CODE (expr)) @@ -2735,7 +2736,12 @@ conversion_warning (tree type, tree expr case REAL_CST: case INTEGER_CST: - if (unsafe_conversion_p (type, expr, true)) + conversion_kind = unsafe_conversion_p (type, expr, true); + if (conversion_kind == UNSAFE_REAL) + warning_at (loc, OPT_Wfloat_conversion, + "conversion to %qT alters %qT constant value", + type, expr_type); + else if (conversion_kind) warning_at (loc, OPT_Wconversion, "conversion to %qT alters %qT constant value", type, expr_type); @@ -2754,7 +2760,12 @@ conversion_warning (tree type, tree expr } default: /* 'expr' is not a constant. */ - if (unsafe_conversion_p (type, expr, true)) + conversion_kind = unsafe_conversion_p (type, expr, true); + if (conversion_kind == UNSAFE_REAL) + warning_at (loc, OPT_Wfloat_conversion, + "conversion to %qT from %qT may alter its value", + type, expr_type); + else if (conversion_kind) warning_at (loc, OPT_Wconversion, "conversion to %qT from %qT may alter its value", type, expr_type); Index: gcc/c-family/c-common.h =================================================================== --- gcc/c-family/c-common.h (revision 204343) +++ gcc/c-family/c-common.h (working copy) @@ -688,6 +688,16 @@ struct visibility_flags unsigned inlines_hidden : 1; /* True when -finlineshidden in effect. */ }; +/* These enumerators are possible types of unsafe conversions. + SAFE_CONVERSION The conversion is safe + UNSAFE_OTHER Another type of conversion with problems + UNSAFE_SIGN Conversion between signed and unsigned integers + which are all warned about immediately, so this is unused + UNSAFE_REAL Conversions that reduce the precision of reals + including conversions from reals to integers + */ +enum conversion_safety { SAFE_CONVERSION = 0, UNSAFE_OTHER, UNSAFE_SIGN, UNSAFE_REAL }; + /* Global visibility options. */ extern struct visibility_flags visibility_options; @@ -741,7 +751,7 @@ extern tree c_common_signed_type (tree); extern tree c_common_signed_or_unsigned_type (int, tree); extern void c_common_init_ts (void); extern tree c_build_bitfield_integer_type (unsigned HOST_WIDE_INT, int); -extern bool unsafe_conversion_p (tree, tree, bool); +extern enum conversion_safety unsafe_conversion_p (tree, tree, bool); extern bool decl_with_nonnull_addr_p (const_tree); extern tree c_fully_fold (tree, bool, bool *); extern tree decl_constant_value_for_optimization (tree); Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 204343) +++ gcc/c-family/c.opt (working copy) @@ -387,6 +387,10 @@ Werror-implicit-function-declaration C ObjC RejectNegative Warning Alias(Werror=, implicit-function-declaration) This switch is deprecated; use -Werror=implicit-function-declaration instead +Wfloat-conversion +C ObjC C++ ObjC++ Var(warn_float_conversion) LangEnabledBy(C ObjC C++ ObjC++,Wconversion) +Warn for implicit type conversions that cause loss of floating point precision + Wfloat-equal C ObjC C++ ObjC++ Var(warn_float_equal) Warning Warn if testing floating point numbers for equality Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 204343) +++ gcc/doc/invoke.texi (working copy) @@ -262,7 +262,8 @@ Objective-C and Objective-C++ Dialects}. -Wpointer-arith -Wno-pointer-to-int-cast @gol -Wredundant-decls -Wno-return-local-addr @gol -Wreturn-type -Wsequence-point -Wshadow @gol --Wsign-compare -Wsign-conversion -Wsizeof-pointer-memaccess @gol +-Wsign-compare -Wsign-conversion -Wfloat-conversion @gol +-Wsizeof-pointer-memaccess @gol -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol -Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol @@ -4574,6 +4575,14 @@ value, like assigning a signed integer e integer variable. An explicit cast silences the warning. In C, this option is enabled also by @option{-Wconversion}. +@item -Wfloat-conversion +@opindex Wfloat-conversion +@opindex Wno-float-conversion +Warn for implicit conversions that reduce the precision of a real value. +This includes conversions from real to integer, and from higher precision +real to lower precision real values. This option is also enabled by +@option{-Wconversion}. + @item -Wsizeof-pointer-memaccess @opindex Wsizeof-pointer-memaccess @opindex Wno-sizeof-pointer-memaccess Index: gcc/testsuite/c-c++-common/Wfloat-conversion.c =================================================================== --- gcc/testsuite/c-c++-common/Wfloat-conversion.c (revision 0) +++ gcc/testsuite/c-c++-common/Wfloat-conversion.c (working copy) @@ -0,0 +1,58 @@ +/* Test for diagnostics for Wconversion for floating-point. */ + +/* { dg-do compile } */ +/* { dg-options "-std=c99 -Wfloat-conversion" { target c } } */ +/* { dg-options "-Wfloat-conversion" { target c++ } } */ +/* { dg-require-effective-target large_double } */ +/* { dg-require-effective-target int32plus } */ +/* { dg-require-effective-target double64plus } */ +#include + +float vfloat; +double vdouble; +long double vlongdouble; +int bar; + +void fsi (signed int x); +void fui (unsigned int x); +void ffloat (float f); +void fdouble (double d); +void flongdouble (long double ld); + +void h (void) +{ + unsigned int ui = 3; + int si = 3; + unsigned char uc = 3; + signed char sc = 3; + float f = 0; + double d = 0; + long double ld = 0; + + ffloat (3.1); /* { dg-warning "conversion to 'float' alters 'double' constant value" } */ + vfloat = 3.1; /* { dg-warning "conversion to 'float' alters 'double' constant value" } */ + ffloat (3.1L); /* { dg-warning "conversion to 'float' alters 'long double' constant value" } */ + vfloat = 3.1L; /* { dg-warning "conversion to 'float' alters 'long double' constant value" } */ + fdouble (3.1L); /* { dg-warning "conversion to 'double' alters 'long double' constant value" "" { target large_long_double } } */ + vdouble = 3.1L; /* { dg-warning "conversion to 'double' alters 'long double' constant value" "" { target large_long_double } } */ + ffloat (vdouble); /* { dg-warning "conversion to 'float' from 'double' may alter its value" } */ + vfloat = vdouble; /* { dg-warning "conversion to 'float' from 'double' may alter its value" } */ + ffloat (vlongdouble); /* { dg-warning "conversion to 'float' from 'long double' may alter its value" } */ + vfloat = vlongdouble; /* { dg-warning "conversion to 'float' from 'long double' may alter its value" } */ + fdouble (vlongdouble); /* { dg-warning "conversion to 'double' from 'long double' may alter its value" "" { target large_long_double } } */ + vdouble = vlongdouble; /* { dg-warning "conversion to 'double' from 'long double' may alter its value" "" { target large_long_double } } */ + + fsi (3.1f); /* { dg-warning "conversion to 'int' alters 'float' constant value" } */ + si = 3.1f; /* { dg-warning "conversion to 'int' alters 'float' constant value" } */ + fsi (3.1); /* { dg-warning "conversion to 'int' alters 'double' constant value" } */ + si = 3.1; /* { dg-warning "conversion to 'int' alters 'double' constant value" } */ + fsi (d); /* { dg-warning "conversion to 'int' from 'double' may alter its value" } */ + si = d; /* { dg-warning "conversion to 'int' from 'double' may alter its value" } */ + ffloat (INT_MAX); /* { dg-warning "conversion to 'float' alters 'int' constant value" } */ + vfloat = INT_MAX; /* { dg-warning "conversion to 'float' alters 'int' constant value" } */ + ffloat (16777217); /* { dg-warning "conversion to 'float' alters 'int' constant value" } */ + vfloat = 16777217; /* { dg-warning "conversion to 'float' alters 'int' constant value" } */ + + sc = bar != 0 ? 2.1 : 10; /* { dg-warning "conversion to 'signed char' alters 'double' constant value" } */ + uc = bar != 0 ? 2.1 : 10; /* { dg-warning "conversion to 'unsigned char' alters 'double' constant value" } */ +}