Skip to content

Commit 018e6cd

Browse files
committed
(arrays) prepare for array optimizations
Refactor array handling for value_variant, introduce array_value_base, and improve memory management** This commit introduces significant refactoring to the handling of array types in the jsonlogic C++ implementation, with the following key changes: - **Introduced `array_value_base`:** - Added an abstract base class `array_value_base` to unify array handling and allow for future extensions such as array views. - All array-related logic in `value_variant` and associated functions now use `array_value_base const*` instead of `array_value const*`. - **Refactored `array_value`:** - Now derives from `array_value_base`. - Internally stores elements in a `std::shared_ptr` for safer memory management and copy semantics. - Implements `value()` and `copy()` methods for interface consistency. - **Updated `value_variant` and memory management:** - All code paths that previously used `array_value const*` now use `array_value_base const*`. - Copy/move constructors and assignment operators for `value_variant` now correctly handle deep copying and deletion of array objects. - Helper functions `delete_array`, `copy_array`, and `invalidate_array` manage resource ownership and avoid memory leaks. - **Visitor pattern and operator handling:** - Visitors now support both `array_value_base` and `array_value`. - Logic for array operations (e.g., merge, map, filter, membership, etc.) updated to use new array types and methods. - Improved type coercion and equality/relational operations for arrays. - **Logging and evaluation:** - Replaced direct use of `std::ostream` for logging with a `variant_logger` function type for more flexible logging. - Updated evaluator and sequence functions to use the new logger type. - **Test infrastructure:** - Updated test runner (`testeval.cpp`) to convert results to string representations for comparison. - Added a new test for the `merge` operator. - Improved safety and clarity of string table management. - Marked some classes as non-copyable for safety. - Added comments and clarified TODOs for future enhancements (e.g., array views). - Improved assertion and error handling in several places. This refactor lays the groundwork for future enhancements (such as array views), improves memory safety and clarity of ownership, and unifies array handling across the codebase. The changes also make it easier to extend array support and maintain high performance. --- **Note:** This commit introduces breaking changes to the API for array handling. All user code and tests have been updated accordingly.
1 parent e1927c6 commit 018e6cd

File tree

5 files changed

+324
-245
lines changed

5 files changed

+324
-245
lines changed

cpp/include/jsonlogic/details/ast-full.hpp

Lines changed: 95 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <set>
66
#include <vector>
77
#include <iosfwd>
8+
#include <iostream>
89

910
#include "cxx-compat.hpp"
1011

@@ -18,21 +19,18 @@ struct visitor;
1819

1920
// the root class
2021
struct expr {
21-
expr() = default;
22-
virtual ~expr() = default;
22+
virtual ~expr() = default;
23+
expr() = default;
24+
expr(expr &&) = default;
25+
expr(const expr &) = default;
26+
expr &operator=(expr &&) = default;
27+
expr &operator=(const expr &) = default;
2328

2429
virtual void accept(visitor &) const = 0;
25-
26-
private:
27-
expr(expr &&) = delete;
28-
expr(const expr &) = delete;
29-
expr &operator=(expr &&) = delete;
30-
expr &operator=(const expr &) = delete;
3130
};
3231

3332
using any_expr = std::unique_ptr<expr>;
3433

