Fix #339: Prevent implicit move from lvalue references during dispatch

Cast_Helper_Inner<T&&> unconditionally called std::move() on the underlying
object, even when the Boxed_Value held an lvalue reference (is_ref() == true).
This caused T&& overloads to win over const T& overloads for objects returned
by reference, silently moving from and destroying persistent objects. The fix
rejects the T&& cast for reference Boxed_Values, allowing dispatch to fall
through to the correct const T& overload.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
leftibot 2026-04-13 19:47:57 -06:00
parent d4c5bdb3e4
commit 67c98814d2
2 changed files with 79 additions and 0 deletions

View File

@ -122,6 +122,9 @@ namespace chaiscript {
template<typename Result>
struct Cast_Helper_Inner<Result &&> {
static Result &&cast(const Boxed_Value &ob, const Type_Conversions_State *) {
if (ob.is_ref()) {
throw chaiscript::detail::exception::bad_any_cast();
}
return std::move(*static_cast<Result *>(verify_type(ob, typeid(Result), ob.get_ptr())));
}
};

View File

@ -1970,3 +1970,79 @@ TEST_CASE("Exception from C++ [] operator is catchable in ChaiScript") {
caught
)") == true);
}
class Issue339_Object {
public:
Issue339_Object() = default;
Issue339_Object(const Issue339_Object &) { ++copy_count(); }
Issue339_Object(Issue339_Object &&) { ++move_count(); }
Issue339_Object &operator=(const Issue339_Object &) {
++copy_assign_count();
return *this;
}
Issue339_Object &operator=(Issue339_Object &&) {
++move_assign_count();
return *this;
}
static int &copy_count() {
static int c = 0;
return c;
}
static int &move_count() {
static int c = 0;
return c;
}
static int &copy_assign_count() {
static int c = 0;
return c;
}
static int &move_assign_count() {
static int c = 0;
return c;
}
static void reset_counts() {
copy_count() = 0;
move_count() = 0;
copy_assign_count() = 0;
move_assign_count() = 0;
}
};
TEST_CASE("Issue #339 - lvalue reference should not implicitly move") {
chaiscript::ChaiScript_Basic chai(create_chaiscript_stdlib(), create_chaiscript_parser());
auto m = std::make_shared<chaiscript::Module>();
m->add(chaiscript::user_type<Issue339_Object>(), "Obj");
m->add(chaiscript::constructor<Issue339_Object()>(), "Obj");
m->add(chaiscript::constructor<Issue339_Object(const Issue339_Object &)>(), "Obj");
m->add(chaiscript::constructor<Issue339_Object(Issue339_Object &&)>(), "Obj");
m->add(chaiscript::fun(static_cast<Issue339_Object &(Issue339_Object::*)(const Issue339_Object &)>(&Issue339_Object::operator=)), "=");
m->add(chaiscript::fun(static_cast<Issue339_Object &(Issue339_Object::*)(Issue339_Object &&)>(&Issue339_Object::operator=)), "=");
m->add(chaiscript::fun([]() -> Issue339_Object & {
static Issue339_Object persistent;
return persistent;
}),
"getObj");
m->add(chaiscript::fun([]() -> Issue339_Object { return Issue339_Object(); }), "makeObj");
chai.add(m);
SECTION("var from lvalue ref uses copy ctor, not move ctor on the source") {
Issue339_Object::reset_counts();
chai.eval("var o = getObj()");
CHECK(Issue339_Object::copy_count() >= 1);
}
SECTION("assignment from lvalue ref uses copy assign, not move assign") {
Issue339_Object::reset_counts();
chai.eval("var o2 = Obj(); o2 = getObj()");
CHECK(Issue339_Object::copy_assign_count() > 0);
CHECK(Issue339_Object::move_assign_count() == 0);
}
SECTION("return-by-value can still use move") {
Issue339_Object::reset_counts();
chai.eval("var o3 = makeObj()");
CHECK((Issue339_Object::copy_count() + Issue339_Object::move_count()) >= 1);
}
}