fix: compare extern references without an equals method by identity in the JS runtime - #6498
Open
fabiomadge wants to merge 8 commits into
Open
fix: compare extern references without an equals method by identity in the JS runtime#6498fabiomadge wants to merge 8 commits into
fabiomadge wants to merge 8 commits into
Conversation
…n the JS runtime
The runtime's universal equality dispatches objects that are not
Dafny-generated classes (marked with _tname) to a.equals(b). Extern
class instances carry no _tname, so an ordinary extern class with no
equals method crashed any generic comparison — constructing {a, b}
died with 'a.equals is not a function' — and one WITH a structural
equals subverted verified reference equalities, as on the other
backends.
Fall back to identity for objects without an equals method, matching
the equality Dafny gives reference types. Defining an equals method
remains the structural opt-in for extern value types, the same contract
as the Go runtime's EqualsGeneric (dafny-lang#6497). The only observable change
is on programs that previously crashed: JS collections compare
elements through this same dispatch, so set/map/seq operations over
method-less externs become sound as well.
Part of dafny-lang#6491.
This was referenced Jul 28, 2026
Document that extern objects compare by identity unless they define an equals
method (the opt-in this PR relies on), and point to the datatype /
type {:extern} T(==) alternatives.
Relates to dafny-lang#6491.
The new arm is not reached only by extern references, and the crash it replaces
was reachable from pure Dafny. Two native reference kinds are plain JavaScript
objects with no equals method:
- a multi-dimensional array, since _dafny.newArray returns a bare {dims, elmts}
object literal with no _tname and no equals;
- an iterator.
Comparing either generically -- here through a set -- fails on master with
"TypeError: a.equals is not a function", and compares by identity with this
change. Verified against a master build: it crashes, this branch prints
"true false" twice and verifies.
Added a test for both, with no extern code and no --input shim, and widened the
news fragment, which described the fix as extern-only.
The uniformity check requires a backend-specific test to either use %testDafnyForEachCompiler or explain itself. This one cannot use the former, because Java does not support iterators.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #6491, JavaScript variant — the sibling of #6497 (Go).
The runtime's universal equality
_dafny.areEqualsends objects that are not Dafny-generated classes (no_tnamemarker) toa.equals(b). Extern class instances carry no_tname, so:equalsmethod crashed any generic comparison —var s := {a, b}died withTypeError: a.equals is not a function;equalssilently subverted verified reference equalities, as on the other backends.Fix. A 3-line arm in the dispatch: objects without an
equalsmethod compare by identity — the equality Dafny gives reference types. Defining anequalsmethod remains the structural opt-in for extern value types, the same contract as Go'sEqualsGeneric(#6497). Because JS collections are linear scans through this same dispatch, set/map/seq operations over method-less externs become sound too — no hashing discontinuity exists on this backend.The behavioral delta is confined to programs that previously crashed. Probed inhabitants of the changed arm: extern classes without
equals, plain object literals, host objects (Date), and typed arrays all previously threw and now compare by identity; everyequals-bearing object (including all runtime value types) is dispatched exactly as before. Survey of in-repo JS externs (5 files): all define classes with noequals— each was one generic comparison away from the crash; none relies on structural equality.One nuance for review: the check is per-object capability (
typeof a.equals !== "function"), not per-type — an extern inheritingequalsthrough its prototype chain counts as opted in. That is the only type boundary JS offers, and it matches the pre-existing structural dispatch.The regression test uses a method-less extern class and exercises set construction and membership (previously the crash site).
Scope: the crash was reachable without externs
The new arm is not reached only by extern references. Two native reference kinds are plain JavaScript objects with no
equalsmethod — a multi-dimensional array (_dafny.newArrayreturns a bare{dims, elmts}literal, with no_tnameeither) and an iterator. Comparing either generically fails on master withTypeError: a.equals is not a function:Verified against a master build: it crashes; this branch prints
true falseand verifies.git-issue-6491-js-native.dfycovers both kinds, with no extern code and no--inputshim, and the news fragment has been widened accordingly — it previously described the fix as extern-only.By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.