Fix out double destruction bug, added support for out chaining, closes #109

Fixed `out::construct*`` to actually forward
Just unconditionally don't include `<execution>`

Author: hsutter

include/cpp2util.h

@@ -131,11 +131,6 @@
         #include <iterator>
         #include <ranges>
         #include <algorithm>
-        // libstdc++ sometime after GCC 10 got a dependency on linking TBB if <execution> is
-        // included, so let's not pull in that header in our "import std;" simulation mode
-        #if not defined(__GLIBCXX__) || __GLIBCXX__ < 20210101
-            #include <execution>
-        #endif
         #include <bit>
         #include <cfenv>
         #include <cmath>
@@ -185,6 +180,15 @@
         #endif
         #include <thread>
         #include <iso646.h>
+
+        // libstdc++ currently has a dependency on linking TBB if <execution> is
+        // included, and TBB seems to be not automatically installed and linkable
+        // on some GCC installations, so let's not pull in that little-used header
+        // in our -pure-cpp2 "import std;" simulation mode... if you need this,
+        // use mixed mode (not -pure-cpp2) and #include all the headers you need
+        // including this one
+        // 
+        // #include <execution>
     #endif
 
 //  Otherwise, we're not in -pure-cpp2 and so just #include
@@ -395,9 +399,11 @@ class deferred_init {
     template<typename U>
     friend class out;
 
+    auto destroy() -> void         { if (init) { t().~T(); }  init = false; }
+
 public:
     deferred_init() noexcept       { }
-   ~deferred_init() noexcept       { if (init) t().~T(); }
+   ~deferred_init() noexcept       { destroy(); }
     auto value()    noexcept -> T& { Default.expects(init);  return t(); }
 
     auto construct     (auto&& ...args) -> void { Default.expects(!init);  new (&data) T(std::forward<decltype(args)>(args)...);  init = true; }
@@ -411,46 +417,79 @@ class out {
         T* t;
         deferred_init<T>* dt;
     };
-    int  uncaught_count   = std::uncaught_exceptions();
+    out<T>* ot = nullptr;
     bool has_t;
-    bool called_construct = false;
+
+    //  Each out in a chain contains its own uncaught_count ...
+    int  uncaught_count   = std::uncaught_exceptions();
+    //  ... but all in a chain share the topmost called_construct_
+    bool called_construct_ = false;
 
 public:
-    out(T* t)                 noexcept : t{t},   has_t{true}  { }
-    out(deferred_init<T>* dt) noexcept : dt{dt}, has_t{false} { }
+    out(T*                 t) noexcept :  t{ t}, has_t{true}      { Default.expects( t); }
+    out(deferred_init<T>* dt) noexcept : dt{dt}, has_t{false}     { Default.expects(dt); }
+    out(out<T>*           ot) noexcept : ot{ot}, has_t{ot->has_t} { Default.expects(ot);
+        if (has_t) {  t = ot->t;  }
+        else       { dt = ot->dt; }
+    }
+
+    auto called_construct() -> bool& {
+        if (ot) { return ot->called_construct(); }
+        else    { return called_construct_; }
+    }
 
     //  In the case of an exception, if the parameter was uninitialized
     //  then leave it in the same state on exit (strong guarantee)
     ~out() {
-        if (called_construct && uncaught_count != std::uncaught_exceptions()) {
+        if (called_construct() && uncaught_count != std::uncaught_exceptions()) {
             Default.expects(!has_t);
-            dt->value().~T();
+            dt->destroy();
+            called_construct() = false;
         }
     }
 
-    auto construct     (auto ...args) -> void {
+    auto construct(auto&& ...args) -> void {
         if (has_t) {
-            *t = T(args...);
-        }
-        else if (dt->init) {
-            dt->value() = T(args...);
+            Default.expects( t );
+            *t = T(std::forward<decltype(args)>(args)...);
         }
         else {
-            dt->construct(args...);
-            called_construct = true;
+            Default.expects( dt );
+            if (dt->init) {
+                dt->value() = T(std::forward<decltype(args)>(args)...);
+            }
+            else {
+                dt->construct(std::forward<decltype(args)>(args)...);
+                called_construct() = true;
+            }
         }
     }
 
-    auto construct_list(auto ...args) -> void {
+    auto construct_list(auto&& ...args) -> void {
         if (has_t) {
-            *t = T{args...};
+            Default.expects( t );
+            *t = T{std::forward<decltype(args)>(args)...};
+        }
+        else {
+            Default.expects( dt );
+            if (dt->init) {
+                dt->value() = T{std::forward<decltype(args)>(args)...};
+            }
+            else {
+                dt->construct_list(std::forward<decltype(args)>(args)...);
+                called_construct() = true;
+            }
         }
-        else if (dt->init) {
-            dt->value() = T{args...};
+    }
+
+    auto value() noexcept -> T& {
+        if (has_t) {
+            Default.expects( t );
+            return *t;
         }
         else {
-            dt->construct_list(args...);
-            called_construct = true;
+            Default.expects( dt );
+            return dt->value();
         }
     }
 };
@@ -860,8 +899,8 @@ class c_raii {
 
     c_raii(c_raii const&)         = delete;
     auto operator=(c_raii const&) = delete;
-    c_raii(c_raii&& that)         : t  {std::move(that.t)}, dtor  {that.dtor} { that.dtor = nullptr; }
-    auto operator=(c_raii&& that) { t = std::move(that.t);  dtor = that.dtor;   that.dtor = nullptr; }
+    c_raii(c_raii&& that)         noexcept : t  {std::move(that.t)}, dtor  {that.dtor} { that.dtor = nullptr; }
+    auto operator=(c_raii&& that) noexcept { t = std::move(that.t);  dtor = that.dtor;   that.dtor = nullptr; }
 };
 
 inline auto fopen( const char* filename, const char* mode ) {

regression-tests/mixed-out-destruction.cpp2

@@ -0,0 +1,38 @@
+#include <iostream>
+
+struct X {
+    inline static int Xnum = 0;
+    int num;
+    X() : num{++Xnum}                { std::cout << "+X " << num << "\n"; }
+    ~X()                             { std::cout << "-X " << num << "\n"; }
+    X(const X& that) : num{that.num} { std::cout << "+X copy " << num << "\n"; }
+    void operator=(const X& that)    { num = that.num; std::cout << "=X copy " << num << "\n"; }
+};
+
+auto throw_1() { throw 1; }
+
+struct C {
+    std::string f;
+    C(std::string const& fn) : f{fn} { std::cout << "enter " << f << "\n"; }
+    ~C()                             { std::cout << "exit " << f << "\n"; }
+};
+
+//-------------------------------------------------------
+// 0x: Test one level of out and immediate throw
+f00: ()         = { c:C="f00";  x: X;  f01(out x); }
+f01: (out x: X) = { c:C="f01";  x=();  throw_1();  }
+
+//-------------------------------------------------------
+// 1x: Test multiple levels of out and intermediate throw
+f10: ()         = { c:C="f10";  x: X;  f11(out x); }
+f11: (out x: X) = { c:C="f11";         f12(out x); }
+f12: (out x: X) = { c:C="f12";         f13(out x);  throw_1(); }
+f13: (out x: X) = { c:C="f13";         f14(out x); }
+f14: (out x: X) = { c:C="f14";  x=();  }
+
+int main() {
+    C c("main");
+    try { f00(); } catch (int) {}
+    std::cout << "\n";
+    try { f10(); } catch (int) {}
+}

regression-tests/test-results/mixed-out-destruction.cpp

@@ -0,0 +1,56 @@
+// ----- Cpp2 support -----
+#include "cpp2util.h"
+
+#line 1 "mixed-out-destruction.cpp2"
+#include <iostream>
+
+struct X {
+    inline static int Xnum = 0;
+    int num;
+    X() : num{++Xnum}                { std::cout << "+X " << num << "\n"; }
+    ~X()                             { std::cout << "-X " << num << "\n"; }
+    X(const X& that) : num{that.num} { std::cout << "+X copy " << num << "\n"; }
+    void operator=(const X& that)    { num = that.num; std::cout << "=X copy " << num << "\n"; }
+};
+
+auto throw_1() { throw 1; }
+
+struct C {
+    std::string f;
+    C(std::string const& fn) : f{fn} { std::cout << "enter " << f << "\n"; }
+    ~C()                             { std::cout << "exit " << f << "\n"; }
+};
+
+#line 22 "mixed-out-destruction.cpp2"
+auto f00() -> void;
+auto f01(cpp2::out<X> x) -> void;
+#line 27 "mixed-out-destruction.cpp2"
+auto f10() -> void;
+auto f11(cpp2::out<X> x) -> void;
+auto f12(cpp2::out<X> x) -> void;
+auto f13(cpp2::out<X> x) -> void;
+auto f14(cpp2::out<X> x) -> void;
+
+int main() {
+    C c("main");
+    try { f00(); } catch (int) {}
+    std::cout << "\n";
+    try { f10(); } catch (int) {}
+}
+
+//=== Cpp2 definitions ==========================================================
+
+#line 19 "mixed-out-destruction.cpp2"
+
+//-------------------------------------------------------
+// 0x: Test one level of out and immediate throw
+auto f00() -> void{ C c { "f00" }; cpp2::deferred_init<X> x; f01(&x);}
+auto f01(cpp2::out<X> x) -> void{C c { "f01" }; x.construct();throw_1();}
+
+//-------------------------------------------------------
+// 1x: Test multiple levels of out and intermediate throw
+auto f10() -> void{ C c { "f10" }; cpp2::deferred_init<X> x; f11(&x);}
+auto f11(cpp2::out<X> x) -> void{C c { "f11" }; f12(&x);}
+auto f12(cpp2::out<X> x) -> void{C c { "f12" }; f13(&x);throw_1();}
+auto f13(cpp2::out<X> x) -> void{C c { "f13" }; f14(&x);}
+auto f14(cpp2::out<X> x) -> void{C c { "f14" }; x.construct();}

regression-tests/test-results/mixed-out-destruction.cpp2.output

@@ -0,0 +1,2 @@
+mixed-out-destruction.cpp2... ok (mixed Cpp1/Cpp2, Cpp2 code passes safety checks)
+

regression-tests/test-results/msvc-2022/mixed-out-destruction.cpp.execution

@@ -0,0 +1,21 @@
+enter main
+enter f00
+enter f01
++X 1
+exit f01
+-X 1
+exit f00
+
+enter f10
+enter f11
+enter f12
+enter f13
+enter f14
++X 2
+exit f14
+exit f13
+exit f12
+-X 2
+exit f11
+exit f10
+exit main

regression-tests/test-results/msvc-2022/mixed-out-destruction.cpp.output

@@ -0,0 +1 @@
+mixed-out-destruction.cpp