Skip to content

Commit 042f2e0

Browse files
lefticusclaudeClang Robot
authored
Error propagation handling (#16)
* Add failing tests for container overflow error propagation Tests verify that SmallVector container overflows (values, strings, object_scratch) return Error SExprs instead of silently corrupting data. Currently these tests crash/fail, proving the need for error propagation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Propagate SmallVector container overflow errors to callers Add has_container_error() and make_container_error() to detect and report when any SmallVector container enters error state. Check for overflow in evaluate(), sequence(), and parse() so errors bubble up as Error SExprs instead of causing silent data corruption. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add specific error messages for each container overflow type Distinguish between strings, values, scratch, and scope container overflows so callers can identify which resource was exhausted. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add error handling tests for propagation, arg counts, and evaluate_to Test error propagation through nested expressions, if branches, let bindings/body, cond results, begin, and eval. Test wrong arg counts for error?, quote, if, and cons. Test that evaluate_to surfaces errors via std::expected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Simplify parse_number from state machine to sequential phase processing Replace the 5-state switch-based parser with sequential digit consumption phases (sign, integer, fraction, exponent). Same functionality in roughly 35% less code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Extract to_quoted/from_quoted helpers to DRY up car, cons, and quote The nested get_if chains for converting between quoted representations (list_type↔literal_list_type, identifier↔symbol) were repeated in car, cons, and quote. Extract into two inverse helper functions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Replace hand-rolled error_p with make_type_predicate<error_type>() error_p was doing exactly what the generic make_type_predicate already does: check param count, eval, check type, return bool. Eliminate the dedicated function and reuse the existing template. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * 🎨 Committing clang-format changes --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Clang Robot <robot@example.com>
1 parent 351b990 commit 042f2e0

2 files changed

Lines changed: 221 additions & 146 deletions

File tree

include/cons_expr/cons_expr.hpp

Lines changed: 114 additions & 146 deletions
Original file line numberDiff line numberDiff line change
@@ -259,103 +259,64 @@ template<typename T, typename CharType>
259259
requires std::is_signed_v<T>
260260
[[nodiscard]] constexpr std::pair<bool, T> parse_number(std::basic_string_view<CharType> input) noexcept
261261
{
262+
using ch = chars<CharType>;
262263
static constexpr std::pair<bool, T> failure{ false, 0 };
263-
if (input == chars<CharType>::str("-")) { return failure; }
264-
265-
enum struct State : std::uint8_t {
266-
Start,
267-
IntegerPart,
268-
FractionPart,
269-
ExponentPart,
270-
ExponentStart,
271-
};
272264

273-
State state = State::Start;
274-
T value_sign = 1;
275-
long long value = 0LL;
276-
long long frac = 0LL;
277-
long long frac_digits = 0LL;
278-
long long exp_sign = 1LL;
279-
long long exp = 0LL;
265+
if (input.empty() || input == ch::str("-")) { return failure; }
266+
267+
auto it = input.begin();
268+
const auto end = input.end();
269+
270+
const T value_sign = (*it == ch::ch('-')) ? (++it, T{ -1 }) : T{ 1 };
280271

281272
constexpr auto pow_10 = [](std::integral auto power) noexcept {
282-
auto result = 1ll;
283-
for (int iteration = 0; iteration < power; ++iteration) { result *= 10ll; }
273+
auto result = 1LL;
274+
for (int i = 0; i < power; ++i) { result *= 10LL; }
284275
return result;
285276
};
286277

287-
const auto parse_digit = [](auto &cur_value, auto ch) {
288-
if (ch >= chars<CharType>::ch('0') && ch <= chars<CharType>::ch('9')) {
289-
cur_value = (cur_value * 10) + ch - chars<CharType>::ch('0');
290-
return true;
278+
const auto consume_digits = [&](auto &accum) {
279+
long long count = 0;
280+
while (it != end && *it >= ch::ch('0') && *it <= ch::ch('9')) {
281+
accum = accum * 10 + (*it - ch::ch('0'));
282+
++it;
283+
++count;
291284
}
292-
return false;
285+
return count;
293286
};
294287

295-
for (const auto ch : input) {
296-
switch (state) {
297-
case State::Start:
298-
state = State::IntegerPart;
299-
if (ch == chars<CharType>::ch('-')) {
300-
value_sign = -1;
301-
} else if (ch == chars<CharType>::ch('.')) {
302-
state = State::FractionPart;
303-
} else if (!parse_digit(value, ch)) {
304-
return failure;
305-
}
306-
break;
307-
case State::IntegerPart:
308-
if (ch == chars<CharType>::ch('.')) {
309-
state = State::FractionPart;
310-
} else if (ch == chars<CharType>::ch('e') || ch == chars<CharType>::ch('E')) {
311-
state = State::ExponentStart;
312-
} else if (!parse_digit(value, ch)) {
313-
return failure;
314-
}
315-
break;
316-
case State::FractionPart:
317-
if (parse_digit(frac, ch)) {
318-
++frac_digits;
319-
} else if (ch == chars<CharType>::ch('e') || ch == chars<CharType>::ch('E')) {
320-
state = State::ExponentStart;
321-
} else {
322-
return failure;
323-
}
324-
break;
325-
case State::ExponentStart:
326-
if (ch == chars<CharType>::ch('-')) {
327-
exp_sign = -1;
328-
} else if (!parse_digit(exp, ch)) {
329-
return failure;
330-
}
331-
state = State::ExponentPart;
332-
break;
333-
case State::ExponentPart:
334-
if (!parse_digit(exp, ch)) { return failure; }
335-
}
336-
}
288+
long long value = 0;
289+
const auto int_digits = consume_digits(value);
337290

338291
if constexpr (std::is_integral_v<T>) {
339-
if (state != State::IntegerPart) { return failure; }
340-
292+
if (it != end || int_digits == 0) { return failure; }
341293
return { true, value_sign * static_cast<T>(value) };
342294
} else {
343-
if (state == State::Start || state == State::ExponentStart) { return failure; }
295+
long long frac = 0, frac_digits = 0;
296+
if (it != end && *it == ch::ch('.')) {
297+
++it;
298+
frac_digits = consume_digits(frac);
299+
}
344300

345-
const auto integral_part = static_cast<T>(value);
346-
const auto floating_point_part = static_cast<T>(frac) / static_cast<T>(pow_10(frac_digits));
347-
const auto signed_shifted_number = (integral_part + floating_point_part) * value_sign;
348-
const auto shift = exp_sign * exp;
301+
if (int_digits == 0 && frac_digits == 0) { return failure; }
349302

350-
const auto number = [&]() {
351-
if (shift < 0) {
352-
return signed_shifted_number / static_cast<T>(pow_10(-shift));
353-
} else {
354-
return signed_shifted_number * static_cast<T>(pow_10(shift));
303+
long long exp = 0, exp_sign = 1;
304+
if (it != end && (*it == ch::ch('e') || *it == ch::ch('E'))) {
305+
++it;
306+
if (it != end && *it == ch::ch('-')) {
307+
exp_sign = -1;
308+
++it;
355309
}
356-
}();
310+
if (consume_digits(exp) == 0) { return failure; }
311+
}
312+
313+
if (it != end) { return failure; }
357314

358-
return { true, number };
315+
const auto number =
316+
(static_cast<T>(value) + static_cast<T>(frac) / static_cast<T>(pow_10(frac_digits))) * value_sign;
317+
const auto shift = exp_sign * exp;
318+
if (shift < 0) { return { true, number / static_cast<T>(pow_10(-shift)) }; }
319+
return { true, number * static_cast<T>(pow_10(shift)) };
359320
}
360321
}
361322

@@ -754,6 +715,7 @@ struct cons_expr
754715
int quote_depth = 0;
755716

756717
while (!token.parsed.empty()) {
718+
if (has_container_error()) { break; }
757719
bool entered_quote = false;
758720

759721
if (token.parsed == str("(")) {
@@ -793,6 +755,7 @@ struct cons_expr
793755

794756
token = next_token(token.remaining);
795757
}
758+
if (has_container_error()) { return { empty_indexed_list, token }; }
796759
return { values.insert_or_find(retval), token };
797760
}
798761

@@ -827,7 +790,7 @@ struct cons_expr
827790
add(str("quote"), SExpr{ FunctionPtr{ quoter, FunctionPtr::Type::other } });
828791
add(str("begin"), SExpr{ FunctionPtr{ begin, FunctionPtr::Type::other } });
829792
add(str("cond"), SExpr{ FunctionPtr{ cond, FunctionPtr::Type::other } });
830-
add(str("error?"), SExpr{ FunctionPtr{ error_p, FunctionPtr::Type::other } });
793+
add(str("error?"), SExpr{ FunctionPtr{ make_type_predicate<error_type>(), FunctionPtr::Type::other } });
831794

832795
// Type predicates using the generic make_type_predicate function
833796
// Simple atomic types
@@ -846,12 +809,42 @@ struct cons_expr
846809

847810
// Even atom? can use the generic predicate with Atom
848811
add(str("atom?"), SExpr{ FunctionPtr{ make_type_predicate<Atom>(), FunctionPtr::Type::other } });
812+
813+
// Pre-register error messages so make_container_error can find them without inserting
814+
strings.insert_or_find(str("strings container overflow"));
815+
strings.insert_or_find(str("values container overflow"));
816+
strings.insert_or_find(str("scratch container overflow"));
817+
strings.insert_or_find(str("scope container overflow"));
818+
}
819+
820+
[[nodiscard]] constexpr bool has_container_error() const noexcept
821+
{
822+
return strings.error_state || values.error_state || object_scratch.error_state || variables_scratch.error_state
823+
|| string_scratch.error_state || global_scope.error_state;
824+
}
825+
826+
[[nodiscard]] constexpr SExpr make_container_error() noexcept
827+
{
828+
if (strings.error_state) {
829+
return SExpr{ error_type{ strings.insert_or_find(str("strings container overflow")), empty_indexed_list } };
830+
}
831+
if (values.error_state) {
832+
return SExpr{ error_type{ strings.insert_or_find(str("values container overflow")), empty_indexed_list } };
833+
}
834+
if (object_scratch.error_state || variables_scratch.error_state || string_scratch.error_state) {
835+
return SExpr{ error_type{ strings.insert_or_find(str("scratch container overflow")), empty_indexed_list } };
836+
}
837+
return SExpr{ error_type{ strings.insert_or_find(str("scope container overflow")), empty_indexed_list } };
849838
}
850839

851840
[[nodiscard]] constexpr SExpr sequence(LexicalScope &scope, list_type expressions)
852841
{
853842
auto result = SExpr{ Atom{ std::monostate{} } };
854-
std::ranges::for_each(values[expressions], [&, engine = this](auto expr) { result = engine->eval(scope, expr); });
843+
for (const auto &expr : values[expressions]) {
844+
if (has_container_error()) { return make_container_error(); }
845+
result = eval(scope, expr);
846+
}
847+
if (has_container_error()) { return make_container_error(); }
855848
return result;
856849
}
857850

@@ -1288,23 +1281,7 @@ struct cons_expr
12881281
const auto &[front, list] = *evaled_params;
12891282

12901283
Scratch result{ engine.object_scratch };
1291-
1292-
if (const auto *list_front = std::get_if<literal_list_type>(&front.value); list_front != nullptr) {
1293-
// First element is a list, add it as a nested list
1294-
result.push_back(SExpr{ list_front->items });
1295-
} else if (const auto *atom = std::get_if<Atom>(&front.value); atom != nullptr) {
1296-
if (const auto *identifier_front = std::get_if<symbol_type>(atom); identifier_front != nullptr) {
1297-
// Convert symbol to identifier when adding to result list
1298-
// Note: should maybe fix this so quoted lists are always lists of symbols?
1299-
result.push_back(SExpr{ Atom{ to_identifier(*identifier_front) } });
1300-
} else {
1301-
// Regular atom, keep as-is
1302-
result.push_back(front);
1303-
}
1304-
} else {
1305-
// Any other expression type
1306-
result.push_back(front);
1307-
}
1284+
result.push_back(from_quoted(front));
13081285

13091286
// Add the remaining elements from the second list
13101287
for (const auto &value : engine.values[list.items]) { result.push_back(value); }
@@ -1322,6 +1299,32 @@ struct cons_expr
13221299
return obj.error();
13231300
}
13241301

1302+
// Convert an SExpr to its quoted representation (list_type→literal_list_type, identifier→symbol)
1303+
[[nodiscard]] static constexpr SExpr to_quoted(const SExpr &expr)
1304+
{
1305+
if (const auto *list = std::get_if<list_type>(&expr.value); list != nullptr) {
1306+
return SExpr{ literal_list_type{ *list } };
1307+
}
1308+
if (const auto *atom = std::get_if<Atom>(&expr.value); atom != nullptr) {
1309+
if (const auto *id = std::get_if<identifier_type>(atom); id != nullptr) {
1310+
return SExpr{ Atom{ symbol_type{ to_symbol(*id) } } };
1311+
}
1312+
}
1313+
return expr;
1314+
}
1315+
1316+
// Convert an SExpr from its quoted representation back to evaluable form
1317+
[[nodiscard]] static constexpr SExpr from_quoted(const SExpr &expr)
1318+
{
1319+
if (const auto *lit = std::get_if<literal_list_type>(&expr.value); lit != nullptr) { return SExpr{ lit->items }; }
1320+
if (const auto *atom = std::get_if<Atom>(&expr.value); atom != nullptr) {
1321+
if (const auto *sym = std::get_if<symbol_type>(atom); sym != nullptr) {
1322+
return SExpr{ Atom{ to_identifier(*sym) } };
1323+
}
1324+
}
1325+
return expr;
1326+
}
1327+
13251328
// (cdr '(1 2 3)) -> '(2 3)
13261329
// (cdr '(1)) -> '()
13271330
// (cdr '()) -> ERROR
@@ -1344,24 +1347,8 @@ struct cons_expr
13441347
{
13451348
return error_or_else(
13461349
engine.eval_to<literal_list_type>(scope, params, str("(car Non-Empty-LiteralList)")), [&](const auto &list) {
1347-
// Check if list is empty
13481350
if (list.items.size == 0) { return engine.make_error(str("car: cannot take car of empty list"), params); }
1349-
1350-
// Get the first element of the list
1351-
const auto &elem = engine.values[list.items.front()];
1352-
1353-
// If the element is a list_type, return it as a literal_list_type
1354-
if (const auto *nested_list = std::get_if<list_type>(&elem.value); nested_list != nullptr) {
1355-
return SExpr{ literal_list_type{ *nested_list } };
1356-
}
1357-
1358-
if (const auto *atom = std::get_if<Atom>(&elem.value); atom != nullptr) {
1359-
if (const auto *identifier = std::get_if<identifier_type>(atom); identifier != nullptr) {
1360-
return SExpr{ Atom{ symbol_type{ to_symbol(*identifier) } } };
1361-
}
1362-
}
1363-
1364-
return elem;
1351+
return to_quoted(engine.values[list.items.front()]);
13651352
});
13661353
}
13671354

@@ -1450,20 +1437,6 @@ struct cons_expr
14501437
return SExpr{ Atom{ std::monostate{} } };
14511438
}
14521439

1453-
// error?: Check if the expression is an error
1454-
[[nodiscard]] static constexpr SExpr error_p(cons_expr &engine, LexicalScope &scope, list_type params)
1455-
{
1456-
if (params.size != 1) { return engine.make_error(str("(error? expr)"), params); }
1457-
1458-
// Evaluate the expression
1459-
auto expr = engine.eval(scope, engine.values[params[0]]);
1460-
1461-
// Check if it's an error type
1462-
const bool is_error = std::holds_alternative<error_type>(expr.value);
1463-
1464-
return SExpr{ Atom(is_error) };
1465-
}
1466-
14671440
// Generic type predicate template for any type(s)
14681441
template<typename... Types> [[nodiscard]] static constexpr function_ptr make_type_predicate()
14691442
{
@@ -1483,24 +1456,12 @@ struct cons_expr
14831456
[[nodiscard]] static constexpr SExpr quote(cons_expr &engine, list_type params)
14841457
{
14851458
if (params.size != 1) { return engine.make_error(str("(quote expr)"), params); }
1486-
14871459
const auto &expr = engine.values[params[0]];
1488-
1489-
// If it's a list, convert it to a literal list
1490-
if (const auto *list = std::get_if<list_type>(&expr.value); list != nullptr) {
1491-
// Special case for empty lists - use a canonical empty list with start index 0
1492-
if (list->size == 0) { return SExpr{ literal_list_type{ empty_indexed_list } }; }
1493-
return SExpr{ literal_list_type{ *list } };
1494-
}
1495-
// If it's an identifier, convert it to a symbol
1496-
else if (const auto *atom = std::get_if<Atom>(&expr.value); atom != nullptr) {
1497-
if (const auto *id = std::get_if<identifier_type>(atom); id != nullptr) {
1498-
return SExpr{ Atom{ symbol_type{ to_symbol(*id) } } };
1499-
}
1460+
// Special case: empty lists use canonical empty_indexed_list
1461+
if (const auto *list = std::get_if<list_type>(&expr.value); list != nullptr && list->size == 0) {
1462+
return SExpr{ literal_list_type{ empty_indexed_list } };
15001463
}
1501-
1502-
// Otherwise return as is
1503-
return expr;
1464+
return to_quoted(expr);
15041465
}
15051466

15061467
[[nodiscard]] static constexpr SExpr quoter(cons_expr &engine, LexicalScope &, list_type params)
@@ -1649,7 +1610,14 @@ struct cons_expr
16491610
return engine.make_error(str("supported types"), params);
16501611
}
16511612

1652-
[[nodiscard]] constexpr SExpr evaluate(string_view_type input) { return sequence(global_scope, parse(input).first); }
1613+
[[nodiscard]] constexpr SExpr evaluate(string_view_type input)
1614+
{
1615+
auto [parsed, remaining] = parse(input);
1616+
if (has_container_error()) { return make_container_error(); }
1617+
auto result = sequence(global_scope, parsed);
1618+
if (has_container_error()) { return make_container_error(); }
1619+
return result;
1620+
}
16531621

16541622
template<typename Result> [[nodiscard]] constexpr std::expected<Result, SExpr> evaluate_to(string_view_type input)
16551623
{

0 commit comments

Comments
 (0)