GOTOphobia considered harmful in C

“Goto fail” would have been wrong in a non-goto language that use RAII.

The problem was not goto, it was that the else path was also “give up”. In a non goto/RAII/defer language it would have been something like

    If (error)
        Return
        Return

My assumption has been this was some kind of merge error rather being wrong off the bay. Interestingly mandatory indenting or mandatory braces might have stopped this, but then i would have thought -Werror with the dead code warnings would have as well :-/

But again the error was not the goto, and believing it was is the exact problem the article is talking about: people are so opposed to goto they are unable to see real issues (I have seen C code that tries to avoid goto completely and error handling code becomes horrific, far more complex, and far more error prone that just using goto). The problem with goto is that it is very easy to use it unnecessarily, in ways that complicated control flow but don’t actually make things better.

The trouble with RAII is when it's necessary to unwind the transactions. See the article I linked to.

yes, for `scope(exit)`. Here's the assembler generated:

                push    RBP
                mov     RBP,RSP
                sub     RSP,020h
                mov     -020h[RBP],RBX
                mov     -018h[RBP],R12
                mov     -010h[RBP],R13
                mov     -8[RBP],EDI
                mov     EBX,-8[RBP]
                mov     EDI,EBX
                call      _D5test312do_somethingFiZi@PC32
                test    EAX,EAX
                jne     L2F
                xor     R13D,R13D
                mov     EBX,1
                jmp short       L79
    L2F:        mov     EDI,EBX
                call      _D5test310init_stuffFiZi@PC32
                test    EAX,EAX
                jne     L4A
                xor     R13D,R13D
                mov     R12D,4
                mov     EBX,4
                jmp short       L68
    L4A:        mov     -8[RBP],EBX
                mov     EDI,-8[RBP]
                call      _D5test312do_the_thingFiZPi@PC32
                mov     R13,RAX
                mov     R12D,7
                mov     EBX,7
                call      _D5test38cleanup3FZv@PC32
    L68:        call      _D5test38cleanup2FZv@PC32
                cmp     R12D,4
                je      L79
                cmp     R12D,7
                jne     L8B
    L79:        call      _D5test38cleanup1FZv@PC32
                cmp     EBX,1
                je      L8B
                cmp     EBX,4
                je      L8B
                cmp     EBX,7
    L8B:        mov     RAX,R13
                mov     RBX,-020h[RBP]
                mov     R12,-018h[RBP]
                mov     R13,-010h[RBP]
                mov     RSP,RBP
                pop     RBP
                ret

I have a distaste for all of these examples, which comes from the existence of a side-effecting operation: calling do_something() necessitates the need to call a cleanup function, which means there's some state being changed but hidden behind the internals of these methods. It is really easy to call this incorrectly which says to me it's just a badly-designed API.

In C# the idiomatic way would be to have each of these 3 things be defined in a class using IDisposable, which is similar to D's scope() -- the declaring class gets a cleanup method when the variable goes out of scope, no matter how that happens.

I assume there's some interaction between these classes, but IMHO that should be explicitly defined and so the code would look something like:

    public void foo(int bar) {
        using var something = new Something(bar)
        if (something.do()) {
            using var stuff = new Stuff(bar);
            if (stuff.init()) {
                using var stuff2 = new Stuff2(bar); // two "stuff"s looks dumb but this is example code
                if (stuff2.prepare()) {
                    return do_the_thing(something, stuff, stuff2, bar);
                }
            }
        }

        return null;
    }

There's actually several ways to structure this code which would result in something that looks better than the above, but being example code and not knowing how `something` and `stuff` interact, it's hard to write this nicely. I'd probably aim for something much more concise like:

    public void foo(int bar) {
        using var something = new Something(bar);
        using var stuff = new Stuff(something);
        using var stuff2 = new Stuff2(stuff);

        return stuff2.prepare() ? do_the_thing(stuff2) : null;
    }

In the above, I assume stuff2.prepare() calls everything it needs to on the dependent objects, but how I'd structure this for real entirely depends on what they're actually doing.

Expressions that aren't required to return a value for all possible branches of execution that can also mutate state considered harmful.