Question about a warning message

Question about a warning message

[Juan Cabrera]

Hello,

I'm getting a "control reaches end of non-void function" warning
message on the following code (tested with gcc version 7.3 and above):

enum class T { A, B };

int f(T t) {
   switch (t) {
   case T::A: return 10;
   case T::B: return 20;
   }
}

https://godbolt.org/z/IXC4Ff

Shouldn't the compiler assume that function `f` always returns
something for all valid inputs `t`? (Given that the parameter `t` is
of type `T` which is an enum class and the siwtch statement covers all
the enum values).

Regards,
Juan.

Re: Question about a warning message

[Joshua Scoggins]

Hello,


On 09/06/2018 10:58 AM, Juan Cabrera wrote:
> Hello,
> 
> I'm getting a "control reaches end of non-void function" warning
> message on the following code (tested with gcc version 7.3 and above):
> 
> enum class T { A, B };
> 
> int f(T t) {
>    switch (t) {
>    case T::A: return 10;
>    case T::B: return 20;
>    }
> }
> 
> 
> Shouldn't the compiler assume that function `f` always returns
> something for all valid inputs `t`? (Given that the parameter `t` is
> of type `T` which is an enum class and the siwtch statement covers all
> the enum values).
> 

No the compiler should not assume this as an enum class does not protect
you from invalid input and casts (only implicit conversion is
prevented). Here is an example:

int performOperation() {
  return f(T::A) + f(T::B) + f(static_cast<T>(3));
}

The legal values for T are A and B which are 0 and 1 respectively. Three
is not a legal value for type T but it can be cast into it. Thus an
invisible fallthrough will happen and an undefined result will be
returned from f in the case of 3 (or any other number).


> Regards,
> Juan.
> 

-- 

Josh Scoggins

Re: Question about a warning message

[Ruslan Garipov]

> Shouldn't the compiler assume that function `f` always returns
> something for all valid inputs `t`?

I believe no, because for some crazy reason I can call `f` in the 
following way[1]:

     f(static_cast<T>(123));

Therefore, argument `t` is neither `T::A`, nor `T::B`.

Moreover:

 > 9.3.4 List-initialization [dcl.init.list]
 > 3 List-initialization of an object or reference of type T is defined
 > as follows:
 > (3.8) Otherwise, if T is an enumeration with a fixed underlying type
 > (9.6), the initializer-list has a single element v , and the
 > initialization is direct-list-initialization, the object is
 > initialized with the value T(v) (7.6.1.3); if a narrowing conversion
 > is required to convert v to the underlying type of T , the program is
 > ill-formed.

Thus (I believe with `-std=c++17`) we can "end up" with:

     T bar{123};
     int foo = f(bar);

Once again, the `f` function gets neither `T::A`, nor `T::B`.

[1] "7.6.1.9 Static cast [expr.static.cast]. 10 A value of integral or 
enumeration type can be explicitly converted to a complete enumeration 
type."

On 9/6/2018 10:58 PM, Juan Cabrera wrote:
> Hello,
> 
> I'm getting a "control reaches end of non-void function" warning
> message on the following code (tested with gcc version 7.3 and above):
> 
> enum class T { A, B };
> 
> int f(T t) {
>     switch (t) {
>     case T::A: return 10;
>     case T::B: return 20;
>     }
> }
> 
> https://godbolt.org/z/IXC4Ff
> 
> Shouldn't the compiler assume that function `f` always returns
> something for all valid inputs `t`? (Given that the parameter `t` is
> of type `T` which is an enum class and the siwtch statement covers all
> the enum values).
> 
> Regards,
> Juan.
> 

Re: Question about a warning message

[Juan Cabrera]

Hello!

Thank you for your answers.
I'm aware that you can cast anything into an `enum class` but I
thought that casting a non valid enum-class value into an enum-class
was itself "wrong" or "unedfined behavior" or something like that :D.

I just checked and it looks like clang does not emit a warning in this case.

