Compare byte-code objects slot-wise in equal and sxhash, like GNU's PVEC_CLOSURE

[tag-und-nacht]

Summary

equal on two distinct byte-code objects always answered nil, and an equal hash table never found a closure by a rebuilt twin. GNU compares a PVEC_CLOSURE exactly like a vector: internal_equal_1 (src/fns.c:2984-2998 in emacs-31.1, :2987-3001 on master) checks ASIZE first, then every slot element-wise (arglist, bytecode string, constants, max depth, doc, interactive spec, extras), and sxhash_obj (:5525-5536) hashes the same slots through sxhash_vector. Here the pseudovector arm of equal_value_inner, its error-propagating twin, emacs_sxhash_obj, and the equal-table key builder all fell through to identity for VecLikeType::ByteCode, while interpreted closures were already compared structurally.

This PR adds the slot walk in all four places, in GNU's slot order, over the typed fields that stand in for GNU's vector (bytecode_equal, try_bytecode_equal, emacs_sxhash_bytecode, bytecode_to_equal_key). Two cases have no GNU counterpart and compare strictly: a function with no retained GNU bytes compares its decoded instructions, and a captured env alist is compared in addition to the constants. The equal-table key reuses existing HashKey variants (a tagged EqualVec with the bytecode keyed byte-exact), so the pdump codec is untouched.

User-visible failure

