TMI: I'm out of puns about destructors

[tim]

I skipped writing a post yesterday, but fortunately I didn't work on anything all that different from today: which is, destructors, all the time!

Porting the run-fail tests to use classes instead of resources mostly went smoothly, with the exception of the test morestack3, which after porting looked like (in part):

fn getbig_and_fail(&&i: int) {
     let _r = and_then_get_big_again();
[snip]
 }

class and_then_get_big_again {
[snip]

I redacted the parts that aren't so relevant. The relevant bit is that the function get_big_and_fail appears before the definition of the class and_then_get_big_again. This caused the compiler to complain that the name and_then_get_big_again was undefined. Apparently, all of the test cases with classes so far have placed the class definition before any uses of its constructor. The problem was in trans (that is, the code generator) and had to do with a few cooperating functions whose relations I still don't entirely understand (get_item_val is one), where there was an unstated invariant that one would always be called before the other, but that only actually held true if the class declaration appeared before any uses of it. So I fixed that to allow classes to appear in any order, which is correct according to Rust semantics, where all top-level items are mutually recursive with each other. And that was actually the only bug I needed to fix to port all the run-fail tests!

Next, I started porting the standard libraries to use classes instead of resources. The first uses of resources (using the time-honored heuristic of alphabetical order) appeared in core::comm, implementing Rust's ports (for communication between tasks). Once I changed over the comm code, I got an internal compiler error in trans as well, which took a while to narrow down, but the problem was entirely my fault. At runtime, instances of classes with destructors have a different representation than classes that don't have destructors: it's a two-word representation with a field that's a "drop flag" (that gets set when a destructor is invoked, ensuring the same destructor doesn't run twice) and the other field being a pointer to the actual class itself. That means that trans has to insert code to create the drop flag and, where necessary, extract out the second field, so that code that manipulates classes doesn't have to know whether there's a destructor or not. I was doing this by inserting a field selection on any reference to a variable with a "class with a dtor" type (I really should think of a better name for that), but that didn't work in the case where a closure captured a variable with such a type, since the closure needs to capture the entire record including the drop flag. So instead, I made the code that translates record field accesses include an extra selection to ignore the drop flag, and everything worked. I don't know why I didn't just do that in the first place...

After fixing that, the next bug was a metadata bug, but that meant I'd gotten all the way through libcore! Hooray! The problem arose in whichever libstd modules use core::comm: recall that Rust's compiles polymorphism by monomorphization, so it inlines all generic functions. In particular, this function that constructs a port:

fn port<T: send>() -> port {
    port_t(@port_ptr(rustrt::new_port(sys::size_of::<T>() as size_t)))
}

is generic -- it has a type parameter T (that is constrained to be sendable, but that doesn't matter here). port_ptr is now a class constructor:

class port_ptr<T:send> {
  let po: *rust_port;
  new(po: *rust_port) { self.po = po; }
  drop unsafe {

(redacting the details of the destructor since it's long and the specifics aren't relevant). port_ptr is essentially the private representation of ports, which comm doesn't export -- but that's exactly what caused the problem, since code in a different crate had an inlined copy of port, but had no information about port_ptr and thus couldn't translate the resulting inlined instance. There is code that's part of serialization that should figure out that port_ptr needs to be serialized too, even though it's not explicitly exported -- but I don't quite understand how it works; the metadata printer appears to skip items that are either unreachable (as in, not mentioned or transitively mentioned by any code that's exported) or not exported, but it seems to me that it should only skip items that are both unreachable or not exported. Sadly, the people who I could ask were busy when I was looking at this... so in the meantime, I decided to just go ahead and try changing the or to an and, knowing I'll check with them before I commit the code.

Once I made that change, the next bug was that the reachability analysis just was never designating class constructors as reachable -- just an overlooked case in a pattern-match. And once I fixed that bug, I got a bug to do with looking up the type for a class field defined in a different class and not finding it, which I made a bit of a questionable fix for (by rewriting some code, specifically in ty::is_instantiable... but I think the original version should have worked too, so it's questionable).

The next bug was a dreaded memory leak, which looks like:

compile_and_link: x86_64-apple-darwin/stage1/lib/rustc/x86_64-apple-darwin/lib/libcore.dylib
warning: no debug symbols in executable (-arch x86_64)
leaked memory in rust main loop (4 objects)
Assertion failed: (false), function ~memory_region, file /Users/TimChevalier/rust/src/rt/memory_region.cpp, line 172.

This means there's a bug in either the code that manages automatic reference counting, or in the cycle collector, that resulted in some object being allocated but never freed (specifically, 4 objects, in this case). Since I don't know the RTS code very well, I've always found it pretty painful to debug these errors (one time when I did it successfully, I spent days on it, and it involved minimizing my code till it was small enough that I could look at the generated code and see the missing drop -- not easy when you're starting from the entire compiler as your test case). To minimize my confusion, I decided to finish making a compiler snapshot (because rustc is self-hosting, there's a slightly arcane ritual to do whenever we add a feature or fix a bug such that we need a new "stage 0" compiler) rather than working in two branches and manually copying files back and forth between different parts of my build tree. Unfortunately, this involves waiting for all the buildbots to finish running... and I'm still waiting.