From d5d37abfa579bf01fa64621e8616cc80f74702dc Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Thu, 20 Nov 2025 23:47:21 +0100 Subject: [PATCH] AK+LibRegex: Only set node metadata on Trie::ensure_child if missing a290034a8164dcfe83b3679ab848592a97e0e2b1 passed an empty vector to this, which caused nodes that appeared multiple times to reset the trie metadata...which broke the optimisation. This patchset makes the function take a 'provide missing metadata' function instead, and only invokes it when the node is missing rather than unconditionally setting the metadata on all nodes. --- AK/Trie.h | 26 ++++++++++++++++---------- Libraries/LibRegex/RegexOptimizer.cpp | 2 +- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/AK/Trie.h b/AK/Trie.h index c138fdaff39..05ac79b988b 100644 --- a/AK/Trie.h +++ b/AK/Trie.h @@ -95,23 +95,29 @@ public: ValueType const& value() const { return m_value; } ValueType& value() { return m_value; } - ErrorOr ensure_child(ValueType value, Optional metadata = {}) + template()>> + ErrorOr ensure_child(ValueType value, F metadata_if_missing = [] { return OptionalNone {}; }) { auto it = m_children.find(value); if (it == m_children.end()) { OwnPtr node; - if constexpr (requires { { value->try_clone() } -> SpecializationOf; }) - node = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Trie(TRY(value->try_clone()), move(metadata)))); + Optional missing_metadata; + + if constexpr (requires { { metadata_if_missing() } -> SpecializationOf; }) + missing_metadata = TRY(metadata_if_missing()); else - node = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Trie(value, move(metadata)))); + missing_metadata = metadata_if_missing(); + + if constexpr (requires { { value->try_clone() } -> SpecializationOf; }) + node = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Trie(TRY(value->try_clone()), move(missing_metadata)))); + else + node = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Trie(value, move(missing_metadata)))); auto& node_ref = *node; TRY(m_children.try_set(move(value), node.release_nonnull())); return &static_cast(node_ref); } auto& node_ref = *it->value; - if (metadata.has_value()) - node_ref.m_metadata = move(metadata); return &static_cast(node_ref); } @@ -129,9 +135,9 @@ public: }; for (; it != end; ++it) { if constexpr (requires { { ValueType::ElementType::try_create(*it) } -> SpecializationOf; }) - last_root_node = static_cast(TRY(last_root_node->ensure_child(TRY(ValueType::ElementType::try_create(*it)), TRY(invoke_provide_missing_metadata(static_cast(*last_root_node), it))))); + last_root_node = static_cast(TRY(last_root_node->ensure_child(TRY(ValueType::ElementType::try_create(*it)), [&] { return invoke_provide_missing_metadata(static_cast(*last_root_node), it); }))); else - last_root_node = static_cast(TRY(last_root_node->ensure_child(*it, TRY(invoke_provide_missing_metadata(static_cast(*last_root_node), it))))); + last_root_node = static_cast(TRY(last_root_node->ensure_child(*it, [&] { return invoke_provide_missing_metadata(static_cast(*last_root_node), it); }))); } last_root_node->set_metadata(move(metadata)); return static_cast(last_root_node); @@ -144,9 +150,9 @@ public: Trie* last_root_node = &traverse_until_last_accessible_node(it, end); for (; it != end; ++it) { if constexpr (requires { { ValueType::ElementType::try_create(*it) } -> SpecializationOf; }) - last_root_node = static_cast(TRY(last_root_node->ensure_child(TRY(ValueType::ElementType::try_create(*it)), {}))); + last_root_node = static_cast(TRY(last_root_node->ensure_child(TRY(ValueType::ElementType::try_create(*it))))); else - last_root_node = static_cast(TRY(last_root_node->ensure_child(*it, {}))); + last_root_node = static_cast(TRY(last_root_node->ensure_child(*it))); } return static_cast(last_root_node); } diff --git a/Libraries/LibRegex/RegexOptimizer.cpp b/Libraries/LibRegex/RegexOptimizer.cpp index 7467feef1a6..971e5765416 100644 --- a/Libraries/LibRegex/RegexOptimizer.cpp +++ b/Libraries/LibRegex/RegexOptimizer.cpp @@ -1467,7 +1467,7 @@ void Optimizer::append_alternation(ByteCode& target, Span alternatives node_key_bytes.append(edge.jump_insn); } - active_node = static_cast(MUST(active_node->ensure_child(DisjointSpans { move(node_key_bytes) }, Vector {}))); + active_node = static_cast(MUST(active_node->ensure_child(DisjointSpans { move(node_key_bytes) }, [] -> Vector { return {}; }))); auto next_compare = [&alternative, &state](StaticallyInterpretedCompares& compares) { TemporaryChange state_change { state.instruction_position, state.instruction_position };