Failure of type safety system to catch error -- why?

Failure of type safety system to catch error -- why?

Post by Roy Smit » Mon, 28 Jul 2003 19:42:09



I had a function which looked like:

void foo (bool master)
{
    if (master) {
       blah, blah, blah....
    }

Quote:}

I decided to make it a little more typesafe, and a little more
self-documenting, so I defined an enun to use instead of bool:

enum masterType { master, slave };
void foo (masterType type)
{
    if (master) {
       blah, blah, blah....
    }

Quote:}

The error, of course, is that I forgot to change the test to be "if
(type == master)".  The surprising thing is that the code compiled (so
much for improving type safety, eh?).  Fortunately unit tests caught the
problem.

So, my question is: Other than writing unit tests (which I do anyway),
is there anything I can do to reduce the chances of introducing a
similar bug in the future?

It seems somewhat surprising to me that an enum has a truth value.  I'm
explicitly saying, "Give me two magic cookies that I can tell apart from
each other, and from all other values", and the compiler is coming along
and saying, "OK, here they are, and by the way, one of them is true and
the other is false, wink, wink".

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by Francis Glassboro » Tue, 29 Jul 2003 02:14:42




Quote:>I had a function which looked like:

>void foo (bool master)
>{
>    if (master) {
>       blah, blah, blah....
>    }
>}

>I decided to make it a little more typesafe, and a little more
>self-documenting, so I defined an enun to use instead of bool:

>enum masterType { master, slave };
>void foo (masterType type)
>{
>    if (master) {
>       blah, blah, blah....
>    }
>}

>The error, of course, is that I forgot to change the test to be "if
>(type == master)".  The surprising thing is that the code compiled (so
>much for improving type safety, eh?).  Fortunately unit tests caught the
>problem.

Or that you declared the enum in the wrong order, or relied on default
initialisation values:

enum type{slave, master};

works as does:

enum type {master = true, slave = false};

--
ACCU Spring Conference 2003 April 2-5
The Conference you should not have missed
ACCU Spring Conference 2004 Late April
Francis Glassborow      ACCU

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by Pete Becke » Tue, 29 Jul 2003 02:17:20



> It seems somewhat surprising to me that an enum has a truth value.  I'm
> explicitly saying, "Give me two magic cookies that I can tell apart from
> each other, and from all other values",

Nope. You're saying "here are two named constants." All integral types
are convertible to bool.

--

"To delight in war is a merit in the soldier,
a dangerous quality in the captain, and a
positive crime in the statesman."
        George Santayana

"Bring them on."
        George W. Bush

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by Sebastian Kapfe » Tue, 29 Jul 2003 02:19:12



> The error, of course, is that I forgot to change the test to be "if
> (type == master)".  The surprising thing is that the code compiled (so
> much for improving type safety, eh?).  Fortunately unit tests caught the
> problem.

> So, my question is: Other than writing unit tests (which I do anyway),
> is there anything I can do to reduce the chances of introducing a
> similar bug in the future?

I'm afraid not. Not with plain enum. You could wrap every enum in a
separate class

        template< typename enum_type >
        class Enum
        {
        public:
                // insert boilerplate here
                // do not provide conversions to enum_type or int
        private:
                enum_type value;
        };

to prevent implicit conversions to integers. Of course this makes enum
look quite pointless.

Quote:> It seems somewhat surprising to me that an enum has a truth value.

As always, C++ is suffering from its C heritage. enum's convert to ints
implicitly, and ints do have a truth value. After all, there was no
boolean type in C. "if(0)" is the same as "if(false)", and "if(42)" (or
any other nonzero integer) is the same as "if(true)".

The funny part is, if you had defined your enum the other way

        enum masterType { slave, master };

it would have actually worked, and the unit test wouldn't have noticed the
bug.

        if(slave) => if(0) => if(false)
        if(master) => if(1) => if(true)

So you can consider yourself lucky nevertheless :-)

Quote:> I'm explicitly saying, "Give me two magic cookies that I can tell apart
> from each other, and from all other values", and the compiler is coming
> along and saying, "OK, here they are, and by the way, one of them is
> true and the other is false, wink, wink".

Sadly, this is the way C++ works. I'm unsure if your code should have
triggered a compiler warning... it isn't exactly idiomatic (ab)use of enum
values.

--
Best Regards,   |   Hi! I'm a .signature virus. Copy me into
 Sebastian      |   your ~/.signature to help me spread!

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by Andy Sawye » Tue, 29 Jul 2003 08:31:12



 on 27 Jul 2003 06:42:09 -0400,

Quote:> I had a function which looked like:

> void foo (bool master)
> {
>     if (master) {
>        blah, blah, blah....
>     }
> }

> I decided to make it a little more typesafe, and a little more
> self-documenting, so I defined an enun to use instead of bool:

> enum masterType { master, slave };
> void foo (masterType type)
> {
>     if (master) {
>        blah, blah, blah....
>     }
> }

> The error, of course, is that I forgot to change the test to be "if
> (type == master)".  The surprising thing is that the code compiled (so
> much for improving type safety, eh?).

The code is "typesafe" (for some value of typesafe). You can no longer
call foo as "foo(true);" - which seems to be what you want to achieve.

Quote:> So, my question is: Other than writing unit tests (which I do anyway),
> is there anything I can do to reduce the chances of introducing a
> similar bug in the future?

The obvious answer is don't write code like that - all built-in types
have an implicit coversion to bool (see 4.12).

Quote:> It seems somewhat surprising to me that an enum has a truth value.

It doesn't seem in the least surprising to me.

Quote:> I'm explicitly saying, "Give me two magic cookies that I can tell
> apart from each other, and from all other values",

No, you're not. The aren't "magic cookies", they're well-defined integer
values.

Quote:> and the compiler is coming along and saying, "OK, here they are, and
> by the way, one of them is true and the other is false, wink, wink".

The integer values of master and slave are 0 and 1 respectively. In a
boolean context (such as if(...)) these are converted to false and true
respectively. There's nothing subversive going on (as your "wink wink"
seems to imply) - those are the well defined semantics of the langauge.
You might want to read section 7.2 of the standard, which defines
enumerated types.

Regards,
 Andy S.
--
"Light thinks it travels faster than anything but it is wrong. No matter
 how fast light travels it finds the darkness has always got there first,
 and is waiting for it."                  -- Terry Pratchett, Reaper Man

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by Andy Sawye » Tue, 29 Jul 2003 08:37:34



 on 27 Jul 2003 13:19:12 -0400,

Quote:> Sadly, this is the way C++ works. I'm unsure if your code should have
> triggered a compiler warning...

Probably not - it's perfectly well-formed. (but then again, there are
compilers which warn you about their de*s being flakey ;)

Quote:> it isn't exactly idiomatic (ab)use of enum values.

On the contrary, I find it very idiomatic:

enum error_t { err_none = 0, err_comms, error_crc, /* ... */ ) error;

if( error ) { .... }

Regards,
 Andy S
--
"Light thinks it travels faster than anything but it is wrong. No matter
 how fast light travels it finds the darkness has always got there first,
 and is waiting for it."                  -- Terry Pratchett, Reaper Man

      [ See http://www.veryComputer.com/]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by James Dennet » Tue, 29 Jul 2003 09:18:54




>>The error, of course, is that I forgot to change the test to be "if
>>(type == master)".  The surprising thing is that the code compiled (so
>>much for improving type safety, eh?).  Fortunately unit tests caught the
>>problem.

>>So, my question is: Other than writing unit tests (which I do anyway),
>>is there anything I can do to reduce the chances of introducing a
>>similar bug in the future?

[snip]

Quote:

> Sadly, this is the way C++ works. I'm unsure if your code should have
> triggered a compiler warning... it isn't exactly idiomatic (ab)use of enum
> values.

I've seen compilers issue warnings when the expression used
in an if test is constant, at least in some forms.  It can
be annoying if they do so for if(true) or if(false) as I find
those occasionally preferable to conditional compilation
though.

-- James.

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by Sebastian Kapfe » Tue, 29 Jul 2003 18:52:28



> I've seen compilers issue warnings when the expression used
> in an if test is constant, at least in some forms.  It can
> be annoying if they do so for if(true) or if(false) as I find
> those occasionally preferable to conditional compilation
> though.

I don't mean "if" conditions which depend on constants. The code here was

        enum Something { /* ... */} ;

        void f( Something foo )
        {
                if( foo ) doBar();
        }

I.e. an "if" statement which takes a (non-constant) enum value. I don't
think this is idiomatic C++ in any way; however, Andy seems to disagree.

--
Best Regards,   |   Hi! I'm a .signature virus. Copy me into
 Sebastian      |   your ~/.signature to help me spread!

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by johnc » Tue, 29 Jul 2003 18:52:59



 > I had a function which looked like:
 >
 > void foo (bool master)
 > {
 >     if (master) {
 >        blah, blah, blah....
 >     }
 > }
 >
 > I decided to make it a little more typesafe, and a little more
 > self-documenting, so I defined an enun to use instead of bool:
 >
 > enum masterType { master, slave };
 > void foo (masterType type)
 > {
 >     if (master) {
 >        blah, blah, blah....
 >     }
 > }
 >
 > The error, of course, is that I forgot to change the test to be "if
 > (type == master)".  The surprising thing is that the code compiled (so
 > much for improving type safety, eh?).  Fortunately unit tests caught the
 > problem.
 >
 > So, my question is: Other than writing unit tests (which I do anyway),
 > is there anything I can do to reduce the chances of introducing a
 > similar bug in the future?

