Hacker News Reader

Comment by WalterBright | Hacker News Reader

The C version:

    int* foo(int bar) {
        int* return_value = NULL;

        if (!do_something(bar))
            goto error_1;
        if (!init_stuff(bar))
            goto error_2;
        if (!prepare_stuff(bar))
            goto error_3;
        return_value = do_the_thing(bar);

    error_3:       cleanup_3();
    error_2:       cleanup_2();
    error_1:       cleanup_1();

        return return_value;
    }

The D version:

    int* foo(int bar) {
        scope(exit) cleanup1();
        if (!do_something(bar))
            return null;

        scope(exit) cleanup2();
        if (!init_stuff(bar))
            return null;

        scope(exit) cleanup3();
        return do_the_thing(bar);
    }

https://dlang.org/articles/exception-safe.html

View on HN

I find that after writing a lot of Go, manually having to defer/scope(exit) is a lot more error prone than just RAII destructors: it’s impossible to forget to defer the destructor.

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

That seems like an inside-out way of doing it to me. I would schedule work onto the transaction struct and make it ultimately responsible for if it should roll back the work or keep it committed.

   let mut tx = Transaction::new();
   dofoo(&mut tx)?;
   dobar(&mut tx)?;
   tx.commit();

There is some overhead to boxing the rollback functions for dofoo/dobar into the transaction object, but it's far less error prone (or maybe you can avoid the boxing by encoding all the rollback operations at the type level: less ergonomic but not by much).

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.

In C# it's customary just to wave away the worst kinds of problem that C and D developers try to handle, and let the runtime kill your program. (This is more an artifact of why people pick their languages than anything inherent on the languages themselves.)

But rest assured, your C# code is full of global state hidden on its runtime and is subject to the same kinds of errors people are discussing here.

"hidden on its runtime" keeps it out of the rest of thr program.

The entire program still has the same failure modes. If you wanted to handle them, you would get the same problems.

Plain C version with attribute((__cleanup)):

    int* foo(int bar) {
        int* return_value = NULL;

        __cleanup__((cleanup_1)) int c1 = 0;
        if (!do_something(bar))
            return NULL;
        __cleanup__((cleanup_2)) int c2 = 0;
        if (!init_stuff(bar))
            return NULL;
        __cleanup__((cleanup_3)) int c3 = 0;
        if (!prepare_stuff(bar))
            return NULL;
        return_value = do_the_thing(bar);

        return return_value;
    }

That's GCC, not standard C, right?

Clang works too. Not sure about the rest.

Do scopes work without linking against the D runtime?

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

It's still wrong by default.

The correct pattern here is RAII, which requires no explicit cleanup code so there's no forgetting to use it.

D does support RAII. But RAII has a problem: If you want transactions A and B to be both successful, or both are unwound, using RAII is a clumsy technique. It gets much worse if you need A, B and C to either all succeed or all fail. This article goes into detail:

https://dlang.org/articles/exception-safe.html

The article specifically talks about C, which does not have RAII.

Reply on news.ycombinator.com