After a language server shut down cleanly (follow-up to #310), every mouse movement produced

LSP :: Sending to process failed with the following error: Process nixd-lsp not running: killed: 9

For requests sent with :mode 'unchanged, lsp--send-request-async installs a cancel closure on the global post-command-hook and later removes it by rebuilding an equal closure and calling remove-hook (delete). With identity-only equal the hook was never removed; it fired on every later command, and with eldoc-box's mouse mode setting track-mouse every mouse motion is a command, so each one sent $/cancelRequest to the dead workspace.

Oracle

Same probe under GNU Emacs 32 and neomacs (-Q --batch), before and after:

(let* ((a (make-byte-code 257 "\300\207" [42] 2))
       (b (make-byte-code 257 "\300\207" [42] 2))
       (proto (make-byte-code 257 "\300\207" [placeholder] 2))
       (k1 (make-closure proto 'w)) (k2 (make-closure proto 'w)))
  (list (equal a b) (equal k1 k2) (length (delete b (list a 1)))
        (= (sxhash-equal a) (sxhash-equal b))))

	result
GNU Emacs 32	(t t 1 t)
neomacs before	(nil nil 2 nil)
neomacs after	(t t 1 t)

With the user's real lsp-mode (plists) and real nixd in batch: the cancel closure count on the global hook goes 1 → 0 after one command in another buffer, and stays 0 after the server is shut down, identical to vanilla. Before the fix the closure stayed on the hook (count 1) and remove-hook with the factory's rebuilt closure was a no-op.

Test

equal_compares_byte_code_functions_element_wise_like_gnu_closures (red first) pins a twenty-probe list against GNU's values: make-byte-code twins; differing constants, depth, doc, bytecode, arglist, and slot count (ASIZE); make-closure instances with equal and different captures; closure vs. plain vector; delete, member, sxhash-equal, and equal hash-table lookup.

Ran on macOS (Darwin 25.6.0, arm64): the new test plus the value::tests, hashtab, equal_including, sxhash, bytecode::tests, make_closure, and closure filters. One slow test (expanded_cache_replay_preserves_oclosure_define_class_registration) failed once in the parallel run and passed solo three times.

🤖 Generated with Claude Code

https://claude.ai/code/session_0117NsCB7AbwgdEqduGF5Kww

[eval-exec]

Thanks—this is the right overall direction. GNU Emacs compares PVEC_CLOSURE objects slot-wise in internal_equal_1 and hashes them through the corresponding vector path in sxhash_obj.

I found two correctness issues that should be fixed before merging:

1. The equal-table key loses environment presence.
   bytecode_to_equal_key maps both env: None and env: Some(Value::NIL) to HashKey::Nil, although bytecode_equal distinguishes them. Some(nil) is reachable when a closure captures a nil lexical environment. Consequently, two byte-code objects can be unequal while being treated as the same key in an equal hash table.

2. Docstrings use Rust representation equality instead of GNU string equality.
   The comparison at value/mod.rs compares LispString directly, including its internal unibyte/multibyte representation. GNU string equality compares character count, byte count and contents—not that representation flag. I verified with GNU Emacs that an ASCII unibyte string and string-as-multibyte equivalent are equal, and byte-code objects containing them are also equal. Neomacs currently returns unequal here.

I reproduced both problems with release-mode regression tests. The PR's existing twenty-probe GNU test passes, but it does not exercise captured environments, decoded-operation fallback, or alternate equal string representations.

Please add regression coverage for:

• env: None versus env: Some(nil), including equal hash-table lookup;
• unibyte versus equivalent multibyte docstrings;
• byte-code objects without retained GNU byte strings;
• constants and captured environments both participating in sxhash.

Longer-term, I recommend defining one typed structural view of the observable closure slots and deriving normal equality, fallible equality, sxhash, and equal-table keys from it. Currently the closure schema is encoded separately in four places, which makes semantic drift likely.

The core approach is good, but these two equality/key inconsistencies are blockers for merging as-is.

[Copilot]

🔵 Needs a closer look

It changes core equal/sxhash/equal-hash-table semantics affecting the whole runtime, so despite verified internal consistency and strong oracle tests, a human should give final sign-off.

Pull request overview

This PR fixes byte-code (closure) objects so equal, sxhash-equal, and equal hash tables compare them slot-wise instead of by identity, mirroring how GNU Emacs treats a PVEC_CLOSURE exactly like a vector (internal_equal_1 / sxhash_obj). Previously two distinct-but-identical make-byte-code/make-closure objects were never equal and never collided in an equal hash table, which broke lsp-mode's remove-hook on the global post-command-hook (the observable failure described in the PR and follow-up to #310). The change lives entirely in the core runtime value/hashtab layer and reuses existing HashKey variants, so the pdump codec is untouched.

Changes:

• Add slot-wise byte-code comparison in equal and its error-propagating twin (bytecode_equal / try_bytecode_equal) in GNU's slot order (arglist, bytecode string, constants + captured env, max depth, doc, interactive, extras), gating on GNU's ASIZE slot count first.
• Add the matching equal-hash-table key builder (bytecode_to_equal_key) and sxhash-equal path (emacs_sxhash_bytecode) so equal objects share a bucket and hash alike.
• Add a 20-probe oracle test pinned to GNU Emacs 32 values covering twins, differing slots, make-closure captures, delete/member/gethash, and sxhash-equal.

File summaries

File	Description
crates/neovm-core/src/emacs_core/runtime/value/mod.rs	Adds ByteCode arms to equal_value_inner, try_equal_value_inner, and to_equal_key_depth_swp, plus bytecode_equal, try_bytecode_equal, and bytecode_to_equal_key.
crates/neovm-core/src/emacs_core/runtime/hashtab/mod.rs	Adds emacs_sxhash_bytecode and routes VecLikeType::ByteCode through it in emacs_sxhash_obj.
crates/neovm-core/src/emacs_core/runtime/value/tests/mod.rs	Adds equal_compares_byte_code_functions_element_wise_like_gnu_closures GNU-oracle test.

I reviewed all four dispatch points for mutual consistency and verified the key invariants (equal ⟹ same HashKey for hash-table correctness, and equal ⟹ same sxhash) hold — including the deliberate slot‑2 env‑vs‑constants handling in emacs_sxhash_bytecode, which preserves the hash invariant because two equal closures always share env‑presence and equal values. The try_bytecode_equal walk matches bytecode_equal exactly in order and extra_depth weights, and bytecode_to_equal_key includes every field bytecode_equal compares. I found no objective defects to comment on.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[tag-und-nacht]

Both issues confirmed and fixed in the follow-up commit, and I took the structural suggestion now rather than later.

1. Environment presence in the key — reproduced: env: None vs env: Some(nil) were equal-distinct but keyed identically, so an equal table found one by the other. The key now carries the captured environment as its own element, with an absent environment keyed as #<absent> and a present nil as nil.

2. Docstring equality — reproduced: LispString::eq compares the representation flag. Docstrings now compare under GNU string equality (schars, sbytes, contents) via gnu_string_contents_equal, and the key includes the character count so two raw bytes and the one multibyte character with those bytes stay distinct there too. (Their sxhash-equal values collide, as GNU's byte hashing makes them; unequal objects sharing a hash is legal, and the test says so.)

3. Found by the requested coverage: with a captured environment the hash read the environment instead of the constants, so sxhash-equal ignored the constants entirely. Fixed by the same change.

Structure: ByteCodeFunction::structural_slots() (runtime/bytecode/chunk.rs) is now the single typed view — GNU's slot order, the captured environment beside the constants as its own ByteCodeSlot, Absent distinct from a present nil, Text for docstrings, Ops standing in for a missing GNU byte string. bytecode_equal and try_bytecode_equal share one walk over it (bytecode_slots_equal), and emacs_sxhash_bytecode and bytecode_to_equal_key fold the same sequence. The schema is spelled once; the four consumers cannot drift independently any more.

Regression coverage added (each probe asserts equal, sxhash-equal agreement, and equal-table lookup together, on objects built the way Op::MakeClosure / make-closure build them, since make-byte-code cannot reach these states):

• equal_table_keys_keep_the_presence_of_a_captured_environment — absent vs captured nil, captured nil twins, differing captures;
• docstrings_compare_with_gnu_string_equality_not_representation — unibyte vs multibyte ASCII (equal), raw bytes vs é (unequal);
• functions_without_gnu_bytes_compare_their_decoded_instructions — same/different instructions, bytes vs none;
• sxhash_equal_folds_constants_and_the_captured_environment.

All were red first. value::tests, hashtab, bytecode::tests, and the equal_including/sxhash suites pass on macOS (105 tests), plus the original twenty-probe GNU test.

[eval-exec]

Approved after the updated implementation and the typed structural-key follow-up. The byte-code equality, sxhash, equal-table, and sequence-removal paths now share one structural contract; absence is domain-tagged rather than encoded as Lisp-reachable sentinel text; the hot equality/hash walk is allocation-free; and the behavior is covered by red-first regression tests plus a release differential oracle against GNU Emacs 31.0.90.