One option to consider:

class Master {};
class Slave {};

void foo( const Master& ) {
   // foo impl for master

Quote:}

void foo( const Slave& ) {
   // foo impl for slave

Quote:}

... and if there's a lot of common code for the two cases, you might
factor it out into a separate function (say, foo_common()).

If you want to dispatch on an enumerator, consider using a switch
statement instead of an "if":

void foo(masterType t) {
   // ... do stuff
   switch (t) {
     case master:
       // ... do master stuff
       break;
     case slave:
       // ... do slave stuff
       break;
     case else:
       assert(false);
   }

Quote:}

It's easier to see that you've handled all the cases, and the assert()
will catch any unexpected input (e.g.  master | slave ).

hth!

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by llewell » Tue, 29 Jul 2003 18:55:41


[snip]
 > Sadly, this is the way C++ works. I'm unsure if your code should have
 > triggered a compiler warning... it isn't exactly idiomatic (ab)use of enum
 > values.

Testing enums as truth values is a vernerable idiom inherted from
     C. Reams of working C++ code uses it. IMO it would be
     unjustifiable for a compiler to warn about such code.

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by Markus Werl » Wed, 30 Jul 2003 06:21:32



> I had a function which looked like:

> void foo (bool master)
> {
>     if (master) {
>        blah, blah, blah....
>     }
> }

> I decided to make it a little more typesafe, and a little more
> self-documenting, so I defined an enun to use instead of bool:

> enum masterType { master, slave };
> void foo (masterType type)
> {
>     if (master) {
>        blah, blah, blah....
>     }
> }

> The error, of course, is that I forgot to change the test to be "if
> (type == master)".  

There is no firewall against errors like these.
Coding at night always bears some risk.

Quote:> So, my question is: Other than writing unit tests (which I do anyway),
> is there anything I can do to reduce the chances of introducing a
> similar bug in the future?

No.

Quote:> It seems somewhat surprising to me that an enum has a truth value.  I'm
> explicitly saying, "Give me two magic cookies that I can tell apart from
> each other, and from all other values", and the compiler is coming along
> and saying, "OK, here they are, and by the way, one of them is true and
> the other is false, wink, wink".

I tried to prohibit automatic conversion using
template args, but besides the gain of some efficiency your
error is not catched by this solution either:

enum Type { master, slave };
// (a masterType with value "slave" was unbearable to me ;-))

template <Type T>
void foo ()
{
    if (T == master) { // still required :-(
      // blah, blah, blah....
    }

Quote:}

int main()
{
  foo<true>(); // at least this one is catched by the compiler

Quote:}

So let us use brute force (which I think is overkill here)
and go into the type system completely.

namespace Type
{
struct master {};
struct slave {};

Quote:}

// our function foo
template <class T> struct Foo;

template <>
struct Foo<Type::master>
{
  inline void operator()()
  {
    // blah, blah, blah....
  }

Quote:};

template <>
struct Foo<Type::slave>
{
  inline void operator()()
  {
    // ...
  }

Quote:};

int main()
{
  Foo<Type::master>()();

Quote:}

best regards,

Markus

--

Build your own Expression Template Library with Daixtrose!
Visit http://daixtrose.sourceforge.net/

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by Francis Glassboro » Wed, 30 Jul 2003 06:45:28




Quote:>One option to consider:

>class Master {};
>class Slave {};

>void foo( const Master& ) {
>   // foo impl for master
>}

>void foo( const Slave& ) {
>   // foo impl for slave
>}

>... and if there's a lot of common code for the two cases, you might
>factor it out into a separate function (say, foo_common()).

>If you want to dispatch on an enumerator, consider using a switch
>statement instead of an "if":

>void foo(masterType t) {
>   // ... do stuff
>   switch (t) {
>     case master:
>       // ... do master stuff
>       break;
>     case slave:
>       // ... do slave stuff
>       break;
>     case else:
>       assert(false);
>   }
>}

>It's easier to see that you've handled all the cases, and the assert()
>will catch any unexpected input (e.g.  master | slave ).

Well I, for one, would not dream of going that route. I just might
consider:

class type {
public:
        virtual ~type(){}
        virtual void doit() = 0;

Quote:};

class master : public type {
// ...

Quote:};

class slave : public type {
// ...

Quote:};

