From 21b222429501e19281113c4c436a98d383926d16 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 21 Nov 2024 16:49:45 -0400 Subject: [PATCH] Track evaluation depth as a recursive argument (#201) Signed-off-by: Juan Cruz Viotti --- src/evaluator/context.cc | 26 +----- src/evaluator/dispatch.h | 79 +++++++++++-------- src/evaluator/evaluator.cc | 18 ++++- .../sourcemeta/blaze/evaluator_context.h | 2 - 4 files changed, 63 insertions(+), 62 deletions(-) diff --git a/src/evaluator/context.cc b/src/evaluator/context.cc index a64dd855..d6d7ea72 100644 --- a/src/evaluator/context.cc +++ b/src/evaluator/context.cc @@ -1,38 +1,18 @@ #include -#include #include // assert namespace sourcemeta::blaze { +// TODO: Completely inline push/pop + auto EvaluationContext::push( const sourcemeta::jsontoolkit::Pointer &relative_schema_location, const sourcemeta::jsontoolkit::Pointer &relative_instance_location, const bool track) -> void { - // Guard against infinite recursion in a cheap manner, as - // infinite recursion will manifest itself through huge - // ever-growing evaluate paths - constexpr auto EVALUATE_PATH_LIMIT{300}; - if (track) { - if (this->evaluate_path.size() > EVALUATE_PATH_LIMIT) [[unlikely]] { - throw sourcemeta::blaze::EvaluationError( - "The evaluation path depth limit was reached " - "likely due to infinite recursion"); - } - this->evaluate_path.push_back(relative_schema_location); this->instance_location.push_back(relative_instance_location); - } else { - if (this->evaluate_path_size > EVALUATE_PATH_LIMIT) [[unlikely]] { - throw sourcemeta::blaze::EvaluationError( - "The evaluation path depth limit was reached " - "likely due to infinite recursion"); - } - - // We still need to somewhat keep track of this to prevent infinite - // recursion - this->evaluate_path_size += relative_schema_location.size(); } } @@ -42,8 +22,6 @@ auto EvaluationContext::pop(const std::size_t relative_schema_location_size, if (track) { this->evaluate_path.pop_back(relative_schema_location_size); this->instance_location.pop_back(relative_instance_location_size); - } else { - this->evaluate_path_size -= relative_schema_location_size; } } diff --git a/src/evaluator/dispatch.h b/src/evaluator/dispatch.h index ae04e493..3f2404ab 100644 --- a/src/evaluator/dispatch.h +++ b/src/evaluator/dispatch.h @@ -390,7 +390,8 @@ HANDLER_START { result = true; assert(std::holds_alternative(entry)); for (const auto &child : std::get(entry).children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; break; } @@ -416,7 +417,8 @@ HANDLER_START { result = true; assert(std::holds_alternative(entry)); for (const auto &child : std::get(entry).children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; EVALUATE_END(assertion, AssertionArrayPrefixEvaluate); } @@ -439,7 +441,8 @@ HANDLER_START { const auto &target{sourcemeta::jsontoolkit::get( instance, logical.relative_instance_location)}; for (const auto &child : logical.children) { - if (evaluate_step(child, callback, target, property_target, context)) { + if (evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = true; // This boolean value controls whether we should be exhaustive if (!logical.value) { @@ -457,7 +460,8 @@ HANDLER_START { const auto &target{sourcemeta::jsontoolkit::get( instance, logical.relative_instance_location)}; for (const auto &child : logical.children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; break; } @@ -470,7 +474,8 @@ HANDLER_START { EVALUATE_BEGIN(logical, LogicalWhenType, target.type() == logical.value); result = true; for (const auto &child : logical.children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; break; } @@ -484,7 +489,8 @@ HANDLER_START { target.is_object() && target.defines(logical.value)); result = true; for (const auto &child : logical.children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; break; } @@ -498,7 +504,8 @@ HANDLER_START { target.is_array() && target.size() > logical.value); result = true; for (const auto &child : logical.children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; break; } @@ -514,7 +521,8 @@ HANDLER_START { const auto &target{sourcemeta::jsontoolkit::get( instance, logical.relative_instance_location)}; for (const auto &child : logical.children) { - if (evaluate_step(child, callback, target, property_target, context)) { + if (evaluate_step(child, callback, target, property_target, depth + 1, + context)) { if (has_matched) { result = false; // This boolean value controls whether we should be exhaustive @@ -549,7 +557,7 @@ HANDLER_START { instance, logical.relative_instance_location)}; for (std::size_t cursor = 0; cursor < condition_end; cursor++) { if (!evaluate_step(logical.children[cursor], callback, target, - property_target, context)) { + property_target, depth + 1, context)) { result = false; break; } @@ -568,7 +576,7 @@ HANDLER_START { for (auto cursor = consequence_start; cursor < consequence_end; cursor++) { if (!evaluate_step(logical.children[cursor], callback, instance, - property_target, context)) { + property_target, depth + 1, context)) { result = false; break; } @@ -579,7 +587,7 @@ HANDLER_START { for (auto cursor = consequence_start; cursor < consequence_end; cursor++) { if (!evaluate_step(logical.children[cursor], callback, instance, - property_target, context)) { + property_target, depth + 1, context)) { result = false; break; } @@ -593,7 +601,8 @@ HANDLER_START { case IS_STEP(ControlGroup): { EVALUATE_BEGIN_PASS_THROUGH(control, ControlGroup); for (const auto &child : control.children) { - if (!evaluate_step(child, callback, instance, property_target, context)) { + if (!evaluate_step(child, callback, instance, property_target, depth + 1, + context)) { result = false; break; } @@ -617,7 +626,7 @@ HANDLER_START { // Note that in this control instruction, we purposely // don't navigate into the target if (!evaluate_step(child, callback, instance, property_target, - context)) { + depth + 1, context)) { result = false; break; } @@ -635,7 +644,8 @@ HANDLER_START { instance, control.relative_instance_location)}; result = true; for (const auto &child : control.children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; break; } @@ -676,7 +686,8 @@ HANDLER_START { const auto &target{sourcemeta::jsontoolkit::get( instance, control.relative_instance_location)}; for (const auto &child : context.labels.at(control.value).get()) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; break; } @@ -697,7 +708,7 @@ HANDLER_START { result = true; for (const auto &child : match->second.get()) { if (!evaluate_step(child, callback, target, property_target, - context)) { + depth + 1, context)) { result = false; EVALUATE_END(control, ControlDynamicAnchorJump); } @@ -736,7 +747,8 @@ HANDLER_START { const auto &target{sourcemeta::jsontoolkit::get( instance, logical.relative_instance_location)}; for (const auto &child : logical.children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = true; break; } @@ -751,7 +763,8 @@ HANDLER_START { const auto &target{sourcemeta::jsontoolkit::get( instance, logical.relative_instance_location)}; for (const auto &child : logical.children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = true; break; } @@ -778,7 +791,7 @@ HANDLER_START { const auto &new_instance{target.at(entry.first)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; context.instance_location.pop_back(); EVALUATE_END(loop, LoopPropertiesUnevaluated); @@ -834,7 +847,7 @@ HANDLER_START { const auto &new_instance{target.at(entry.first)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; context.instance_location.pop_back(); EVALUATE_END(loop, LoopPropertiesUnevaluatedExcept); @@ -863,7 +876,8 @@ HANDLER_START { const auto &substep{loop.children[index->second]}; assert(std::holds_alternative(substep)); for (const auto &child : std::get(substep).children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; EVALUATE_END(loop, LoopPropertiesMatch); } @@ -887,7 +901,8 @@ HANDLER_START { const auto &substep{loop.children[index->second]}; assert(std::holds_alternative(substep)); for (const auto &child : std::get(substep).children) { - if (!evaluate_step(child, callback, target, property_target, context)) { + if (!evaluate_step(child, callback, target, property_target, depth + 1, + context)) { result = false; EVALUATE_END(loop, LoopPropertiesMatchClosed); } @@ -908,7 +923,7 @@ HANDLER_START { const auto &new_instance{target.at(entry.first)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; if (track) { context.instance_location.pop_back(); @@ -936,7 +951,7 @@ HANDLER_START { const auto &new_instance{target.at(entry.first)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; if (track) { context.instance_location.pop_back(); @@ -971,7 +986,7 @@ HANDLER_START { const auto &new_instance{target.at(entry.first)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; if (track) { context.instance_location.pop_back(); @@ -1007,7 +1022,7 @@ HANDLER_START { const auto &new_instance{target.at(entry.first)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; if (track) { context.instance_location.pop_back(); @@ -1039,7 +1054,7 @@ HANDLER_START { const auto &new_instance{target.at(entry.first)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; if (track) { context.instance_location.pop_back(); @@ -1093,7 +1108,7 @@ HANDLER_START { const auto &new_instance{target.at(entry.first)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; if (track) { context.instance_location.pop_back(); @@ -1240,7 +1255,7 @@ HANDLER_START { const auto &new_instance{target.at(entry.first)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, - std::cref(entry.first), context)) { + std::cref(entry.first), depth + 1, context)) { result = false; if (track) { context.instance_location.pop_back(); @@ -1269,7 +1284,7 @@ HANDLER_START { const auto &new_instance{target.at(index)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; if (track) { context.instance_location.pop_back(); @@ -1301,7 +1316,7 @@ HANDLER_START { const auto &new_instance{target.at(index)}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { result = false; context.instance_location.pop_back(); EVALUATE_END(loop, LoopItemsUnevaluated); @@ -1381,7 +1396,7 @@ HANDLER_START { bool subresult{true}; for (const auto &child : loop.children) { if (!evaluate_step(child, callback, new_instance, property_target, - context)) { + depth + 1, context)) { subresult = false; break; } diff --git a/src/evaluator/evaluator.cc b/src/evaluator/evaluator.cc index cdbb949f..a55e74ef 100644 --- a/src/evaluator/evaluator.cc +++ b/src/evaluator/evaluator.cc @@ -1,4 +1,5 @@ #include + #include #include @@ -58,11 +59,22 @@ auto evaluate_step( const std::optional< std::reference_wrapper> &property_target, - sourcemeta::blaze::EvaluationContext &context) -> bool { + const std::uint64_t depth, sourcemeta::blaze::EvaluationContext &context) + -> bool { SOURCEMETA_TRACE_REGISTER_ID(trace_id); using namespace sourcemeta::jsontoolkit; using namespace sourcemeta::blaze; + // Guard against infinite recursion in a cheap manner, as + // infinite recursion will manifest itself through huge + // ever-growing evaluate paths + constexpr auto DEPTH_LIMIT{300}; + if (depth > DEPTH_LIMIT) [[unlikely]] { + throw sourcemeta::blaze::EvaluationError( + "The evaluation path depth limit was reached " + "likely due to infinite recursion"); + } + #define STRINGIFY(x) #x #define EVALUATE_BEGIN(step_category, step_type, precondition) \ @@ -259,7 +271,7 @@ evaluate_internal(const sourcemeta::jsontoolkit::JSON &instance, -> bool { bool overall{true}; for (const auto &step : steps) { - if (!evaluate_step(step, callback, instance, std::nullopt, context)) { + if (!evaluate_step(step, callback, instance, std::nullopt, 0, context)) { overall = false; break; } @@ -268,7 +280,6 @@ evaluate_internal(const sourcemeta::jsontoolkit::JSON &instance, // The evaluation path and instance location must be empty by the time // we are done, otherwise there was a frame push/pop mismatch assert(context.evaluate_path.empty()); - assert(context.evaluate_path_size == 0); assert(context.instance_location.empty()); assert(context.resources.empty()); return overall; @@ -296,7 +307,6 @@ auto evaluate(const Instructions &steps, EvaluationContext &context) -> bool { // Do a full reset for the next run assert(context.evaluate_path.empty()); - assert(context.evaluate_path_size == 0); assert(context.instance_location.empty()); assert(context.resources.empty()); context.labels.clear(); diff --git a/src/evaluator/include/sourcemeta/blaze/evaluator_context.h b/src/evaluator/include/sourcemeta/blaze/evaluator_context.h index d7f61297..164795a4 100644 --- a/src/evaluator/include/sourcemeta/blaze/evaluator_context.h +++ b/src/evaluator/include/sourcemeta/blaze/evaluator_context.h @@ -60,7 +60,6 @@ class SOURCEMETA_BLAZE_EVALUATOR_EXPORT EvaluationContext { -> bool; auto unevaluate() -> void; - // TODO: Remove this const sourcemeta::jsontoolkit::JSON null{nullptr}; // Exporting symbols that depends on the standard C++ library is considered @@ -70,7 +69,6 @@ class SOURCEMETA_BLAZE_EVALUATOR_EXPORT EvaluationContext { #pragma warning(disable : 4251 4275) #endif sourcemeta::jsontoolkit::WeakPointer evaluate_path; - std::uint64_t evaluate_path_size{0}; sourcemeta::jsontoolkit::WeakPointer instance_location; const std::hash hasher_{}; std::vector resources;