Re: [c++] switch ( enum ) vs. default statment.
On Jan 29, 2007, Manuel López-Ibáñez [EMAIL PROTECTED] wrote: * You can add a return 0 or an exit(1) at the end of the function or in a default label. Since in your case the code is unreachable, the optimiser may remove it or it will never be executed. But this would generate additional code for no useful purpose. See Ralf Baechle's posting with Subject: False ‘noreturn’ function does return warnings. /me thinks it would make sense for us to add a __builtin_unreachable() that would indicate to GCC that no further action is needed from that point on. A command-line flag could then tell whether GCC should generate abort() for such cases, or take a more general undefined behavior approach without generating additional code to that end. Meanwhile, there's __builtin_trap() already, and Ralf might use that even to remove the asm volatile, and Paweł could use it in a default: label. It's still worse than a __builtin_assume(e == X || e == Y), but it's probably much simpler to implement. But then, __builtin_unreachable() might very well be implemented as __builtin_assume(0). -- Alexandre Oliva http://www.lsd.ic.unicamp.br/~oliva/ FSF Latin America Board Member http://www.fsfla.org/ Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org} Free Software Evangelist [EMAIL PROTECTED], gnu.org}
Re: [c++] switch ( enum ) vs. default statment.
On Tue, Feb 06, 2007 at 04:44:50PM -0200, Alexandre Oliva wrote: * You can add a return 0 or an exit(1) at the end of the function or in a default label. Since in your case the code is unreachable, the optimiser may remove it or it will never be executed. But this would generate additional code for no useful purpose. See Ralf Baechle's posting with Subject: False ‘noreturn’ function does return warnings. /me thinks it would make sense for us to add a __builtin_unreachable() that would indicate to GCC that no further action is needed from that point on. A command-line flag could then tell whether GCC should generate abort() for such cases, or take a more general undefined behavior approach without generating additional code to that end. Meanwhile, there's __builtin_trap() already, and Ralf might use that even to remove the asm volatile, and Paweł could use it in a default: label. It's still worse than a __builtin_assume(e == X || e == Y), but it's probably much simpler to implement. But then, __builtin_unreachable() might very well be implemented as __builtin_assume(0). The Linux/MIPS BUG() macro uses a break 512 instruction while __builtin_trap() generates a plain break that is break 0 instruction. Aside of that __builtin_trap() would be fine ... Ralf
Re: [c++] switch ( enum ) vs. default statment.
On 06/02/07, Alexandre Oliva [EMAIL PROTECTED] wrote: On Jan 29, 2007, Manuel López-Ibáñez [EMAIL PROTECTED] wrote: * You can add a return 0 or an exit(1) at the end of the function or in a default label. Since in your case the code is unreachable, the optimiser may remove it or it will never be executed. But this would generate additional code for no useful purpose. See Ralf Baechle's posting with Subject: False 'noreturn' function does return warnings. GCC already generates additional code even if no explicit 'default' or 'return' is added. :-/ See the original PR c++/28236. Cheers, Manuel.
Re: [c++] switch ( enum ) vs. default statment.
On Tue, Feb 06, 2007 at 08:00:29PM +, Ralf Baechle wrote: On Tue, Feb 06, 2007 at 04:44:50PM -0200, Alexandre Oliva wrote: Meanwhile, there's __builtin_trap() already, and Ralf might use that even to remove the asm volatile, and Paweł could use it in a default: label. It's still worse than a __builtin_assume(e == X || e == Y), but it's probably much simpler to implement. But then, __builtin_unreachable() might very well be implemented as __builtin_assume(0). The Linux/MIPS BUG() macro uses a break 512 instruction while __builtin_trap() generates a plain break that is break 0 instruction. Aside of that __builtin_trap() would be fine ... __builtin_break_512() ?
Re: [c++] switch ( enum ) vs. default statment.
Mark Mitchell wrote:
For GCC, what happens (though we need not document it) is that the
value is converted to the underlying type -- but not masked down
to { 0, 1 }, because that masking would be costly.
So, ((int) e == 7) may be true after the assignment above.
(Perhaps, in some modes GCC may optimize away the assignment because
it will know that e cannot be 7,
hhmm :-) this can lead to another felix-like war ;-)
I DID NOT WRITE THE BROKEN CODE. (...)
IT DOES NOT MATTER WHAT THE ... STANDARD SAYS.
You broke code, people are suffering damage. Now revert it.
So, now, what should we do about the warning? I think there are good
arguments in both directions. On the one hand, portable programs
cannot assume that assigning out-of-range values to e does anything
predictable, so portable programs should never do that. So, if you've
written the entire program and are sure that it's portable, you don't
want the warning.
and this is exactly what i'm expecting - no warnings for pure c++ code.
moreover such warnings-on-valid code make -Werror unusable with -Wall.
On the other hand, if you are writing a portable library designed
to be used with other people's programs, you might every well want
the warning -- because you can't be sure that they're not going to
pass 7 in as the value of e, and you may want to be robust in
the face of this *unspecified* behavior.
sorry, i don't care about unspecified/undefined behavior triggered
by users glitches. it's not a problem of my library.
In practice, this warning from GCC is keyed off what it thinks the
effective range of E is. If it starts assuming that e can only
have the values { 0, 1 }, it will also stop warning about the missing
case. It would be hard to stop emitting the warning without making
that assumption, and it may not be easy to make the assumption, but
still avoid the expensive masking operations.
i'm not familiar with gcc internals. for me enum looks like a set
and detecting missed `case' is a basic operation from set theory.
finally, if PR/28236 is valid then (according to the current discussion)
it should be marked as suspended or resolved/wontfix with appropriate
note.
BR,
Pawel.
Re: [c++] switch ( enum ) vs. default statment.
On 29/01/07, Paweł Sikora [EMAIL PROTECTED] wrote:
Mark Mitchell wrote:
So, now, what should we do about the warning? I think there are good
arguments in both directions. On the one hand, portable programs
cannot assume that assigning out-of-range values to e does anything
predictable, so portable programs should never do that. So, if you've
written the entire program and are sure that it's portable, you don't
want the warning.
and this is exactly what i'm expecting - no warnings for pure c++ code.
moreover such warnings-on-valid code make -Werror unusable with -Wall.
We do emit a lot of warnings for pure and valid c++ code. See
-Wunused-*, see -Wparentheses, see -Wuninitialized, see
-Wchar-subscripts, see -Wsign-compare, etc. Some could be avoided if
we had dataflow information in the front-end, but others are given
because there is a serious suspicious that something could go wrong
despite the code being valid and pure C++.
On the other hand, if you are writing a portable library designed
to be used with other people's programs, you might every well want
the warning -- because you can't be sure that they're not going to
pass 7 in as the value of e, and you may want to be robust in
the face of this *unspecified* behavior.
sorry, i don't care about unspecified/undefined behavior triggered
by users glitches. it's not a problem of my library.
You have many ways to work-around it if you are 100% sure you are the
only one using that function and that you will never pass anything
different from that enum type.
* You can add a return 0 or an exit(1) at the end of the function or
in a default label. Since in your case the code is unreachable, the
optimiser may remove it or it will never be executed.
* You can use the appropriate -Wno-X option to disable the warning.
* You can use the appropriate #pragma GCC diagnostics ignored -WX to
disable the warning.
* You can use #pragma GCC diagnostic warning -WX to emit just a
warning despite using -Werror in the command-line. You do the same
with -Wno-error=X.
In practice, this warning from GCC is keyed off what it thinks the
effective range of E is. If it starts assuming that e can only
have the values { 0, 1 }, it will also stop warning about the missing
case. It would be hard to stop emitting the warning without making
that assumption, and it may not be easy to make the assumption, but
still avoid the expensive masking operations.
i'm not familiar with gcc internals. for me enum looks like a set
and detecting missed `case' is a basic operation from set theory.
Out-of-range values are unspecified, they can be within the range of E
and they can be outside. Currently, GCC generates out-of-range values,
so we warn you about it. If GCC modified the value to fit within the
range (maybe randomly), it still will be correct but we won't generate
any warning. Yet, people will also complain that they cannot detect
when the value was out-of-range and will request a warning. Hey! one
moment! it seems we already did that at some moment and people did
complain! http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12242
finally, if PR/28236 is valid then (according to the current discussion)
it should be marked as suspended or resolved/wontfix with appropriate
note.
I think it should be marked as invalid but WONTFIX is ok as well.
Since you agree, I will close it right now and point to your message.
Cheers,
Manuel.
Re: [c++] switch ( enum ) vs. default statment.
Paweł Sikora wrote: On the other hand, if you are writing a portable library designed to be used with other people's programs, you might every well want the warning -- because you can't be sure that they're not going to pass 7 in as the value of e, and you may want to be robust in the face of this *unspecified* behavior. sorry, i don't care about unspecified/undefined behavior triggered by users glitches. it's not a problem of my library. The point I was trying to make was that unspecified and undefined are actually very different. I wouldn't be too surprised if, in the future, G++ defined the behavior of the e = (E) 7 case as storing the value in the underlying type. Then, might indeed rely on that. Obviously, you're free to make your own decisions, but, personally, I would certainly feel free to assume that no undefined behavior happened in the application -- but I wouldn't assume that no unspecified behavior occurred. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: [c++] switch ( enum ) vs. default statment.
Paweł Sikora wrote:
typedef enum { X, Y } E;
int f( E e )
{
switch ( e )
{
case X: return -1;
case Y: return +1;
}
}
In this example g++ produces a warning:
e.cpp: In function ‘int f(E)’:
e.cpp:9: warning: control reaches end of non-void function
Adding `default' statemnet to `switch' removes the warning but
in C++ out-of-range values in enums are undefined.
Not quite. They are unspecified; see below.
I see no reason to handling any kind of UB ( especially this ).
IMHO this warning is a bug in C++ frontend.
This is a tricky issue. You are correct that the values of E are
exactly { 0, 1 } (or, equivalently, { X, Y }). But, the underlying type
of the enumeration is at least char. And, the standard says that
assigning an integer value to enumeration type has unspecified behavior
if it outside the range of the enumeration.
So:
E e;
e = (E) 7;
has unspecified behavior, which is defined as:
behavior, for a well-formed program construct and correct data, that
depends on the implementation. The implementation is not required to
document which behavior occurs.
Because the program is unspecified, not undefined, the usual this could
erase your disk thinking does not apply. Unspecified is meant to be
more like Ada's bounded errors (though not as closely specified), in
that something vaguely sensible is supposed to happen.
For GCC, what happens (though we need not document it) is that the value
is converted to the underlying type -- but not masked down to { 0, 1 },
because that masking would be costly. So, ((int) e == 7) may be true
after the assignment above. (Perhaps, in some modes GCC may optimize
away the assignment because it will know that e cannot be 7, but it
does not do so at -O2.)
So, now, what should we do about the warning? I think there are good
arguments in both directions. On the one hand, portable programs cannot
assume that assigning out-of-range values to e does anything
predictable, so portable programs should never do that. So, if you've
written the entire program and are sure that it's portable, you don't
want the warning. On the other hand, if you are writing a portable
library designed to be used with other people's programs, you might
every well want the warning -- because you can't be sure that they're
not going to pass 7 in as the value of e, and you may want to be
robust in the face of this *unspecified* behavior.
In practice, this warning from GCC is keyed off what it thinks the
effective range of E is. If it starts assuming that e can only have
the values { 0, 1 }, it will also stop warning about the missing case.
It would be hard to stop emitting the warning without making that
assumption, and it may not be easy to make the assumption, but still
avoid the expensive masking operations.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: [c++] switch ( enum ) vs. default statment.
On 1/23/07, Paweł Sikora [EMAIL PROTECTED] wrote:
typedef enum { X, Y } E;
int f( E e )
{
switch ( e )
{
case X: return -1;
case Y: return +1;
}
+ throw runtime_error(invalid value got shoehorned into E enum)
}
In this example g++ produces a warning:
e.cpp: In function 'int f(E)':
e.cpp:9: warning: control reaches end of non-void function
Adding `default' statemnet to `switch' removes the warning but
in C++ out-of-range values in enums are undefined.
nevertheless, that integer type might get its bits twiddled somehow.
