Skip to content

Fix a join reporting an empty result for a non-empty byte node - #178

Merged
imlvts merged 3 commits into
Adam-Vandervorst:masterfrom
imlvts:fix/join-byte-node-empty-status
Nov 4, 2026
Merged

imlvts merged 3 commits into
Adam-Vandervorst:masterfrom
imlvts:fix/join-byte-node-empty-status

Conversation

@imlvts

@imlvts imlvts commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

DenseByteNode::join_child_into passed on the None that two empty nodes join to. merge_from_list_node merges the per-slot statuses, so a list node holding only dangling paths the byte node also has made the whole join None, and the byte node's other contents were dropped. Two dangling paths joined stay one dangling path, which is no change: Identity. join_child_into_left already did this.

This is the debug assertion the algebraic fuzzer reported as finding 6, and one way a join lost values.

DenseByteNode::join_child_into passed on the None that two empty nodes
join to.  merge_from_list_node merges the per-slot statuses, so a list
node holding only dangling paths the byte node also has made the whole
join None, and the byte node's other contents were dropped.  Two
dangling paths joined stay one dangling path, which is no change:
Identity.  join_child_into_left already did this.

This is the debug assertion the algebraic fuzzer reported as finding 6,
and one way a join lost values.
@adamv-symbolica

Copy link
Copy Markdown
Contributor

Perhaps a cleaner example

let mut a = PathMap::<()>::new();
a.create_path([0u8]); a.create_path([1u8]); a.set_val_at([2u8], ());
let mut b = PathMap::<()>::new();
b.create_path([0u8]); b.create_path([1u8]);
assert(a.join(&b).contains([2u8]))

Two dangling paths shared by both sides and a value only in the left one:
the join must keep the value.
@imlvts

imlvts commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

done

Finding 4 of the algebraic fuzzer: with b a clone of a plus one value,
a.join(&b) dropped that value while b.join(&a) kept it.  Same cause as
the empty-status fix: the byte node reported the list node's join as
empty.
@imlvts

imlvts commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

re: "add the join fix separately to [here]"
this PR already has that fix. moved the test over.

@Adam-Vandervorst

Copy link
Copy Markdown
Owner

LGTM

@imlvts
imlvts merged commit cb5ba6a into Adam-Vandervorst:master Nov 4, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants