top | item 25419995

(no title)

rseacord | 5 years ago

Based on committee discussion, I think it is unlikely we will attempt to capture the values. The capture will most likely be done by reference.

This case of the defer in the loop is frequently cited, probably because it is a problematic case. However, I looked at a lot of real code and the only case I found of resources being allocated in a loop they were allocated at the beginning of the loop and deallocated at the end. Another option we are considering is to use the scope for the guarded block. In this case, deferred statements would be executed at the end of each iteration of the for loop which would be ideal for this sort of code. For example, you could rewrite this function using defer:

https://github.com/openssl/openssl/blob/a829b735b645516041b5...

like this:

    for (;;) {
        raw = 0;
        ptype = 0;
        i = PEM_read_bio(bp, &name, &header, &data, &len);
        defer {
          OPENSSL_free(name);
          name = NULL;
          OPENSSL_free(header);
          header = NULL;
          OPENSSL_free(data);
          data = NULL;
        }
 ...
        } else {
            /* unknown */
        }
    } // end for loop, run deferred statements

discuss

order

mskslal|5 years ago

Agreed. In Go, defers accumulate and are all called at the end of a function, which I believe is a mistake (but there are practical uses for it). I think it would be much more fitting for C just to have the defer happen at the end of the scope. It would be very easy for compiler devs to implement it too--that above example could be done just by taking the defer block and pasting it at the end of the scope, with zero overhead. More complex control flow can be implemented with a simple goto.

eqvinox|5 years ago

The fact that the loop case is rare does not mean it isn't still problematic. It's creating a new opportunity to shoot yourself in the foot while trying to solve another one. (This is with the separate guard{} block.)

I would strongly agree that the dedicated guard{} block is a bad idea and this should just tie into the innermost scope. I see what this is trying to do ("if (...) { foo.x = malloc(); defer free(foo.x); }") but you don't solve a UI problem (as in, user interface for the programmer to their code) by adding more weird UI.

("Worst" example for shooting yourself in the foot: defer inside macros. Programmer then forgets that the macro contains a defer, and the defer defaults to the function-implicit outer guard block. But it's really in a nested loop. That's gonna be a fun week of debugging... much less of a risk when you have the guarantee in terms of innermost scope.)

gpderetta|5 years ago

Closures capturing iteration variables by reference is a common footgun in many languages unfortunately.

dooglius|5 years ago

The simple solution to me seems to be to just ban use of loops hierarchically between a defer statement and a guard block. The guard block should definitely be kept as using scope seems like it confuses and complicates the notion of scope; for instance, it seems like a footgun if "if(some condition that turns out to be always true) {blah blah}" can't be refactored into "blah blah". Also, using scope seems to make it difficult to use defer in macros.

gustedt|5 years ago

That is effectively one of the options that is under discussion. One of the advantage of the approach with defer is really that everything happens in the open and all uses that one would want to classify as misuse can easily be flagged by the compiler.