Summary: | Wrong and misleading warning encourages writing non-portable code | ||
---|---|---|---|
Product: | gcc | Reporter: | bagnara |
Component: | c | Assignee: | Not yet assigned to anyone <unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | enhancement | CC: | eggert, gcc-bugs, giovannibajo, manu, qrczak, sthoenna, vincent-gcc |
Priority: | P2 | Keywords: | diagnostic |
Version: | 3.4.0 | ||
Target Milestone: | 4.3.0 | ||
Host: | Target: | ||
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | 2005-12-28 06:05:58 | |
Attachments: | Shows how GCC (3.4.2) is inconsistent concerning the mentioned warning |
Description
bagnara
2003-11-08 13:42:20 UTC
The warning is correct for this implementation and since you are compiling with this implementation, you can recieve warnings (yes it would be better if it was enabled by a - W* but I think it is good warning no matter what). To disable the warning use -w (notice the lower case w). (In reply to comment #1) > The warning is correct for this implementation and since you are compiling with this > implementation, you can recieve warnings (yes it would be better if it was enabled by a - > W* but I think it is good warning no matter what). To disable the warning use -w (notice > the lower case w). If I understand correctly, -w inhibits all warning messages, which is certainly not what I call for. The fact that, when producing code for platforms where CHAR_BIT == 8, the test is redundant is a matter for the optimizer (who can omit the test), not something the user should be warned about. In general, I believe target-dependent warnings should default to disabled. By the way, are there other target-dependent warnings in GCC? I mean that this is the first time I meet one (unless I am missing something). Now try with void* foo(void* c) { if (c <= (void*) 0xFFFFFFFF) return 0; else return c; } and you will see that (notice -W -Wall) g++ -W -Wall -c t.cc -O3 -fomit-frame-pointer -S does not produce any warning. Yet, the produced code is, .globl _Z3fooPv .type _Z3fooPv, @function _Z3fooPv: .LFB3: xorl %eax, %eax ret as expected. Would you advocate a warning also in this case? There are a large number of target-dependent (and sizeof dependent) warnings in GCC. GCC also warns about "integer overflow in expression" and fp. Since GCC is warning based on the target you are using, I am closing as will not fix. (In reply to comment #4) > Since GCC is warning based on the target you are using, I am closing as will not fix. In my opinion, this is a mistake: people wishing to write 100% portable code should not be forced to switch all warnings off in order to get rid of this useless warning (yes, useless, because there is nothing sensible the sensible programmer can do about it). Warnings such as this one should be issued only when specifically requested. At the very least, it should be possible to selectively switch off this kind of warnings. But closing the bug report as you are doing (without even addressing my question "why then there is no such a warning for void*, not even with -W -Wall?") will not make the problem go away. The problem is there. I don't think this anxiety to close bug reports before the issue has been studied and discussed in depth is going to help. I agree with Robert on this point: we shouldn't get this warning _when not giving any flag_. IMHO, the warning is quite valid and tells the programmer about something that he might not have considered, but in case he has it is pointless to issue a warning that is issued even without any of the -W... flags and without the possibility to selectively switch it off. Let's keep this open until we reach some kind of consensus whether it is possible to implement this or not. W. Subject: Re: Wrong and misleading warning encourages writing non-portable code "bangerth at dealii dot org" <gcc-bugzilla@gcc.gnu.org> writes: | Let's keep this open until we reach some kind of consensus whether it | is possible to implement this or not. I would suggest "enhancement request". -- Gaby Subject: Re: Wrong and misleading warning encourages writing non-portable code On Fri, 21 Nov 2003, bangerth at dealii dot org wrote: > I agree with Robert on this point: we shouldn't get this warning > _when not giving any flag_. IMHO, the warning is quite valid and tells As I previously pointed out <http://gcc.gnu.org/ml/gcc-patches/2003-03/msg01954.html>. Given this sort of portability issue, a separate option (maybe in -Wextra, but probably not in -Wall), such as -Wcomparison-fixed, is appropriate for these warnings and those already in -Wextra I mentioned in that message. To my claim that the warning "comparison is always false due to limited range of data type" should be disabled by default, I would like to add the fact that GCC is highly inconsistent in this respect. In fact, as the attached file v.cc shows, the warning is given only for char and short whereas it is not given for int, long and long long. Created attachment 7177 [details]
Shows how GCC (3.4.2) is inconsistent concerning the mentioned warning
Lines 56 and 57 cause the instantiations with char and short: warning given.
Lines 58, 59 and 60, for which no warning is given, cause the instantiations
with int, long and long long.
Here is what happens with GCC 3.4.2:
$ g++ -c v.cc
v.cc: In function `int assign_c(To&, From) [with To = char, From = char]':
v.cc:56: instantiated from here
v.cc:39: warning: comparison is always false due to limited range of data type
v.cc:41: warning: comparison is always false due to limited range of data type
v.cc: In function `int assign_c(To&, From) [with To = short int, From = short
int]':
v.cc:57: instantiated from here
v.cc:39: warning: comparison is always false due to limited range of data type
v.cc:41: warning: comparison is always false due to limited range of data type
Besides the consistency issue, which is a bug and should be reported elsewhere, I do not understand how this encourage writing non-portable code. On the contrary, it does *help* it. Say that you have a program which assumes that char is 16bits, and you port to an architecture which has chars with 8bits. Instead of silently failing, you will get warnings like this one which would help you catching problems. I do not think you provided a good rationale of why we should disable such a warning. Could you please describe your real-life problem about portability and this warning? Subject: Re: Wrong and misleading warning encourages writing non-portable code On Mon, 20 Sep 2004, giovannibajo at libero dot it wrote: > Besides the consistency issue, which is a bug and should be reported elsewhere, > I do not understand how this encourage writing non-portable code. On the > contrary, it does *help* it. A program has some data in a type such as char or uid_t which needs to be stored in an externally defined structure, or transmitted by an externally defined protocol. This external definition imposes a limit of (say 255) on the value held in (say) unsigned char. A portable program checks that the data is within the defined range before storing or transmitting it. Some checks are redundant on a particular system, so receive these warnings; if the warnings are to be silenced, that means removing checks that are not dead code on other systems. I have encountered such problems (warnings that cannot effectively be silenced) in real code, both with system typedefs such as uid_t and user typedefs that may be configured to be a type such as unsigned char or a wider type. My comment #8 still applies: this does not even belong in -Wall. (In reply to comment #1) > The warning is correct for this implementation and since you are compiling with this > implementation, you can recieve warnings (yes it would be better if it was enabled by a - > W* but I think it is good warning no matter what). To disable the warning use -w (notice > the lower case w). I hope that there is a general consensus about the fact that the presence of expected warnings not avoidable on compilation log is a bad thing. This lower the attention threshold of programmers and contributors about true problems and reduce the readability of compilation log. I suppose that the availability of -Werror is derived from this same reasoning. Taken for granted that, I think that the presence of warning options like -wWARNING-NAME to disable specific warnings could solve the problem in a per file fashion. Better than that the availability of something like #pragma expected-warning line WARNING-NAME might remove the warning generated by the following line labeling it as checked, expected and/or unavoidable. Subject: Re: Wrong and misleading warning encourages writing non-portable code On Tue, 21 Sep 2004, abramobagnara at tin dot it wrote: > Better than that the availability of something like > #pragma expected-warning line WARNING-NAME > might remove the warning generated by the following line labeling it as checked, > expected and/or unavoidable. That this is desired is generally agreed. It's just that no-one has yet produced a design for it and still had time left to implement it after all the arguing over that design. DJ produced a design <http://gcc.gnu.org/ml/gcc/2004-06/msg01188.html> (and thread). This led to a mechanical patch adding a parameter 0 to every call to warning() <http://gcc.gnu.org/ml/gcc-patches/2004-07/msg02229.html>; it wasn't applied, nor was there any followup. Previously, Stan Shebs gave a proposal <http://gcc.gnu.org/ml/gcc/2003-01/msg01065.html> (and thread). There have been various other such discussions. <http://gcc.gnu.org/ml/gcc/2000-06/msg00639.html> has a different proposal. <http://gcc.gnu.org/ml/gcc/1998-09/msg00353.html> included, I think uniquely, a patch, but there were objections and it was reverted within 24 hours. Implementing such a feature is not hard. I could probably do a plausible implementation - that did not require all calls to diagnostic functions to be converted at once, rather allowing the conversion to having individual warnings controllable to be a gradual one - in a few days (having enable-mapped-location unconditionally on would help though), as could many other people. Producing a consensus that any given approach is right, or even that any given approach is not unacceptable, is the hard part. *** Bug 20550 has been marked as a duplicate of this bug. *** > Better than that the availability of something like
> #pragma expected-warning line WARNING-NAME
> might remove the warning generated by the following line labeling it as checked,
> expected and/or unavoidable.
This would not help in my case because it's a regular type-generic C macro, not
generated code. The line number of the macro definition is not stable (changes
as surrounding code evolves), and it makes no sense to mark all its invocations.
From my point of view a perfect solution would be making this obvious workaround
working:
int test(int x) {
if ((long long)x <= 0x123456789ABCLL) return 1;
else return 0;
}
There should be no warning if the lhs is cast to the wider type. GCC seems to be
inferring that the range of possible values of (long long)x is the range of int.
The runtime comparison is eliminated, which is good; this implies that the
warning should not be tied to the comparison being eliminated.
I'm not judging whether the warning should be emitted at all. Maybe it should be
removed altogether; after all, it's not guaranteed to be redundant on all
platforms. It definitely should not be emitted by default, especially without
any warning options, especially if the cast suggests that the programmer is
aware of it.
*** Bug 20573 has been marked as a duplicate of this bug. *** *** Bug 22178 has been marked as a duplicate of this bug. *** Just a small update. On one of our projects we have now thousands of warnings on the test "x < 0" for the function below, when Type is instantiated to an unsigned integral type: template <typename Type> inline Result sgn_generic(const Type& x) { if (x < 0) return V_LT; if (x > 0) return V_GT; return V_EQ; } The net result is that some of us started using "-w" or stopped paying attention to warnings, and, as a consequence bugs are creeping in at a much increased rate. Please, give us a way to at least turn off that warning. (In reply to comment #19) > Just a small update. On one of our projects we have now thousands of warnings > on the test "x < 0" for the function below, when Type is instantiated to an > unsigned integral type: > > template <typename Type> > inline Result > sgn_generic(const Type& x) { > if (x < 0) > return V_LT; > if (x > 0) > return V_GT; > return V_EQ; > } A side remark, about a weird workaround I had to use in the past in the library (yes the problem is real, I agree, not sure about the best solution, however). Would boil down to something like (mosulo stupid typos ;) if (x > 0) return V_GT; else if (x) return V_LT; return V_EQ; Seems correct to me and doesn't incur in any warning. Subject: Bug 12963 Author: manu Date: Sun May 20 20:29:55 2007 New Revision: 124875 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124875 Log: 2007-05-20 Manuel Lopez-Ibanez <manu@gcc.gnu.org> PR middle-end/7651 PR c++/11856 PR c/12963 PR c/23587 PR other/29694 * c.opt (Wtype-limits): New. * doc/invoke.texi (Wtype-limits): Document it. (Wextra): Enabled by -Wextra. * c-opts.c (c_common_post_options): Enabled by -Wextra. * c-common.c (shorten_compare): Warn with Wtype-limits. testsuite/ * gcc.dg/compare6.c: Replace Wall with Wtype-limits. * gcc.dg/Wtype-limits.c: New. * gcc.dg/Wtype-limits-Wextra.c: New. * gcc.dg/Wtype-limits-no.c: New. * g++.dg/warn/Wtype-limits.C: New. * g++.dg/warn/Wtype-limits-Wextra.C: New. * g++.dg/warn/Wtype-limits-no.C: New. Added: trunk/gcc/testsuite/g++.dg/warn/Wtype-limits-Wextra.C trunk/gcc/testsuite/g++.dg/warn/Wtype-limits-no.C trunk/gcc/testsuite/g++.dg/warn/Wtype-limits.C trunk/gcc/testsuite/gcc.dg/Wtype-limits-Wextra.c trunk/gcc/testsuite/gcc.dg/Wtype-limits-no.c trunk/gcc/testsuite/gcc.dg/Wtype-limits.c Modified: trunk/gcc/ChangeLog trunk/gcc/c-common.c trunk/gcc/c-opts.c trunk/gcc/c.opt trunk/gcc/doc/invoke.texi trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/compare6.c The diverse warnings of the type "always true/false because of range of data type" have been grouped under -Wtype-limits that is enabled by -Wextra (and not by -Wall). The warning can be disabled by using -Wno-type-limits. Thus, fixed in GCC 4.3. |