From dd9afc832e86a99cba13fff886470d64ce700659 Mon Sep 17 00:00:00 2001 From: leftibot Date: Mon, 13 Apr 2026 15:56:38 -0600 Subject: [PATCH] Address review: use :: instead of . as nested namespace separator Switch from dotted names (e.g. "constants.si") to C++-style :: separator (e.g. "constants::si") for nested namespace declarations, both in the C++ API (register_namespace) and in script (namespace()). The original implementation used . because namespace members are accessed via dot notation at runtime (constants.si.mu_B), making the declaration separator match the access syntax. However, :: is more consistent with C++ namespace conventions and aligns with ChaiScript's existing use of :: for method (def Class::method) and attribute (attr Class::attr) declarations. Member access in scripts remains dot-based (constants.si.mu_B) since that is ChaiScript's member access operator. Requested by @lefticus in PR #675 review. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../chaiscript/language/chaiscript_engine.hpp | 16 ++++++++-------- unittests/compiled_tests.cpp | 8 ++++---- unittests/nested_namespaces.chai | 10 +++++----- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/chaiscript/language/chaiscript_engine.hpp b/include/chaiscript/language/chaiscript_engine.hpp index d13906f1..0b823ff1 100644 --- a/include/chaiscript/language/chaiscript_engine.hpp +++ b/include/chaiscript/language/chaiscript_engine.hpp @@ -190,8 +190,8 @@ namespace chaiscript { m_engine.add(fun([this](const std::string &t_namespace_name) { register_namespace([](Namespace & /*space*/) noexcept {}, t_namespace_name); - const auto dot_pos = t_namespace_name.find('.'); - const std::string root_name = (dot_pos != std::string::npos) ? t_namespace_name.substr(0, dot_pos) : t_namespace_name; + const auto sep_pos = t_namespace_name.find("::"); + const std::string root_name = (sep_pos != std::string::npos) ? t_namespace_name.substr(0, sep_pos) : t_namespace_name; if (!m_engine.get_scripting_objects().count(root_name)) { import(root_name); } else if (m_namespace_generators.count(root_name)) { @@ -744,9 +744,9 @@ namespace chaiscript { } /// \brief Registers a namespace generator, which delays generation of the namespace until it is imported, saving memory if it is never - /// used. Supports dotted names (e.g. "constants.si") for nested namespaces; parent namespaces are auto-registered if absent. + /// used. Supports C++-style nested names (e.g. "constants::si") for nested namespaces; parent namespaces are auto-registered if absent. /// \param[in] t_namespace_generator Namespace generator function. - /// \param[in] t_namespace_name Name of the Namespace function being registered (may contain dots for nesting). + /// \param[in] t_namespace_name Name of the Namespace function being registered (may contain :: for nesting). /// \throw std::runtime_error In the case that the namespace name was already registered. void register_namespace(const std::function &t_namespace_generator, const std::string &t_namespace_name) { chaiscript::detail::threading::unique_lock l(m_use_mutex); @@ -760,7 +760,7 @@ namespace chaiscript { return space; })); - auto pos = t_namespace_name.rfind('.'); + auto pos = t_namespace_name.rfind("::"); while (pos != std::string::npos) { const std::string parent = t_namespace_name.substr(0, pos); if (!m_namespace_generators.count(parent)) { @@ -768,17 +768,17 @@ namespace chaiscript { return space; })); } - pos = parent.rfind('.'); + pos = parent.rfind("::"); } } private: void nest_children(const std::string &t_parent_name, Namespace &t_parent) { - const std::string prefix = t_parent_name + "."; + const std::string prefix = t_parent_name + "::"; for (auto &[name, generator] : m_namespace_generators) { if (name.size() > prefix.size() && name.compare(0, prefix.size(), prefix) == 0) { const std::string remainder = name.substr(prefix.size()); - if (remainder.find('.') == std::string::npos) { + if (remainder.find("::") == std::string::npos) { auto &child_ns = generator(); nest_children(name, child_ns); t_parent[remainder] = var(std::ref(child_ns)); diff --git a/unittests/compiled_tests.cpp b/unittests/compiled_tests.cpp index 2f1eda3f..d662fd59 100644 --- a/unittests/compiled_tests.cpp +++ b/unittests/compiled_tests.cpp @@ -1783,20 +1783,20 @@ TEST_CASE("eval_error with AST_Node_Trace call stack compiles in C++20") { } } -TEST_CASE("Nested namespaces via register_namespace with dotted names") { +TEST_CASE("Nested namespaces via register_namespace with :: separator") { chaiscript::ChaiScript_Basic chai(create_chaiscript_stdlib(), create_chaiscript_parser()); chai.register_namespace( [](chaiscript::Namespace &si) { si["mu_B"] = chaiscript::const_var(9.274); }, - "constants.si"); + "constants::si"); chai.register_namespace( [](chaiscript::Namespace &mm) { mm["mu_B"] = chaiscript::const_var(0.05788); }, - "constants.mm"); + "constants::mm"); chai.import("constants"); @@ -1811,7 +1811,7 @@ TEST_CASE("Deeply nested namespaces via register_namespace") { [](chaiscript::Namespace &leaf) { leaf["val"] = chaiscript::const_var(42); }, - "a.b.c"); + "a::b::c"); chai.import("a"); diff --git a/unittests/nested_namespaces.chai b/unittests/nested_namespaces.chai index 8550e689..9d498fc4 100644 --- a/unittests/nested_namespaces.chai +++ b/unittests/nested_namespaces.chai @@ -1,15 +1,15 @@ -// Test nested namespaces using dotted names -namespace("constants.si") +// Test nested namespaces using C++-style :: separator +namespace("constants::si") constants.si.mu_B = 1.0 -namespace("constants.mm") +namespace("constants::mm") constants.mm.mu_B = 2.0 assert_equal(1.0, constants.si.mu_B) assert_equal(2.0, constants.mm.mu_B) // Test deeper nesting -namespace("a.b.c") +namespace("a::b::c") a.b.c.val = 42 assert_equal(42, a.b.c.val) @@ -18,7 +18,7 @@ assert_equal(42, a.b.c.val) namespace("math") math.square = fun(x) { x * x } -namespace("math.trig") +namespace("math::trig") math.trig.double_angle = fun(x) { 2.0 * x } assert_equal(16, math.square(4))