I guess I'll have to add a return statement to that function to keep
the compiler from complaning :D

Regards,
El jue., 6 sept. 2018 a las 15:56, Ruslan Garipov
(<ruslanngaripov@gmail.com>) escribió:
>
> > Shouldn't the compiler assume that function `f` always returns
> > something for all valid inputs `t`?
>
> I believe no, because for some crazy reason I can call `f` in the
> following way[1]:
>
>      f(static_cast<T>(123));
>
> Therefore, argument `t` is neither `T::A`, nor `T::B`.
>
> Moreover:
>
>  > 9.3.4 List-initialization [dcl.init.list]
>  > 3 List-initialization of an object or reference of type T is defined
>  > as follows:
>  > (3.8) Otherwise, if T is an enumeration with a fixed underlying type
>  > (9.6), the initializer-list has a single element v , and the
>  > initialization is direct-list-initialization, the object is
>  > initialized with the value T(v) (7.6.1.3); if a narrowing conversion
>  > is required to convert v to the underlying type of T , the program is
>  > ill-formed.
>
> Thus (I believe with `-std=c++17`) we can "end up" with:
>
>      T bar{123};
>      int foo = f(bar);
>
> Once again, the `f` function gets neither `T::A`, nor `T::B`.
>
> [1] "7.6.1.9 Static cast [expr.static.cast]. 10 A value of integral or
> enumeration type can be explicitly converted to a complete enumeration
> type."
>
> On 9/6/2018 10:58 PM, Juan Cabrera wrote:
> > Hello,
> >
> > I'm getting a "control reaches end of non-void function" warning
> > message on the following code (tested with gcc version 7.3 and above):
> >
> > enum class T { A, B };
> >
> > int f(T t) {
> >     switch (t) {
> >     case T::A: return 10;
> >     case T::B: return 20;
> >     }
> > }
> >
> > https://godbolt.org/z/IXC4Ff
> >
> > Shouldn't the compiler assume that function `f` always returns
> > something for all valid inputs `t`? (Given that the parameter `t` is
> > of type `T` which is an enum class and the siwtch statement covers all
> > the enum values).
> >
> > Regards,
> > Juan.
> >

Re: Question about a warning message

[Jonathan Wakely]

On Thu, 6 Sep 2018 at 20:30, Juan Cabrera <jjcp.91@gmail.com> wrote:
>
> Hello!
>
> Thank you for your answers.
> I'm aware that you can cast anything into an `enum class` but I
> thought that casting a non valid enum-class value into an enum-class
> was itself "wrong" or "unedfined behavior" or something like that :D.

No, because for an enum class all values of the underlying type are
valid values of the enum.

The compiler is right to warn.

Re: Question about a warning message

[Jonathan Wakely]

On Thu, 6 Sep 2018 at 19:53, Joshua Scoggins <Josh.Scoggins@parasoft.com> wrote:
>
> Hello,
>
>
> On 09/06/2018 10:58 AM, Juan Cabrera wrote:
> > Hello,
> >
> > I'm getting a "control reaches end of non-void function" warning
> > message on the following code (tested with gcc version 7.3 and above):
> >
> > enum class T { A, B };
> >
> > int f(T t) {
> >    switch (t) {
> >    case T::A: return 10;
> >    case T::B: return 20;
> >    }
> > }
> >
> >
> > Shouldn't the compiler assume that function `f` always returns
> > something for all valid inputs `t`? (Given that the parameter `t` is
> > of type `T` which is an enum class and the siwtch statement covers all
> > the enum values).
> >
>
> No the compiler should not assume this as an enum class does not protect
> you from invalid input and casts (only implicit conversion is
> prevented). Here is an example:
>
> int performOperation() {
>   return f(T::A) + f(T::B) + f(static_cast<T>(3));
> }
>
> The legal values for T are A and B which are 0 and 1 respectively.

No, it's an enum class, so all values of the underlying type are
valid. The underlying type here is 'int' so any value of int is a
valid value of T.

