Re: [PATCH 2/2] remove NORETURN from function pointers

[Jeff King <peff@peff.net>]

On Mon, Sep 14, 2009 at 12:16:10PM +0200, Erik Faye-Lund wrote:

> From: Erik Faye-Lund <kusmabite@googlemail.com>
> 
> Some compilers (including at least MSVC and ARM RVDS) supports
> NORETURN on function declarations, but not on function pointers.
> 
> This patch makes it possible to define NORETURN for these compilers.
> 
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>

I didn't see a patch 1/2, so maybe it impacts this in some way, but by
itself, I don't think this patch is a good idea. See below.

> ---
> 
> __attribute__((noreturn)) is, according to the GCC documentation, about
> two things: code generation (performance, really) and warnings.
> 
> On the warnings-side, we need to keep the code warning-free for
> compilers who doesn't support noreturn anyway, so hiding potential
> warnings through this mechanism is probably not a good idea in the
> first place.

[Your justification text would almost certainly be useful as part of the
commit message itself, and should go there.]

Unfortunately, this patch _introduces_ warnings when running with gcc,
as it now thinks those function pointers return (which means it thinks
die() returns). So simply removing the NORETURN is not a good idea.

If I understand you correctly, the problem is that there are actually
three classes of compilers:

  1. Ones which understand some NORETURN syntax for both functions and
     function pointers, and correctly trace returns through both (e.g.,
     gcc).

  2. Ones which understand some NORETURN syntax for just functions, and
     complain about it on function pointers. We currently turn off
     NORETURN for these compilers (from your commit message, MSVC,
     for example).

  3. Ones which have no concept of NORETURN at all.

I think the right solution to turn on NORETURN for (2) is to split it
into two cases: NORETURN and NORETURN_PTR. Gcc platforms can define both
identically, platforms under (2) above can define NORETURN_PTR as empty,
and (3) will keep both off.

I do have to wonder, though. What do compilers that fall under (2) do
with calls to function pointers from NORETURN functions? Do they assume
they don't return, or that they do? Or do they not check that NORETURN
functions actually don't return?

I.e., what does this program do under MSVC:

#include <stdlib.h>
void (*exit_fun)(int) = exit;
static void die(void) __attribute__((__noreturn__));
static void die(void) { exit_fun(1); }
int main(void) { die(); }

In gcc, it rightly complains:

  foo.c: In function ‘die’:
  foo.c:4: warning: ‘noreturn’ function does return

-Peff