Merge pull request #644 from leftibot/fix/issue-421-switch-type-conversion

Fix #421: Switch with type_conversion compares destroyed objects

include/chaiscript/language/chaiscript_eval.hpp

@@ -220,15 +220,16 @@ namespace chaiscript {
       Boxed_Value eval_internal(const chaiscript::detail::Dispatch_State &t_ss) const override {
         auto lhs = this->children[0]->eval(t_ss);
         auto rhs = this->children[1]->eval(t_ss);
-        return do_oper(t_ss, m_oper, this->text, lhs, rhs);
+        return do_oper(t_ss, m_oper, this->text, lhs, rhs, m_loc);
       }
 
-    protected:
-      Boxed_Value do_oper(const chaiscript::detail::Dispatch_State &t_ss,
+      // static and public so we can use this to process Switch_AST_Node case equality
+      static Boxed_Value do_oper(const chaiscript::detail::Dispatch_State &t_ss,
                           Operators::Opers t_oper,
                           const std::string &t_oper_string,
                           const Boxed_Value &t_lhs,
-                          const Boxed_Value &t_rhs) const {
+                          const Boxed_Value &t_rhs,
+                          std::atomic_uint_fast32_t &t_loc) {
         try {
           if (t_oper != Operators::Opers::invalid && t_lhs.get_type_info().is_arithmetic() && t_rhs.get_type_info().is_arithmetic()) {
             // If it's an arithmetic operation we want to short circuit dispatch
@@ -243,7 +244,7 @@ namespace chaiscript {
             chaiscript::eval::detail::Function_Push_Pop fpp(t_ss);
             std::array<Boxed_Value, 2> params{t_lhs, t_rhs};
             fpp.save_params(Function_Params(params));
-            return t_ss->call_function(t_oper_string, m_loc, Function_Params(params), t_ss.conversions());
+            return t_ss->call_function(t_oper_string, t_loc, Function_Params(params), t_ss.conversions());
           }
         } catch (const exception::dispatch_error &e) {
           throw exception::eval_error("Can not find appropriate '" + t_oper_string + "' operator.", e.parameters, e.functions, false, *t_ss);
@@ -985,8 +986,7 @@ namespace chaiscript {
             if (this->children[currentCase]->identifier == AST_Node_Type::Case) {
               // This is a little odd, but because want to see both the switch and the case simultaneously, I do a downcast here.
               try {
-                std::array<Boxed_Value, 2> p{match_value, this->children[currentCase]->children[0]->eval(t_ss)};
-                if (hasMatched || boxed_cast<bool>(t_ss->call_function("==", m_loc, Function_Params{p}, t_ss.conversions()))) {
+                if (hasMatched || boxed_cast<bool>(Binary_Operator_AST_Node<T>::do_oper(t_ss, Operators::Opers::equals, "==", match_value, this->children[currentCase]->children[0]->eval(t_ss), m_loc))) {
                   this->children[currentCase]->eval(t_ss);
                   hasMatched = true;
                 }

unittests/compiled_tests.cpp

@@ -1304,3 +1304,97 @@ TEST_CASE("Test if non copyable/movable types can be registered") {
   chai.add(chaiscript::user_type<Nothing>(), "Nothing");
   chai.add(chaiscript::constructor<Nothing()>(), "Nothing");
 }
+
+// Issue #421: Class with type_conversion from int and "==" operator
+// causes switch statement to compare destroyed objects.
+// The switch case comparison must use Function_Push_Pop to properly
+// manage the lifetime of temporaries created by type conversions.
+TEST_CASE("Issue #421 - Switch with type_conversion does not compare destroyed objects") {
+  struct MyType {
+    int value;
+    explicit MyType(int v) : value(v) {}
+  };
+
+  chaiscript::ChaiScript_Basic chai(create_chaiscript_stdlib(), create_chaiscript_parser());
+  chai.add(chaiscript::user_type<MyType>(), "MyType");
+  chai.add(chaiscript::constructor<MyType(int)>(), "MyType");
+  chai.add(chaiscript::fun(&MyType::value), "value");
+  chai.add(chaiscript::fun([](const MyType &a, const MyType &b) { return a.value == b.value; }), "==");
+  chai.add(chaiscript::type_conversion<int, MyType>([](const int &i) { return MyType(i); }));
+
+  // Test switch with type conversion - the case integer literals must be
+  // converted to MyType via the registered conversion before comparison.
+  // Without Function_Push_Pop, the converted temporaries may be destroyed
+  // before the == operator can compare them.
+  CHECK(chai.eval<int>(R"({
+    var result = 0
+    var obj = MyType(2)
+    switch(obj) {
+      case (1) {
+        result = 1
+        break
+      }
+      case (2) {
+        result = 2
+        break
+      }
+      case (3) {
+        result = 3
+        break
+      }
+    }
+    result
+  })") == 2);
+
+  // Test fall-through with type conversion
+  CHECK(chai.eval<int>(R"({
+    var total = 0
+    var obj = MyType(2)
+    switch(obj) {
+      case (1) {
+        total += 1
+      }
+      case (2) {
+        total += 2
+      }
+      case (3) {
+        total += 4
+      }
+    }
+    total
+  })") == 6);
+
+  // Test matching the first case
+  CHECK(chai.eval<int>(R"({
+    var result = 0
+    var obj = MyType(1)
+    switch(obj) {
+      case (1) {
+        result = 10
+        break
+      }
+      case (2) {
+        result = 20
+        break
+      }
+    }
+    result
+  })") == 10);
+
+  // Test no match
+  CHECK(chai.eval<int>(R"({
+    var result = 0
+    var obj = MyType(5)
+    switch(obj) {
+      case (1) {
+        result = 1
+        break
+      }
+      case (2) {
+        result = 2
+        break
+      }
+    }
+    result
+  })") == 0);
+}

unittests/switch_type_conversion.chai

@@ -0,0 +1,50 @@
+// Test for issue #421: switch statement with custom == operator on
+// dynamic objects must properly manage object lifetimes during
+// case comparisons.
+
+class MyType {
+  var value
+  def MyType(v) { this.value = v }
+}
+
+def `==`(a, b) : a.is_type("MyType") && b.is_type("MyType") {
+  return a.value == b.value
+}
+
+var result = 0
+var obj = MyType(2)
+
+switch(obj) {
+  case (MyType(1)) {
+    result = 1
+    break
+  }
+  case (MyType(2)) {
+    result = 2
+    break
+  }
+  case (MyType(3)) {
+    result = 3
+    break
+  }
+}
+
+assert_equal(result, 2)
+
+// Also test fall-through with custom == operator
+var total = 0
+var obj2 = MyType(2)
+
+switch(obj2) {
+  case (MyType(1)) {
+    total += 1
+  }
+  case (MyType(2)) {
+    total += 2
+  }
+  case (MyType(3)) {
+    total += 4
+  }
+}
+
+assert_equal(total, 6)