Fix trans cleanups to be more flexible

[nikomatsakis]

Right now trans introduces cleanup scopes at semi-arbitrary times. This was fine when first designed but now it is user visible because it affects the maximum lifetimes of values and hence causes errors in the borrow checker.

It interacts particularly poorly with @mut freezes. For example, if I write:

let some_map: @mut HashMap<K,V> = ...;
let foo = *some_map.get(some_key);

this will cause some_map to be frozen for the entire enclosing block rather than just the let foo statement, because trans would not introduce a cleanup scope there. I at least issue a rather cryptic warning in this case (citing this issue #). The workaround is to introduce a block or a method:

// Option 1: add a method that combines the `*` and the call to `get` into one thing
// In this case, HashMap defines get_copy:
let foo = some_map.get_copy(some_key);

// Option 2: introduce a block and store the borrowed @mut into a local variable
let foo = {
    let some_map = &mut *some_map;
    *some_map.get(some_key)
};

We should permit cleanups after every expression exits. If we do this with the existing system, though, it'll cause a flood of basic blocks and jack up compilation times significantly. I want to modify the system so that rather than adding cleanups to a block scope, as we do now, you would instead associate cleanups with an expr id (in trans, I mean). The cleanup generation would thus be driven off the AST. I have added various FIXMEs in the code referencing this issue or #3511 where this might be relevant.

[pnkfelix]

visited for bug triage, 16 july 2013. Looks like just a work tracking item, no need for nomination (at least not beyond what is already tracked in #3511).

[graydon]

Curiously I was assigned this one too (triage email jul 15) -- dupe triage assignment? @pnkfelix can you send me your triage email so I can try to debug this? The triage script is not supposed to be doing it, but it's been reported a few times now.

[pnkfelix]

@graydon I'm not able to keep up with the same amount of triage every week (free time ebbs and flows), so sometimes I go through a backlog. I'll happily send you the emails I've gotten, but the collision here isn't necessarily a bug; at the start of this week, I had a backlog list with three weeks of triage but had only visited four bugs.

In this case, this particular bug was assigned to me in last week's triage, not this week's.

[pnkfelix]

(while I understand that such collisions can be annoying, I assume its better for me to attempt to attempt to catch up with a backlog than to leave them untouched. . . not ideal of course, since it could lead to an overall unfair distribution of labor, but for the triaging task, I think each individual can decide how much effort they want to actually put in on each bug, and I think going through the backlog is likely to yield a fairer distribution (even if unfair) than leaving them untouched.)

[pnkfelix]

@graydon I will try to use the date of the triage email in the future when I mark the bugs, rather than the current date. That should make you able to tell if/when there is a real collision; sound reasonable?

[graydon]

@pnkfelix ah no problem. I think this is what keeps happening (people drop a bug, pick it up next week but someone else has got it reassigned, then surprised to think they are colliding when really it's functioning as intended). If that's all we're seeing here, no worries.

[pcwalton]

This caused a soundness bug in Servo. I nominate this to block 1.0.

[alexcrichton]

Do you have a small example which exposes the soundness bug? I'd want to confirm it before we decide to block on it.

[pcwalton]

https://gist.github.com/pcwalton/7203881

[nikomatsakis]

As I told @pcwalton on IRC, I have a patch for this about 50% done. This patch also fixes #3511 (temporary lifetimes) and is a first step towards completing #8861 (more expressive destructors) which in turn enables RAII and thus completion of #2202 (closure reform). So I agree with the nomination.

[pnkfelix]

accepted for 1.0. Semantics of when dtors run puts it in backcompat-lang

[pcwalton]

@nikomatsakis What do we still have to do here?

[nikomatsakis]

I think ... this is done.