but only if I had reason to expect that there might be some third
classification in the future.
IMO this is a classic example of where not to use a switch statement.

--
ACCU Spring Conference 2003 April 2-5
The Conference you should not have missed
ACCU Spring Conference 2004 Late April
Francis Glassborow      ACCU

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by James Dennet » Wed, 30 Jul 2003 19:18:58



 >
 >
 >>I've seen compilers issue warnings when the expression used
 >>in an if test is constant, at least in some forms.  It can
 >>be annoying if they do so for if(true) or if(false) as I find
 >>those occasionally preferable to conditional compilation
 >>though.
 >
 >
 > I don't mean "if" conditions which depend on constants. The code here was
 >
 >         enum Something { /* ... */} ;
 >
 >         void f( Something foo )
 >         {
 >                 if( foo ) doBar();
 >         }
 >
 > I.e. an "if" statement which takes a (non-constant) enum value. I don't
 > think this is idiomatic C++ in any way; however, Andy seems to disagree.
 >

As do I; it's fairly consistent with the traditional C and C++
abuse of contexts where boolean expressions "ought" to be used
(and without any implicit conversions going on).  So, I'd say
that it's fairly idiomatic, although I'd prefer to work with
coding conventions that ban such use.

-- James.

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by johnc » Thu, 31 Jul 2003 07:59:09




> >If you want to dispatch on an enumerator, consider using a switch
> >statement instead of an "if":

> >void foo(masterType t) {
> >   // ... do stuff
> >   switch (t) {
> >     case master:
> >       // ... do master stuff
> >       break;
> >     case slave:
> >       // ... do slave stuff
> >       break;
> >     case else:
> >       assert(false);
> >   }
> >}

> >It's easier to see that you've handled all the cases, and the assert()
> >will catch any unexpected input (e.g.  master | slave ).

> Well I, for one, would not dream of going that route. I just might
> consider:

> class type {
> public:
>         virtual ~type(){}
>         virtual void doit() = 0;
> };

> class master : public type {
> // ...
> };

> class slave : public type {
> // ...
> };

> but only if I had reason to expect that there might be some third
> classification in the future.

Well, sure, but the predicate ("if you want to dispatch on an
enumerator") was meant.

Quote:> IMO this is a classic example of where not to use a switch statement.

Hmm... I would have thought that dispatching on an enumerator
would be a classic case of where you *do* want to use a switch
statement.  (Whether it's a good idea to dispatch on an enumerator at
all is another question.)

The drawback I see with the solution you posted earlier

  > enum type{slave, master};
  >
  > works as does:
  >
  > enum type {master = true, slave = false};

   (plus, I assume, continuing to test the value of the function
    paramater against true)

is that it continues to tie the meaning of each enumerator to its
numeric value (which, as the OP noticed, is error prone).  Of course,
you're correcting the OP's error, but the question was how to write
code that won't make such an error in the first place.

Using a switch (or, equivalently, a series of if (blah == foo)
statements) ties the meaning of the enumerator (i.e. what code path
will be executed) directly to the name of the enumerator.  Easier to
get right, harder to get wrong, IMHO.

No?

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

 
 
 

Failure of type safety system to catch error -- why?

Post by llewell » Fri, 01 Aug 2003 08:43:52





>  >>I've seen compilers issue warnings when the expression used
>  >>in an if test is constant, at least in some forms.  It can
>  >>be annoying if they do so for if(true) or if(false) as I find
>  >>those occasionally preferable to conditional compilation
>  >>though.

>  > I don't mean "if" conditions which depend on constants. The code here was

>  >         enum Something { /* ... */} ;

>  >         void f( Something foo )
>  >         {
>  >                 if( foo ) doBar();
>  >         }

>  > I.e. an "if" statement which takes a (non-constant) enum value. I don't
>  > think this is idiomatic C++ in any way; however, Andy seems to disagree.

> As do I; it's fairly consistent with the traditional C and C++
> abuse of contexts where boolean expressions "ought" to be used
> (and without any implicit conversions going on).  So, I'd say
> that it's fairly idiomatic, although I'd prefer to work with
> coding conventions that ban such use.

I used to think this, when I was in college. Since then I've found
    most programmers I've worked actually read:

    while(cin >> foo)
    {
        /* something with foo*/
    }

    more easily than:

    while(cin >> foo != 0)
    {
        /* something with foo*/
    }

    and chosing a function name to make 'if(some_condition())' and
    such read nicely is easier than making 'if(some_condition() != 0)'
    read nicely.

Of course, the fact that these things are associated with implicit
    conversions is largely a matter of history (and for a new
    function, the 'some_condition()' from above should usually return
    bool, but lots of functions named nicely for if and while return
    int), but that's what we have.

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]