Nojson #39
Merged
Nojson #39
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- consequently, strings need to be stored at a safe place, so they can be refered to thorugh std::string_views - TODO: Currently, this implementation uses a global data store for that. This is neither thread safe nor long-term usable.
* remove to_expr(json::string) to avoid accidental string creation. * created context for string lifetime management TODO: update API of any_expr
- moved evaluation to use variants (value_variant, aka any_value). - updated interfaces: e.g., variable_accessor, apply, etc. - BROKE: invalidated arrays, because they currently conflict with between any_expr and any_variant. - TODO: introduce arrays of variants as second array type - Memory management for value arrays - BROKE: examples and tests
Evaluation generates any_values (aka value_variant). In this version arrays are currently allocated and never deallocated. MEMORY LEAKS.
added valeu_variant constructors and destructors to properly copy array_values.
this change seems necessary to allow for internally managing the lifetime of strings. * enable string management in logic_rule: currently all strings are stored in logic_data until the logic_rule object is destructed. This may lead to string accumulation over time, if a rule generates many strings. e.g., through string concatenation. * remove ast-core. It is no longer needed as the user-side no longer accesses the any_expr directly. * move essential types for logic_data into ast-full.hpp
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.
- removed intermediate abstract class that was not needed
- resolve conflicting updates
- fix merge conflicts with feature branch
Enable optimized membership tests for static arrays - Introduced ENABLE_OPTIMIZATIONS macro to control experimental optimizations. - Added opt_membership_array AST node for efficient membership checks using unordered_set. - Implemented std::hash specializations for value_variant, array_value, and value_variant_base. - Updated membership operator logic to use opt_membership_array when possible. - Refactored operator construction to support optimized membership tests. - Modified evaluator to handle opt_membership_array. - Updated tests to use std::views::transform instead of Boost adaptors. - Minor cleanups and removed obsolete code/comments.
Collaborator
|
Before we can merge this we have to fix integration with clippy-cpp then metalldata repos. I created a "jl_nojson" branch in clippy-cpp to test and it's not building. Pls start there to see what other modifications to user code are needed. |
Collaborator
Author
|
I have updated the jl_nojson branch in clippy-cpp. GIT_TAG in that branch uses nojson, and that needs to be changed after the this branch merges into master. @rogerpearce in what MetallData branch should nojson be integrated? |
Collaborator
|
I just made a branch for you in the MetallData repo: sbromberger/feature/metall_graph/clippy_nojson You'll need update its top-level cmake to pull in the clippy-cpp nojson branch. And yes, as we zip this all together we'll have to add new release vs. |
Collaborator
|
@ppete just get that to build and I'll take it from there. |
Collaborator
|
Looks good @ppete, will start merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.