[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[stevenj at alum dot mit.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #1 from stevenj at alum dot mit.edu 2011-05-11 00:01:32 UTC ---
PPS. I have reproduced this problem in gcc versions 3.3.6, 3.4.6, 4.1.3, 4.3.2,
and 4.6.0 running on an x86_64 Debian GNU/Linux system.

[Bug c/48956] New: -Wconversion should warn when a complex value is assigned to a real result

[stevenj at alum dot mit.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

           Summary: -Wconversion should warn when a complex value is
                    assigned to a real result
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: stevenj@alum.mit.edu


Consider the following C snippet:

  #include <complex.h>
  double foo(complex double x) {
     return x;
  }

This is valid C code: it assigns the complex "x" value to the real result of
"foo", which implicitly returns the real part of "x".  However, the implicit
conversion from complex to real has altered a value by discarding the imaginary
part of x.  Therefore, I would expect -Wconversion to issue a warning.

Compiling this with "gcc -c -Wconversion foo.c" issues no warning, however, nor
can I find any other -W option that causes a warning to be emitted.

Please consider issuing a warning when implicitly converting complex to real
(NOT the reverse) on -Wconversion.  (In fact, -Wconversion already does this in
Fortran as noted below.)

A warning for this type of conversion would be very helpful, since silently
discarding the imaginary part in this way often indicates a bug in my
experience.  (One can come up with many similar examples, e.g. writing sqrt(x)
rather than csqrt(x) currently silently succeeds for complex x, again
discarding the imaginary part.)  If the programmer meant to take the real part,
she probably would have written "creal(x)" explicitly.

Regards,
Steven G. Johnson

PS. Note -Wconversion already DOES emit a warning for conversion to complex to
real in gfortran.  e.g.

      subroutine foo(z)
      complex z
      real x
      x = z
      write(*,*) x
      return
      end

compiles without error with gfortran -c, but gfortran -c -Wconversion emits:

      x = z                                                             
         1
Warning: Conversion from COMPLEX(4) to REAL(4) at (1)

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #3 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2011-05-11 08:16:18 UTC ---
(In reply to comment #0)
> PS. Note -Wconversion already DOES emit a warning for conversion to complex to
> real in gfortran.  e.g.

Except for very few exceptions, the diagnostic machinery of Fortran is
independent of the diagnostic machinery of C/C++ (in some cases worse, in some
cases better). On the other hand, C/C++ share most of the code for Wconversion,
so implementing it for C will make it available for C++.

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.05.11 08:13:47
                 CC|                            |manu at gcc dot gnu.org
     Ever Confirmed|0                           |1
           Severity|normal                      |enhancement

--- Comment #2 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2011-05-11 08:13:47 UTC ---
The Wconversion code makes not attempt to identify conversion from/to complex
value but it is probably easy to implement. 

If you are really interested in this, I can give you some hints and advice and
you will probably get it ready for GCC 4.7, but I don't have free time to do it
myself, sorry.

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[stevenj at alum dot mit.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #4 from stevenj at alum dot mit.edu 2011-05-11 20:16:57 UTC ---
Created attachment 24230
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24230
patch to add a -Wconversion warning for complex -> real conversions

I believe the attached patch (against gcc 4.6.0) implements the feature I
requested.  It emits a warning for implicit conversions of complex types to
real (or integer) types.  It also emits a warning for similarly converting
complex constants, unless the imaginary part of the constant is zero in which
case it is treated as a real (or integer) constant given by the real part.

Note that complex->complex conversions are still not checked (e.g. no warning
is given in converting a complex double to a complex float, etcetera), but at
least I didn't add any regressions in that regard.

For example, when compiling the following test file:

#include <complex.h>
#include <math.h>

double foo(double complex z)
{
     complex double zc = 1 + 1i;
     complex double zc0 = 1 + 0i;

     float complex zf = z;

     double x = z;
     float xf = z;
     int i = z;

     double xc = 1 + 1i;
     double xc0 = 1 + 0i;
     int ic = 1.1 + 1.1i;
     int ic0 = 1.1 + 0.0i;

     return sqrt(z) + csqrt(z);
}

with "gcc -Wconversion -c", the unpatched gcc gives no warnings whereas the
patched gcc emits:

complex-Wconversion-tst.c: In function ‘foo’:
complex-Wconversion-tst.c:11:6: warning: conversion to ‘double’ from ‘complex
double’ may alter its value [-Wconversion]
complex-Wconversion-tst.c:12:6: warning: conversion to ‘float’ from ‘complex
double’ may alter its value [-Wconversion]
complex-Wconversion-tst.c:13:6: warning: conversion to ‘int’ from ‘complex
double’ may alter its value [-Wconversion]
complex-Wconversion-tst.c:15:6: warning: conversion to ‘double’ alters ‘complex
int’ constant value [-Wconversion]
complex-Wconversion-tst.c:17:6: warning: conversion to ‘int’ alters ‘complex
double’ constant value [-Wconversion]
complex-Wconversion-tst.c:18:6: warning: conversion to ‘int’ alters ‘double’
constant value [-Wconversion]
complex-Wconversion-tst.c:20:6: warning: conversion to ‘double’ from ‘complex
double’ may alter its value [-Wconversion]
complex-Wconversion-tst.c:20:21: warning: conversion to ‘double’ from ‘complex
double’ may alter its value [-Wconversion]

Note that two warnings are emitted for the last line: one for the implicit
conversion of z to a real number in sqrt(z), and the other for the implicit
conversion of the complex sqrt(z)+csqrt(z) to a real return value of foo(z).

Using creal(...) and/or explicit casts silences the warnings.  Note also that
"double xc0 = 1 + 0i;" correctly yields no warning.

As mentioned above, "float complex zf = z;" and similar complex->complex
conversions probably should emit a warning, but I have not implemented that. 
However, that is not a regression.

--SGJ

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |stevenj at alum dot mit.edu
                   |gnu.org                     |

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #5 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2011-05-11 21:57:54 UTC ---
(In reply to comment #4)
> Created attachment 24230 [details]
> patch to add a -Wconversion warning for complex -> real conversions
> 
> I believe the attached patch (against gcc 4.6.0) implements the feature I
> requested.  It emits a warning for implicit conversions of complex types to
> real (or integer) types.  It also emits a warning for similarly converting
> complex constants, unless the imaginary part of the constant is zero in which
> case it is treated as a real (or integer) constant given by the real part.

Nice! The patch looks almost ok but I cannot approve it. 

Some small issues:

* The code must follow GCC style, which is described in detail here
http://gcc.gnu.org/contribute.html#standards, but it is just easier too look
code around and follow the same style for braces, comments (they have to start
uppercase and end with a period), you have to add /* Fall through. */ to mark
deliberate fall-through in switches, etc.

* I would say (TREE_CODE (type) != COMPLEX_TYPE) rather than (!(TREE_CODE
(type) == COMPLEX_TYPE)). Even better, test first that == COMPLEX_TYPE, if so
you can return, otherwise, the zero imaginary part, else warning. Does that
sound right?

* You need to add the testcase to the testsuite. Look in
testsuite/gcc.dg/c-c++-common/Wconversion-real.c for an example. More info:
http://gcc.gnu.org/wiki/HowToPrepareATestcase

* Patches should be always be done against a recent SVN revision. I think this
patch should be safe to backport to GCC 4.6, but perhaps the release managers
may disagree (it does generate new warnings, which some people may find
bothersome if they use -Werror).

* Patches need to be bootstrapped and regression tested:
http://gcc.gnu.org/contribute.html#testing . There are scripts that mostly
automate this process. Take a look at contrib/ or this script of mine:
http://gcc.gnu.org/wiki/ManuelL%C3%B3pezIb%C3%A1%C3%B1ez?action=AttachFile&do=view&target=gccfarming

* Finally, patches should be sent to gcc-patches@gcc.gnu.org with a Changelog.
Take a look at the archives to have an idea of the proper way:
http://gcc.gnu.org/ml/gcc-patches/2011-05/

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[stevenj at alum dot mit.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #6 from stevenj at alum dot mit.edu 2011-05-12 22:33:59 UTC ---
Thanks, I was somewhat aware of the additional requirements for applying
patches to the official tree (probably I should also file a copyright
assignment), but I wanted to check with you that I was on the right track
first.   I'm a bit swamped this week, but should be able to do all this next
week.

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #7 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2011-05-12 23:06:14 UTC ---
(In reply to comment #6)
> Thanks, I was somewhat aware of the additional requirements for applying
> patches to the official tree (probably I should also file a copyright
> assignment), but I wanted to check with you that I was on the right track
> first.   I'm a bit swamped this week, but should be able to do all this next
> week.

You should definitely get the copyright assignment, but you do not need to wait
for it to submit the patch properly and get more feedback and approval.

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bratsinot at gmail dot com

--- Comment #8 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-03-24 19:14:49 UTC ---
*** Bug 50992 has been marked as a duplicate of this bug. ***

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #9 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Out of curiosity, what happened to this issue? (is still assigned to Steven)
Note that as far as I can see it's essentially a C issue, because the C++
front-end rejects this kind of code with an hard error.

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[maltsevm at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

Mikhail Maltsev <maltsevm at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |maltsevm at gmail dot com

--- Comment #11 from Mikhail Maltsev <maltsevm at gmail dot com> ---
Proposing a new patch: https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01925.html

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[miyuki at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #12 from miyuki at gcc dot gnu.org ---
Author: miyuki
Date: Fri May 15 18:02:50 2015
New Revision: 223223

URL: https://gcc.gnu.org/viewcvs?rev=223223&root=gcc&view=rev
Log:
PR c/48956

gcc/c-family/
* c-common.c (int_safely_convertible_to_real_p): Define.
(unsafe_conversion_p): Check conversions involving complex types.
(conversion_warning): Add new warning message for conversions which
discard imaginary component.
* c-common.h: (enum conversion_safety): Add new enumerator for such
conversions.

gcc/testsuite/
* gcc.dg/Wconversion-complex-c99.c: New test.
* gcc.dg/Wconversion-complex-gnu.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/Wconversion-complex-c99.c
    trunk/gcc/testsuite/gcc.dg/Wconversion-complex-gnu.c
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/c-family/c-common.h
    trunk/gcc/testsuite/ChangeLog

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #13 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
(In reply to Mikhail Maltsev from comment #12)

Great! Please do not forget to close the bug (using your @gcc.gnu.org account).
I leave the honor to you. Perhaps even worth it to add this to
https://gcc.gnu.org/gcc-6/changes.html ;)
>From gcc-bugs-return-486373-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri May 15 18:55:56 2015
Return-Path: <gcc-bugs-return-486373-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 77744 invoked by alias); 15 May 2015 18:55:56 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 76639 invoked by uid 48); 15 May 2015 18:55:52 -0000
From: "miyuki at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result
Date: Fri, 15 May 2015 18:55:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c
X-Bugzilla-Version: 4.6.0
X-Bugzilla-Keywords: diagnostic, easyhack
X-Bugzilla-Severity: enhancement
X-Bugzilla-Who: miyuki at gcc dot gnu.org
X-Bugzilla-Status: RESOLVED
X-Bugzilla-Resolution: FIXED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status cc resolution
Message-ID: <bug-48956-4-5SkVueYOeT@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-48956-4@http.gcc.gnu.org/bugzilla/>
References: <bug-48956-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-05/txt/msg01213.txt.bz2
Content-length: 553

https://gcc.gnu.org/bugzilla/show_bug.cgi?idH956

Mikhail Maltsev <miyuki at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |miyuki at gcc dot gnu.org
         Resolution|---                         |FIXED

--- Comment #14 from Mikhail Maltsev <miyuki at gcc dot gnu.org> ---
> I leave the honor to you.
Thanks ;).
Fixed for GCC 6.

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[matt at bitbashing dot io]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

Matt Kline <matt at bitbashing dot io> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matt at bitbashing dot io

--- Comment #15 from Matt Kline <matt at bitbashing dot io> ---
> Fixed for GCC 6.

May I ask why this is being deferred until GCC 6.x? I'll readily admit that I'm
not well-versed in the GCC release cycle, but this seems like a trivial
enhancement and would be useful immediately. My team has been bitten by these
silent conversions, and -Wconversion on Clang currently provides a similar
warning.

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[manu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #16 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
(In reply to Matt Kline from comment #15)
> > Fixed for GCC 6.
> 
> May I ask why this is being deferred until GCC 6.x? I'll readily admit that
> I'm not well-versed in the GCC release cycle, but this seems like a trivial
> enhancement and would be useful immediately. My team has been bitten by
> these silent conversions, and -Wconversion on Clang currently provides a
> similar warning.

GCC 6 is the next development release, which will be released early next year.
GCC 5 is the current stable release and, normally, only bugfixes are added to
it. If you wish to propose a backport, you'll need to write to the release
managers (for example, by writing to gcc@) and ask for permission, then you
need to do (or find someone to do) the backport, test it and commit it.

I'd rather have two new features in GCC 6 than one new feature in both GCC 5
and GCC 6, thus I (and likely others) prefer to dedicate my time to fixing
other things (and there is a lot of work to do:
https://gcc.gnu.org/wiki/GettingStarted#Basics:_Contributing_to_GCC_in_10_easy_steps)
rather than on back-porting new features.
>From gcc-bugs-return-490136-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed Jun 24 22:11:42 2015
Return-Path: <gcc-bugs-return-490136-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 61655 invoked by alias); 24 Jun 2015 22:11:42 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 61583 invoked by uid 55); 24 Jun 2015 22:11:36 -0000
From: "kkojima at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/66563] [4.9 Regression] ICE (segmentation fault) on sh4-linux-gnu
Date: Wed, 24 Jun 2015 22:11:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 4.9.2
X-Bugzilla-Keywords: ice-on-valid-code
X-Bugzilla-Severity: normal
X-Bugzilla-Who: kkojima at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 4.9.3
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-66563-4-m0pXhTOVhV@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-66563-4@http.gcc.gnu.org/bugzilla/>
References: <bug-66563-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-06/txt/msg02468.txt.bz2
Content-length: 470

https://gcc.gnu.org/bugzilla/show_bug.cgi?idf563

--- Comment #34 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Author: kkojima
Date: Wed Jun 24 22:11:04 2015
New Revision: 224925

URL: https://gcc.gnu.org/viewcvs?rev"4925&root=gcc&view=rev
Log:
PR target/66563
* [SH] Add a new operand to GOTaddr2picreg so to avoid CSE.  Modify caller
  of gen_GOTaddr2picreg.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.md

[Bug c/48956] -Wconversion should warn when a complex value is assigned to a real result

[matt at bitbashing dot io]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48956

--- Comment #17 from Matt Kline <matt at bitbashing dot io> ---
Thanks for the info and such a quick response! I'll see if I can do the
required legwork.