C++ Logo

SG12

Advanced search

Subject: Re: [SG12] Missing non-void return on some paths
From: Jonathan Wakely (cxx_at_[hidden])
Date: 2021-05-08 15:11:36


On Sat, 8 May 2021, 20:29 Arthur O'Dwyer via SG12, <sg12_at_[hidden]>
wrote:

> On Sat, May 8, 2021 at 3:04 PM JF Bastien via SG12 <sg12_at_[hidden]>
> wrote:
>
>> On Sat, May 8, 2021 at 11:52 AM Nevin Liber via SG12 <
>> sg12_at_[hidden]> wrote:
>>
>>> On Thu, May 6, 2021 at 6:47 PM JF Bastien via SG12 <
>>> sg12_at_[hidden]> wrote:
>>>
>>>> Compilers diagnose when functions can't be proved to return, and I
>>>> wouldn't work on a codebase without this diagnostic enabled as an error. Is
>>>> there a valid reason to keep this UB around?
>>>>
>>>
>>> How does one write an assert-type macro which, when it is disabled,
>>> still prevents this type of warning/error? Because people do write:
>>>
>>> my_assert(false);
>>>
>>> to mean abort in debug mode, take my chances in release mode.
>>>
>>
>> I don’t think I understand what you’re asking... but it sounds like
>>
>> #ifdef _DEBUG
>> #define YOLO() abort()
>> #else
>> #define YOLO() std::unreachable() // or __builtin_unreachable()
>>
>
> That looks right to me.
>
> JF,
> (1) You originally mentioned "[[noreturn]]" in the same breath with
> "unreachable," which is confusing to me. Suppose I write a function `f1`
> declared `[[noreturn]]`, which "seemingly falls off the end"; say, `void
> g(); [[noreturn]] int f1() { g(); }` — Does f's code clearly indicate my
> intent that "I know g() is non-returning, so please don't warn here," or
> does f's code suggest to the compiler that I've written a bug?
>

IMHO the latter. I think you should have to add [[noreturn]] to g or add
unreachable to f.

Would you make it so that a conforming compiler required to diagnose this
> code as ill-formed? Or required *not* to diagnose it?
>

I would like this to be a required diagnostic.

The [[noreturn]] tells callers of f that it doesn't return. I don't think
it would be used to say anything about control flow within f. I think that
could hide bugs in f. It looks to me like g returns normally, so f will
too. Maybe you forgot to write abort() after calling g(). If the compiler
interprets the [[noreturn]] to mean g doesn't return, it won't warn you
about your mistake.

If g doesn't return, the way to say that is to mark g as noreturn or to put
unreachable after the call to g.

> (2) How do we standardize the idea that the compiler should be able to do
> control flow analysis here.
>

Correctly :-P

We already do it for constant evaluation (yes, I know that's more
tractable).

I mean, if we have, like, `int f2() { while (true) { if (rand()) return 42;
> } }`, is a conforming compiler supposed to be smart enough to see the end
> of the function is unreachable?
>

Heck yes.

I would have some sympathy if somebody said that was expecting too much of
a C compiler. I think it's fine to expect that for C++ compilers.

Or what if it's, like,
>
    int f3(bool b) {
> if (b) { return 42; }
> if (!b) { abort(); } // or std::unreachable(), your choice
> }
> Would you make it so that a conforming compiler required to diagnose this
> code as ill-formed? Or required *not* to diagnose it?
>

It either returns or calls a noreturn function on all paths. Required not
to diagnose.

> (3) Alternatively, are you proposing that a conforming compiler should be
> forced to insert a call to `std::terminate()` at the bottom of any function
> it can't prove isn't reached?
>

No, because if we require one of return, throw, calling a noreturn
function, or calling std::unreachable on all paths then that terminate can
never happen. If the end of a non-void function can be reached without one
of those things, issue a diagnostic. Also inserting a trap instruction or
terminate call (or ubsan error message) would be a reasonable option to
support if the diagnostic is only a warning.

(That would be nicely consistent with the big oops C++ made with
> `noexcept`.)
>
> Basically, "needs a paper, needs Tony Tables."
>

I don't think anybody is suggesting changing this without a paper :-)



SG12 list run by sg12-owner@lists.isocpp.org