From f9711f6bca81fbf47c116e8e0e923419c7b7e001 Mon Sep 17 00:00:00 2001 From: leftibot Date: Sat, 11 Apr 2026 15:01:41 -0600 Subject: [PATCH] Address review: remove issue references from comments, add round-trip conversion tests Requested by @lefticus in PR #663 review. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../dispatchkit/proxy_functions.hpp | 2 +- unittests/compiled_tests.cpp | 45 ++++++++++++++----- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/include/chaiscript/dispatchkit/proxy_functions.hpp b/include/chaiscript/dispatchkit/proxy_functions.hpp index 9830f190..002e473b 100644 --- a/include/chaiscript/dispatchkit/proxy_functions.hpp +++ b/include/chaiscript/dispatchkit/proxy_functions.hpp @@ -801,7 +801,7 @@ namespace chaiscript { // Deprioritize functions whose first parameter (object/receiver) requires // type conversion: conversions create temporaries, so mutations on the - // converted object are silently lost (issue #405). + // converted object are silently lost. if (plist.size() > 1 && !func->get_param_types()[1].bare_equal(plist[0].get_type_info())) { numdiffs = plist.size(); } diff --git a/unittests/compiled_tests.cpp b/unittests/compiled_tests.cpp index 2bcbb587..5a3f09d1 100644 --- a/unittests/compiled_tests.cpp +++ b/unittests/compiled_tests.cpp @@ -1437,28 +1437,37 @@ TEST_CASE("Issue #421 - Switch with type_conversion does not compare destroyed o })") == 0); } -// Regression test for issue #405: push_back() on script-created vector has -// no effect when vector_conversion is in effect. The bug occurs because -// dispatch selects the C++ push_back for the converted type over the built-in -// one, operating on a temporary copy of the vector. -TEST_CASE("push_back on script vector with vector_conversion (#405)") { +// Regression test: push_back() on script-created vector has no effect when +// vector_conversion is in effect. The bug occurs because dispatch selects +// the C++ push_back for the converted type over the built-in one, operating +// on a temporary copy of the vector. +TEST_CASE("push_back on script vector with vector_conversion") { chaiscript::ChaiScript_Basic chai(create_chaiscript_stdlib(), create_chaiscript_parser()); - // Register both vector_type and vector_conversion for std::vector, - // which is the combination that triggers the bug auto m = std::make_shared(); chaiscript::bootstrap::standard_library::vector_type>("VectorString", *m); m->add(chaiscript::vector_conversion>()); chai.add(m); - // push_back on an empty script-created vector + // Register a C++ function that accepts the converted type, so we can + // verify that vector_conversion actually works for passing vectors + chai.add(chaiscript::fun([](const std::vector &v) -> std::string { + std::string result; + for (const auto &s : v) { + if (!result.empty()) { result += ","; } + result += s; + } + return result; + }), "join_strings"); + + // push_back on an empty script-created vector must be visible CHECK(chai.eval( "auto x = [];" "x.push_back(\"Hello\");" "x.size() == 1" )); - // push_back on a script-created vector with initial elements + // push_back on a vector with initial elements must grow correctly CHECK(chai.eval( "auto y = [\"a\", \"b\"];" "y.push_back(\"c\");" @@ -1466,12 +1475,28 @@ TEST_CASE("push_back on script vector with vector_conversion (#405)") { "y.size() == 4" )); - // Verify the actual content is preserved + // Verify the actual content is preserved after push_back CHECK(chai.eval( "auto z = [];" "z.push_back(\"World\");" "z[0]" ) == "World"); + + // Round-trip: build a vector in script, push_back elements, then pass it + // to a C++ function via vector_conversion and verify the contents + CHECK(chai.eval( + "auto v = [\"one\", \"two\"];" + "v.push_back(\"three\");" + "join_strings(v)" + ) == "one,two,three"); + + // Verify conversion works on a freshly created vector too + CHECK(chai.eval( + "auto w = [];" + "w.push_back(\"hello\");" + "w.push_back(\"world\");" + "join_strings(w)" + ) == "hello,world"); } // Regression test for issue #607: AST_Node_Trace must be a complete type