[BUG] destructor is called twice when unwinding

[neumannt]

The destructor of cpp2::out destroys the object if an exception has been called, without resetting the init flag. This causes the destructor to be called twice. See below for a code example.

There are basically three possible ways to fix the problem. (I am happy to provide a pull request, but I would need to know what the desired behavior should be):

1. the out destructor resets the init flag. That is the easiest fix, and avoids the problem, but has the problem that this is not composable. What if you pass an out parameter as out argument to another function? (currently not supported by cppfront, but clearly needed). Then you cannot update the init and constructed flags easily any more, as the chain can be arbitrarily long.
2. the out destructor never destroys the object. This avoids the problem, too, the only disadvantage is that in the case of exceptions we do not know if the out object was constructed at all. But the compiler knows that the state of the object is uncertain, and can forbid accesses to the object, just as with uninitialized variables. The destructor is guaranteed to be called be deferred_init.
3. we require that out parameters must be called with uninitialized variables (or other out parameters). This eliminates the has_t case from out, and allows us to unconditionally call the constructor instead of the assignment operator. Everything becomes composable, and we can safely destroy created objects from the destructor ouf out (by inspecting the init flag of deferred_init). This has the additional benefit of fitting nicely with the named constructor concept of D0708.

My personal preference order would be 3, 2, 1.

struct Foo {
   Foo() { std::cerr << "constructing " << this << std::endl; }
   ~Foo() { std::cerr << "destroying " << this << std::endl; }
};

void f1() { throw 1; }

f2: (out x:Foo) = { x=(); f1(); }
f3: () = { x:Foo; f2(out x); }

int main() {
   try { f3(); } catch (int) {}
}

[gregmarr]

@neumannt Why do you think that 1 is not composable? The object that called the constructor then calls the destructor and clears the init flag so that the deferred_init object knows it has already been destroyed.

[neumannt]

Consider this code snippet here:

f1 : () = {
   x: Foo;
   f2(out x);
}
f2: (out x : Foo) = { f3(out x); }
f3: (out x : Foo) = {
  f4(out x);
  fThatThrows();
}
f4: (out x: Foo) = { f5(out x); }
f5: (out x: Foo) = { x = (1,2); }

void fThatThrows() { throw 1; }

Note that in mode 1) the x object must be destroyed when leaving f3 due to an exception, even though f3 did not construct that object. I do not see how that can be implemented efficiently unless we require that x must be uninitialized when calling f3 (which is the mode 3 that I proposed). Note that these call chains can be arbitrarily deep.

[gregmarr]

Why must it be destroyed when leaving f3? Oh, because it was constructed in a lower function. Maybe instead of recording "I constructed it", out should be recording "it was not yet constructed". Then the first object that is destroyed via exception and that has the "it was not yet constructed" flag set will destroy it, and clear the init flag.

[neumannt]

Why must it be destroyed when leaving f3? Oh, because it was constructed in a lower function. Maybe instead of recording "I constructed it", out should be recording "it was not yet constructed". Then the first object that is destroyed via exception and that has the "it was not yet constructed" flag set will destroy it, and clear the init flag.

indeed, that would work. You still have the assignment vs. construction problem, though, in the current model out parameters require an assignment operator. It would be nicer if we could construct unconditionally, that would allow for, e.g., much nicer factory functions.

[hsutter]

Thanks! I've fixed the bug, added out chaining, and added all the above test cases... let me know if I missed anything.