35-
3634
struct oper : expr, private std::vector<any_expr> {
3735
using container_type = std::vector<any_expr>;
3836

@@ -51,6 +49,8 @@ struct oper : expr, private std::vector<any_expr> {
5149
using container_type::reverse_iterator;
5250
using container_type::size;
5351

52+
oper() = default;
53+
5454
// convenience function so that the constructor does not need to be
5555
// implemented in every derived class.
5656
void set_operands(container_type &&opers) { this->swap(opers); }
@@ -62,6 +62,12 @@ struct oper : expr, private std::vector<any_expr> {
6262
expr &operand(int n) const;
6363

6464
virtual int num_evaluated_operands() const;
65+
66+
private:
67+
oper(oper &&) = delete;
68+
oper(const oper &) = delete;
69+
oper &operator=(oper &&) = delete;
70+
oper &operator=(const oper &) = delete;
6571
};
6672

6773
// defines operators that have an upper bound on how many
@@ -326,41 +332,75 @@ struct string_value : value_generic<std::string_view> {
326332
void accept(visitor &) const final;
327333
};
328334

329-
struct array_value : value_base
335+
// \todo most likely not needed, so this can be removed
336+
struct array_value_base : value_base
330337
{
331-
array_value() = default;
332-
~array_value() = default;
338+
using container_type = std::vector<value_variant>;
333339

334-
array_value(array_value&&) = default;
335-
array_value& operator=(array_value&&) = default;
340+
~array_value_base() = default;
341+
array_value_base() = default;
342+
array_value_base(array_value_base&&) = default;
343+
array_value_base(const array_value_base&) = default;
344+
array_value_base& operator=(array_value_base&&) = default;
345+
array_value_base& operator=(const array_value_base&) = default;
346+
347+
virtual container_type const& value() const = 0;
348+
virtual const array_value_base* copy() const = 0;
349+
350+
value_variant to_variant() const final { return this; }
351+
};
352+
353+
struct array_value : array_value_base
354+
{
355+
using container_type = array_value_base::container_type;
356+
357+
~array_value() = default;
358+
array_value(array_value&&) = default;
359+
array_value& operator=(array_value&&) = default;
360+
array_value(const array_value&) = default;
361+
array_value& operator=(const array_value&) = default;
336362

337-
explicit
338-
array_value(std::vector<value_variant>&& elems)
339-
: val(std::move(elems))
340-
{}
341363

342364
explicit
343-
array_value(const std::vector<value_variant>& elems)
344-
: val(elems)
365+
array_value(container_type elems)
366+
: vec(std::make_shared<container_type>(std::move(elems)))
345367
{}
346368

347-
value_variant to_variant() const { return this; } // \todo maybe this should copy or throw
369+
container_type const& value() const final;
370+
const array_value* copy() const final;
371+
void accept(visitor &) const final;
348372

349-
std::vector<value_variant> const& elements() const { return val; }
350-
std::vector<value_variant>& elements() { return val; }
351-
std::vector<value_variant> const& value() const { return val; }
373+
private:
374+
const std::shared_ptr<container_type> vec;
375+
376+
array_value() = delete;
377+
};
352378

353-
std::size_t size() const { return val.size(); }
379+
#if 0
380+
// an internal class that
381+
struct array_view : array_value_base
382+
{
383+
using container_type = array_value_base::container_type;
384+
385+
~array_view() { std::cerr << "view" << std::endl; }
386+
array_view(array_view&&) = default;
387+
array_view(const array_view&) = default;
388+
array_view& operator=(array_view&&) = default;
389+
array_view& operator=(const array_view&) = default;
390+
391+
array_view(const array_value& arrval)
392+
: arr(arrval)
393+
{}
354394

395+
container_type const& value() const final;
396+
const array_view* copy() const final;
355397
void accept(visitor &) const final;
356398

357399
private:
358-
std::vector<value_variant> val;
359-
360-
array_value(const array_value&) = delete;
361-
array_value& operator=(const array_value&) = delete;
400+
const array_value& arr;
362401
};
363402

403+
#endif
364404

365405
// object types do not seem to have strong support by jsonlogic
366406
using object_value_data = std::map<std::string_view, any_expr>;
@@ -455,7 +495,9 @@ struct visitor {
455495
virtual void visit(const unsigned_int_value &) = 0;
456496
virtual void visit(const real_value &) = 0;
457497
virtual void visit(const string_value &) = 0;
498+
virtual void visit(const array_value_base &) = 0;
458499
virtual void visit(const array_value &) = 0;
500+
//~ virtual void visit(const array_view &) = 0;
459501
virtual void visit(const object_value &) = 0;
460502

461503
virtual void visit(const error &) = 0;
@@ -540,6 +582,8 @@ struct generic_dispatcher : visitor {
540582
void visit(const real_value &n) final { res = apply(n, &n); }
541583
void visit(const string_value &n) final { res = apply(n, &n); }
542584
void visit(const array_value &n) final { res = apply(n, &n); }
585+
//~ void visit(const array_view &n) final { res = apply(n, &n); }
586+
void visit(const array_value_base &n) final { res = apply(n, &n); }
543587
void visit(const object_value &n) final { res = apply(n, &n); }
544588

545589
void visit(const error &n) final { res = apply(n, &n); }
@@ -593,19 +637,30 @@ using string_table_base = std::set<std::string>;
593637
/// string table ensures lifetime of string exceeds lifetime of string_views
594638
struct string_table : private string_table_base
595639
{
596-
using string_table_base::size;
640+
using string_table_base::size;
597641

598-
std::string_view safe_string(std::string v)
599-
{
600-
auto res = this->emplace(std::move(v));
601-
return *res.first;
602-
}
642+
string_table() = default;
643+
~string_table() = default;
644+
string_table(string_table&&) = default;
645+
string_table& operator=(string_table&&) = default;
603646

604-
template <class iterator>
605-
std::string_view safe_string(iterator aa, iterator zz)
606-
{
607-
return safe_string(std::string(aa, zz));
608-
}
647+
std::string_view safe_string(std::string v)
648+
{
649+
auto emplaced = this->emplace(std::move(v));
650+
651+
std::string_view res = *emplaced.first;
652+
return res;
653+
}
654+
655+
template <class iterator>
656+
std::string_view safe_string(iterator aa, iterator zz)
657+
{
658+
return safe_string(std::string(aa, zz));
659+
}
660+
661+
private:
662+
string_table(const string_table&) = delete;
663+
string_table& operator=(const string_table&) = delete;
609664
};
610665

611666

cpp/include/jsonlogic/logic.hpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,21 @@ struct variable_resolution_error : std::runtime_error {
2626
//
2727
// API to evaluate/apply an expression
2828

29-
struct array_value;
29+
struct array_value_base;
3030

3131
/// a type representing views on value types in jsonlogic
3232
/// \details
3333
/// (1) the variant contains options for all primitive types + strings and arrays.
3434
/// (2) some rules treat the absence of a value differently from a null value
35-
/// \todo describe lifetime requirements
36-
using value_variant_base = std::variant< std::monostate,
37-
std::nullptr_t,
35+
/// \todo document lifetime requirements
36+
using value_variant_base = std::variant< std::monostate, // not available
37+
std::nullptr_t, // value is null
3838
bool,
3939
std::int64_t,
4040
std::uint64_t,
4141
double,
4242
std::string_view,
43-
array_value const*
44-
// boost::json::value // fallback type; may not be needed..
43+
array_value_base const*
4544
>;
4645

4746
struct value_variant : value_variant_base {

0 commit comments

Comments
 (0)