Skip to content

Commit d5d37ab

Browse files
committed
AK+LibRegex: Only set node metadata on Trie::ensure_child if missing
a290034 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.
1 parent 2c5beea commit d5d37ab

File tree

2 files changed

+16
-10
lines changed

2 files changed

+16
-10
lines changed

AK/Trie.h

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,23 +95,29 @@ class Trie {
9595
ValueType const& value() const { return m_value; }
9696
ValueType& value() { return m_value; }
9797

98-
ErrorOr<Trie*> ensure_child(ValueType value, Optional<MetadataType> metadata = {})
98+
template<typename F = Function<Optional<MetadataType>()>>
99+
ErrorOr<Trie*> ensure_child(ValueType value, F metadata_if_missing = [] { return OptionalNone {}; })
99100
{
100101
auto it = m_children.find(value);
101102
if (it == m_children.end()) {
102103
OwnPtr<Trie> node;
104+
Optional<MetadataType> missing_metadata;
105+
106+
if constexpr (requires { { metadata_if_missing() } -> SpecializationOf<ErrorOr>; })
107+
missing_metadata = TRY(metadata_if_missing());
108+
else
109+
missing_metadata = metadata_if_missing();
110+
103111
if constexpr (requires { { value->try_clone() } -> SpecializationOf<ErrorOr>; })
104-
node = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Trie(TRY(value->try_clone()), move(metadata))));
112+
node = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Trie(TRY(value->try_clone()), move(missing_metadata))));
105113
else
106-
node = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Trie(value, move(metadata))));
114+
node = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Trie(value, move(missing_metadata))));
107115
auto& node_ref = *node;
108116
TRY(m_children.try_set(move(value), node.release_nonnull()));
109117
return &static_cast<BaseType&>(node_ref);
110118
}
111119

112120
auto& node_ref = *it->value;
113-
if (metadata.has_value())
114-
node_ref.m_metadata = move(metadata);
115121
return &static_cast<BaseType&>(node_ref);
116122
}
117123

@@ -129,9 +135,9 @@ class Trie {
129135
};
130136
for (; it != end; ++it) {
131137
if constexpr (requires { { ValueType::ElementType::try_create(*it) } -> SpecializationOf<ErrorOr>; })
132-
last_root_node = static_cast<Trie*>(TRY(last_root_node->ensure_child(TRY(ValueType::ElementType::try_create(*it)), TRY(invoke_provide_missing_metadata(static_cast<BaseType&>(*last_root_node), it)))));
138+
last_root_node = static_cast<Trie*>(TRY(last_root_node->ensure_child(TRY(ValueType::ElementType::try_create(*it)), [&] { return invoke_provide_missing_metadata(static_cast<BaseType&>(*last_root_node), it); })));
133139
else
134-
last_root_node = static_cast<Trie*>(TRY(last_root_node->ensure_child(*it, TRY(invoke_provide_missing_metadata(static_cast<BaseType&>(*last_root_node), it)))));
140+
last_root_node = static_cast<Trie*>(TRY(last_root_node->ensure_child(*it, [&] { return invoke_provide_missing_metadata(static_cast<BaseType&>(*last_root_node), it); })));
135141
}
136142
last_root_node->set_metadata(move(metadata));
137143
return static_cast<BaseType*>(last_root_node);
@@ -144,9 +150,9 @@ class Trie {
144150
Trie* last_root_node = &traverse_until_last_accessible_node(it, end);
145151
for (; it != end; ++it) {
146152
if constexpr (requires { { ValueType::ElementType::try_create(*it) } -> SpecializationOf<ErrorOr>; })
147-
last_root_node = static_cast<Trie*>(TRY(last_root_node->ensure_child(TRY(ValueType::ElementType::try_create(*it)), {})));
153+
last_root_node = static_cast<Trie*>(TRY(last_root_node->ensure_child(TRY(ValueType::ElementType::try_create(*it)))));
148154
else
149-
last_root_node = static_cast<Trie*>(TRY(last_root_node->ensure_child(*it, {})));
155+
last_root_node = static_cast<Trie*>(TRY(last_root_node->ensure_child(*it)));
150156
}
151157
return static_cast<BaseType*>(last_root_node);
152158
}

Libraries/LibRegex/RegexOptimizer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1467,7 +1467,7 @@ void Optimizer::append_alternation(ByteCode& target, Span<ByteCode> alternatives
14671467
node_key_bytes.append(edge.jump_insn);
14681468
}
14691469

1470-
active_node = static_cast<decltype(active_node)>(MUST(active_node->ensure_child(DisjointSpans<ByteCodeValueType const> { move(node_key_bytes) }, Vector<NodeMetadataEntry> {})));
1470+
active_node = static_cast<decltype(active_node)>(MUST(active_node->ensure_child(DisjointSpans<ByteCodeValueType const> { move(node_key_bytes) }, [] -> Vector<NodeMetadataEntry> { return {}; })));
14711471

14721472
auto next_compare = [&alternative, &state](StaticallyInterpretedCompares& compares) {
14731473
TemporaryChange state_change { state.instruction_position, state.instruction_position };

0 commit comments

Comments
 (0)