> Three
> is not a legal value for type T but it can be cast into it.

3 is a valid value.

I wish people understood enums.

Re: Question about a warning message

[David Brown]

On 06/09/18 21:29, Juan Cabrera wrote:
> Hello!
> 
> Thank you for your answers.
> I'm aware that you can cast anything into an `enum class` but I
> thought that casting a non valid enum-class value into an enum-class
> was itself "wrong" or "unedfined behavior" or something like that :D.
> 
> I just checked and it looks like clang does not emit a warning in this case.
> 
> I guess I'll have to add a return statement to that function to keep
> the compiler from complaning :D
> 

It is usually not a good idea to change code just to stop the compiler
from complaining.  Unless it is a false positive warning where you know
the code is good, listen to the compiler.

To me, there are two choices.  One is that you believe "f" /might/ be
called with a T that is not A or B.  In that case, you should handle it
appropriately - not just have a fake return to keep the compiler quiet.
 For example:

int f(T t) {
	switch (t) {
		case T::A : return 10;
		case T::B : return 20;
	}
	throw std::runtime_error("Calling f with a bad t");
}

Alternatively, if you are sure that "f" will never be called with an
inappropriate t, tell the compiler that:

int f(T t) {
	switch (t) {
		case T::A : return 10;
		case T::B : return 20;
	}
	__builtin_unreachable();
}

That at least can lead to better code, and perhaps give more checking or
optimisation opportunities.

In between these are things like "assert" and the option of checking
during development and testing, and disabling checks at later stages.

A "just return something" solution usually combines the disadvantages of
all these options.

Re: Question about a warning message

[Xi Ruoyao]

On 2018-09-07 11:06 +0200, David Brown wrote:
> On 06/09/18 21:29, Juan Cabrera wrote:
> > Hello!
> > 
> > Thank you for your answers.
> > I'm aware that you can cast anything into an `enum class` but I
> > thought that casting a non valid enum-class value into an enum-class
> > was itself "wrong" or "unedfined behavior" or something like that :D.
> > 
> > I just checked and it looks like clang does not emit a warning in this case.
> > 
> > I guess I'll have to add a return statement to that function to keep
> > the compiler from complaning :D
> > 
> 
> It is usually not a good idea to change code just to stop the compiler
> from complaining.  Unless it is a false positive warning where you know
> the code is good, listen to the compiler.
> 
> To me, there are two choices.  One is that you believe "f" /might/ be
> called with a T that is not A or B.  In that case, you should handle it
> appropriately - not just have a fake return to keep the compiler quiet.
>  For example:
> 
> int f(T t) {
> 	switch (t) {
> 		case T::A : return 10;
> 		case T::B : return 20;
> 	}
> 	throw std::runtime_error("Calling f with a bad t");
> }
> 
> Alternatively, if you are sure that "f" will never be called with an
> inappropriate t, tell the compiler that:
> 
> int f(T t) {
> 	switch (t) {
> 		case T::A : return 10;
> 		case T::B : return 20;
> 	}
> 	__builtin_unreachable();
> }

I usually write something like

    #ifdef NDEBUG
    #define unreachable() __builtin_unreachable()
    #else
    #define unreachable() __builtin_trap()
    #endif

and

    switch (x) {
       case A: ...
       case B: ...
       case C: ...
       default: unreachable(); // should not happen
    }

Flowing off the end of a function is equivalent to a return with no value;
this results in undefined behavior in a value-returning function.
(6.6.3 [stmt.return]).  In my point of view "__builtin_unreachable()"
is just like an undefined behavior - the compiler just assume it won't
happen.

> That at least can lead to better code, and perhaps give more checking or
> optimisation opportunities.

> In between these are things like "assert" and the option of checking
> during development and testing, and disabling checks at later stages.
> 
> A "just return something" solution usually combines the disadvantages of
> all these options.

Agree.  If you really want to return something you should return a
well-defined and documented error code (for example -EINVAL, or you
can define a new constant).
-- 
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University