From c08447d9c77259ff0f300b33bb1c4c97cf4fb111 Mon Sep 17 00:00:00 2001 From: leftibot Date: Tue, 14 Apr 2026 15:29:30 -0600 Subject: [PATCH] Address review: enum class syntax, constructor, configurable underlying type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Change syntax from `enum` to `enum class` (only strongly-typed enums) - Support optional underlying type: `enum class Flags : char { ... }` (defaults to `int` when omitted) - Replace `from_int` with a constructor named after the enum type, accessed as `Color::Color(1)` — the underlying type is no longer hardcoded to int - Use Boxed_Number for type-generic value storage and comparison - to_underlying now returns the actual underlying type Note: `Color(1)` syntax is not possible because ChaiScript's global objects shadow functions with the same name; `Color::Color(1)` is the C++-consistent alternative. Requested by @lefticus in PR #679 review. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../chaiscript/language/chaiscript_eval.hpp | 39 +++++++++++-------- .../chaiscript/language/chaiscript_parser.hpp | 26 +++++++++++-- unittests/enum.chai | 31 +++++++++------ 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/include/chaiscript/language/chaiscript_eval.hpp b/include/chaiscript/language/chaiscript_eval.hpp index 466368d0..970cdd3a 100644 --- a/include/chaiscript/language/chaiscript_eval.hpp +++ b/include/chaiscript/language/chaiscript_eval.hpp @@ -899,32 +899,37 @@ namespace chaiscript { Boxed_Value eval_internal(const chaiscript::detail::Dispatch_State &t_ss) const override { const auto &enum_name = this->children[0]->text; + const auto &underlying_type_name = this->children[1]->text; + const auto underlying_ti = t_ss->get_type(underlying_type_name); dispatch::Dynamic_Object container(enum_name); - std::vector valid_values; + std::vector valid_values; - for (size_t i = 1; i < this->children.size(); i += 2) { + for (size_t i = 2; i < this->children.size(); i += 2) { const auto &val_name = this->children[i]->text; - const int val_int = Boxed_Number(this->children[i + 1]->eval(t_ss)).get_as(); - valid_values.push_back(val_int); + const auto val_bv = Boxed_Number(this->children[i + 1]->eval(t_ss)).get_as(underlying_ti).bv; + valid_values.push_back(val_bv); dispatch::Dynamic_Object dobj(enum_name); - dobj.get_attr("value") = Boxed_Value(val_int); + dobj.get_attr("value") = val_bv; dobj.set_explicit(true); container[val_name] = const_var(dobj); } - auto shared_valid = std::make_shared>(std::move(valid_values)); + auto shared_valid = std::make_shared>(std::move(valid_values)); - container["from_int"] = var( - fun([shared_valid, enum_name](int t_val) -> Boxed_Value { - if (std::find(shared_valid->begin(), shared_valid->end(), t_val) == shared_valid->end()) { - throw exception::eval_error("Value " + std::to_string(t_val) + " is not valid for enum '" + enum_name + "'"); + container[enum_name] = var( + fun([shared_valid, enum_name, underlying_ti](const Boxed_Number &t_val) -> Boxed_Value { + const auto converted = t_val.get_as(underlying_ti); + for (const auto &v : *shared_valid) { + if (Boxed_Number::equals(Boxed_Number(v), converted)) { + dispatch::Dynamic_Object dobj(enum_name); + dobj.get_attr("value") = converted.bv; + dobj.set_explicit(true); + return const_var(dobj); + } } - dispatch::Dynamic_Object dobj(enum_name); - dobj.get_attr("value") = Boxed_Value(t_val); - dobj.set_explicit(true); - return const_var(dobj); + throw exception::eval_error("Value is not valid for enum '" + enum_name + "'"); })); t_ss->add_global_const(const_var(container), enum_name); @@ -933,7 +938,7 @@ namespace chaiscript { std::make_shared( enum_name, fun([](const dispatch::Dynamic_Object &lhs, const dispatch::Dynamic_Object &rhs) { - return Boxed_Number(lhs.get_attr("value")).get_as() == Boxed_Number(rhs.get_attr("value")).get_as(); + return Boxed_Number::equals(Boxed_Number(lhs.get_attr("value")), Boxed_Number(rhs.get_attr("value"))); })), "=="); @@ -941,14 +946,14 @@ namespace chaiscript { std::make_shared( enum_name, fun([](const dispatch::Dynamic_Object &lhs, const dispatch::Dynamic_Object &rhs) { - return Boxed_Number(lhs.get_attr("value")).get_as() != Boxed_Number(rhs.get_attr("value")).get_as(); + return !Boxed_Number::equals(Boxed_Number(lhs.get_attr("value")), Boxed_Number(rhs.get_attr("value"))); })), "!="); t_ss->add( std::make_shared( enum_name, - fun([](const dispatch::Dynamic_Object &obj) { return Boxed_Number(obj.get_attr("value")).get_as(); })), + fun([](const dispatch::Dynamic_Object &obj) { return obj.get_attr("value"); })), "to_underlying"); return void_var(); diff --git a/include/chaiscript/language/chaiscript_parser.hpp b/include/chaiscript/language/chaiscript_parser.hpp index d884788d..f110bcd5 100644 --- a/include/chaiscript/language/chaiscript_parser.hpp +++ b/include/chaiscript/language/chaiscript_parser.hpp @@ -2076,6 +2076,12 @@ namespace chaiscript { const auto prev_stack_top = m_match_stack.size(); if (Keyword("enum")) { + if (!Keyword("class")) { + throw exception::eval_error("Expected 'class' after 'enum' (only 'enum class' is supported)", + File_Position(m_position.line, m_position.col), + *m_filename); + } + if (!t_allowed) { throw exception::eval_error("Enum definitions only allowed at top scope", File_Position(m_position.line, m_position.col), @@ -2085,11 +2091,25 @@ namespace chaiscript { retval = true; if (!Id(true)) { - throw exception::eval_error("Missing enum name in definition", File_Position(m_position.line, m_position.col), *m_filename); + throw exception::eval_error("Missing enum class name in definition", File_Position(m_position.line, m_position.col), *m_filename); } + std::string underlying_type = "int"; + if (Char(':')) { + if (!Id(false)) { + throw exception::eval_error("Expected underlying type after ':'", + File_Position(m_position.line, m_position.col), + *m_filename); + } + underlying_type = m_match_stack.back()->text; + m_match_stack.pop_back(); + } + + m_match_stack.push_back( + make_node>(underlying_type, m_position.line, m_position.col, const_var(underlying_type))); + if (!Char('{')) { - throw exception::eval_error("Expected '{' after enum name", File_Position(m_position.line, m_position.col), *m_filename); + throw exception::eval_error("Expected '{' after enum class declaration", File_Position(m_position.line, m_position.col), *m_filename); } int next_value = 0; @@ -2128,7 +2148,7 @@ namespace chaiscript { } if (!Char('}')) { - throw exception::eval_error("Expected '}' to close enum definition", + throw exception::eval_error("Expected '}' to close enum class definition", File_Position(m_position.line, m_position.col), *m_filename); } diff --git a/unittests/enum.chai b/unittests/enum.chai index 3778dd56..7f85de73 100644 --- a/unittests/enum.chai +++ b/unittests/enum.chai @@ -1,5 +1,5 @@ -// Basic enum definition -enum Color { Red, Green, Blue } +// Basic enum class definition (default underlying type: int) +enum class Color { Red, Green, Blue } // Access via :: syntax auto r = Color::Red @@ -12,13 +12,13 @@ assert_false(Color::Red == Color::Green) assert_true(Color::Red != Color::Green) assert_false(Color::Red != Color::Red) -// Construction from valid int -auto c = Color::from_int(1) +// Constructor from valid underlying value +auto c = Color::Color(1) assert_true(c == Color::Green) -// Construction from invalid int throws +// Constructor from invalid value throws try { - Color::from_int(52) + Color::Color(52) assert_true(false) } catch(e) { // expected @@ -28,7 +28,7 @@ try { def takes_color(Color val) { val } takes_color(Color::Red) takes_color(Color::Green) -takes_color(Color::from_int(2)) +takes_color(Color::Color(2)) // Cannot pass int where Color is expected try { @@ -43,17 +43,17 @@ assert_equal(0, Color::Red.to_underlying()) assert_equal(1, Color::Green.to_underlying()) assert_equal(2, Color::Blue.to_underlying()) -// Enum with explicit values -enum Priority { Low = 10, Medium = 20, High = 30 } +// Enum class with explicit values +enum class Priority { Low = 10, Medium = 20, High = 30 } assert_equal(10, Priority::Low.to_underlying()) assert_equal(20, Priority::Medium.to_underlying()) assert_equal(30, Priority::High.to_underlying()) -auto p = Priority::from_int(20) +auto p = Priority::Priority(20) assert_true(p == Priority::Medium) // Mixed auto and explicit values -enum Status { Pending, Active = 5, Done } +enum class Status { Pending, Active = 5, Done } assert_equal(0, Status::Pending.to_underlying()) assert_equal(5, Status::Active.to_underlying()) assert_equal(6, Status::Done.to_underlying()) @@ -93,3 +93,12 @@ switch(Priority::High) { } } assert_equal("high", prio_result) + +// Enum class with explicit underlying type +enum class Flags : char { Read = 1, Write = 2, Execute = 4 } +assert_equal(1, Flags::Read.to_underlying()) +assert_equal(2, Flags::Write.to_underlying()) +assert_equal(4, Flags::Execute.to_underlying()) + +auto f = Flags::Flags(2) +assert_true(f == Flags::Write)