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)
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.
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.
(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++.
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. 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
(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/
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.
(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 50992 has been marked as a duplicate of this bug. ***
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.
(In reply to Paolo Carlini from comment #9) > 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. I think it is safe to assume that anyone is welcome to follow up on this. See also comment #5. I personally would leave this as one of the EasyHacks for new contributors. It is easy to fix but not very important.
Proposing a new patch: https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01925.html
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
(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 ;)
> I leave the honor to you. Thanks ;). Fixed for GCC 6.
> 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.
(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.
Thanks for the info and such a quick response! I'll see if I can do the required legwork.
Backporting a new warning to a released branch isn't viable I'm afraid, it would make upgrading from 5.1 to 5.2 unsafe for some users.