It would be nice to have a macro Py_UNREACHABLE to keep compiler warnings away in unreachable code. This is can call __builtin_unreachable on gcc and abort elsewhere.

What's the specific warning that you want to eliminate?

2012/4/23 Martin v. Löwis <report@bugs.python.org>:
>
> Martin v. Löwis <martin@v.loewis.de> added the comment:
>
> What's the specific warning that you want to eliminate?

"control reaches end of non-void function without return"

Basically, whenever you see "assert(0)" today.

>> What's the specific warning that you want to eliminate?
>
> "control reaches end of non-void function without return"
>
> Basically, whenever you see "assert(0)" today.

Sorry, what I meant is: what specific line (in what specific
source file) is that warning on?

If there is an assert(0) somewhere, there should be no
subsequent code, so no such warning should be generated.

2012/4/23 Martin v. Löwis <report@bugs.python.org>:
>
> Martin v. Löwis <martin@v.loewis.de> added the comment:
>
>>> What's the specific warning that you want to eliminate?
>>
>> "control reaches end of non-void function without return"
>>
>> Basically, whenever you see "assert(0)" today.
>
> Sorry, what I meant is: what specific line (in what specific
> source file) is that warning on?
>
> If there is an assert(0) somewhere, there should be no
> subsequent code, so no such warning should be generated.

Right, we currently avoid this problem by writing "assert(0)" for
example in lookdict_split in dictobject.c. What I'm saying is that
instead writing "assert(0)", we could use Py_UNREACHABLE.

> Right, we currently avoid this problem by writing "assert(0)" for
> example in lookdict_split in dictobject.c. What I'm saying is that
> instead writing "assert(0)", we could use Py_UNREACHABLE.

I don't understand. The assert(0) is not there to silence any compiler
warnings, is it? Instead, ISTM that it is there to truly assert that
the code is not reached, which isn't actually obvious at all (IIUC, it's
not reached because there must be some empty slot eventually unless the
key is in there already).

Instead, ISTM that is actually the "return 0;" that should silence the
compiler warning about not having a return statement.

If that's all true, I fail to see what __builtin_unreachable would achieve:
we certainly have to preserve the return statement, for compilers not  
supporting
such a declaration.

Wouldn't this actually have the undesirable effect of complaining about
the return statement when compiling with -Wunreachable-code?

Let me explain what I meant with code.

Please, go ahead, explain :-)

Did you see the sample patch I posted?

Sorry, I missed the patch. I still fail to see the problem that this solves: what compiler produces "control reaches end of non-void function without return" for the current code? ISTM that your patch has the potential of *introducing* such a warning on some compilers, since you are removing the return statement (and the compiler may not be smart enough to determine that Py_FatalError does not return).

I think you misuse __builtin_unreachable(): the code *is* reachable,
it is just very unlikely.

I really prefer to return something looking valid and continue the
execution of the program, instead of calling the evil Py_FatalError()
in release mode which stops immediatly the program in an insane
manner. For example, -1 is a valid result for findchar(), so the
program execution will continue, even if the kind is invalid.

> I really prefer to return something looking valid and continue the
> execution of the program

How would that be better? Should Python become more like PHP?

Oh, I read again the patch. There are two different cases:

 * The code is really unreachable. Ex: the loop of lookdict() does never stop, return is used in the loop. Py_UNREACHABLE can be used in this case *to avoid a compiler warning*. __builtin_unreachable() helps if you have GCC, but if you don't have GCC, using return with a dummy value would be better than a Py_FatalError(). I don't think that calling Py_FatalError() does make the warning quiet.

 * The code is reachable, but if it is reached, it's a bug. Ex: switch/case on the Unicode kinde of a string. I like the current solution: assert(0) + return a almost valid value, but Antoine and Benjamin don't like the "almost valid value" (and so prefer a fatal error?). We may issue a warning instead of a fatal error (e.g. write a message into stderr with fprintf ?) in release mode.

--

Using return, it gives something like:

+#ifdef NDEBUG
+#ifdef __GNUC__
+#define Py_UNREACHABLE(value) __builtin_unreachable()
+#else
+#define Py_UNREACHABLE(value) return (value);
+#endif
+#else
+#define Py_UNREACHABLE(value) do { assert(0); return (value); } while (0)
+#endif

It cannot be used if the function has no result value (void), but quite all Python functions have a result.

> I like the current
> solution: assert(0) + return a almost valid value, but Antoine and
> Benjamin don't like the "almost valid value" (and so prefer a fatal
> error?). We may issue a warning instead of a fatal error (e.g. write a 
> message into stderr with fprintf ?) in release mode.

If there's a bug, either an exception should be raised, or a fatal error.
We should discourage warnings on stderr (the PHP approach).

> If there's a bug, either an exception should be raised, or a fatal error.
> We should discourage warnings on stderr (the PHP approach).

Agreed.

That said, I'm with Martin in that don't see how the patch in this issue helps.

> Py_UNREACHABLE can be used in this case *to avoid a compiler warning*.

What is the specific warning that you want to avoid, and what specific compiler version produces it currently on what specific code? 

It's not "control reaches end of non-void function without return", is it? (since if the compiler correctly determines that the code is unreachable, it shouldn't complain that control reaches the end of the function, as it doesn't reach the end of the function).

It does not avoid any warning because we have the "assert(0); return X" pattern. I was just attempting to provide a standard way of marking unreachable code.

> I was just attempting to provide a standard way of marking unreachable code.

I'm -1 for the proposed patch (and probably -0 on the general idea). I think the patch has the potential *introducing* new warnings, as compilers might warn that a return is lacking in these functions. 

I'm -0 on the general idea, as I think the status quo is just fine.

As for Victor's second use case (run-time checking that supposedly-unreachable code is indeed not reached, in release mode), I'm -0 also: we check that in debug mode; this looks sufficient to me.

The Py_UNREACHABLE() macro was implemented in bpo-31338, so this issue can be closed.