Object representation cleanup: typed field mutability and structural access - #8597
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2cd0226fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8597 +/- ##
==========================================
+ Coverage 75.92% 76.33% +0.41%
==========================================
Files 475 476 +1
Lines 63029 63253 +224
==========================================
+ Hits 47854 48284 +430
+ Misses 15175 14969 -206
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8597 |
cristianoc
added a commit
that referenced
this pull request
Aug 29, 2026
Four review findings on #8597, all confirmed by probe or inspection: Variance ignored field mutability. The old phantom "x#=" setter member was an arrow whose contravariant occurrence incidentally made settable fields invariant; removing the phantom left compute_variance treating every field payload with the ambient variance, so an explicitly covariant parameter could annotate a settable field and leak write capability through an abstract type. The Tfield arm now sends a Mutable field's payload through Variance.full, like a mutable record label; Immutable fields keep the ambient variance, preserving read-only covariance. Pinned by object_settable_field_covariant_param (Bad_variance). Writes instantiated polymorphic field schemes. Assigning to {@set "id": 'a. 'a => 'a} typed the value at one instance, so a monomorphic function satisfied the field while reads kept instantiating the unchanged scheme. A field's type is a scheme: reading eliminates it, writing must establish it. The write path now uses the checker's scheme-introduction discipline - fixed instantiation, typing at that instance, check_univars - extracted as type_object_field_value next to its record twin type_label_exp, returning the value at an ordinary instance as both siblings do. type_label_exp's PR#4862 retry is a label-specific completeness recovery and is deliberately not replicated; the helper's comment records that. Pinned by object_write_poly_field_less_general (Less_general) and a positive settable-poly case in object_poly_field. Along the way, instance_poly's positional boolean becomes ~fixed with a contract comment in ctype.mli: the flag controls fixed copying of polymorphic-variant rows; scheme introduction is identified by the whole operation, not by this flag. reanalyze missed Texp_object_literal. Side-effect analysis fell through to the permissive default, so a dead binding whose object literal called effectful code was classified as removable; it now checks every field expression (ObjectLiteralSideEffects deadcode case). Termination analysis crashed on the wildcard; it now compiles the literal as an ordered sequence of its fields - ordered, not unordered, because fields evaluate in source order and crediting a later field's progress past a non-returning earlier field would be unsound (the testObjectLiteralRecursionFirst case is now reported as a possible infinite loop while testObjectLiteralProgressFirst passes). Texp_object_get/Texp_object_set traverse their receiver and value instead of asserting (testObjectAccess). Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
This was referenced Aug 29, 2026
Collaborator
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
cristianoc
commented
Aug 29, 2026
cristianoc
left a comment
cristianoc
left a comment
Collaborator
Author
There was a problem hiding this comment.
Summary
Reported by Grok 4.6.
Assignment treats a private object row as an inferred open row (opened_object includes Tconstr), while unification only promotes when the rest is a Tvar. A write can therefore promote the declaration's shared mutability cell, and for_saving persists that @set into the .cmi. Closed-row coercion, signature inclusion, and the copy-session story otherwise look correct.
Issue counts by severity
- bugs: 1
- suggestions: 0
- nits: 0
| filter_method_field env name priv ty1 | ||
| | Tobject (f, _) -> filter_method_field env name priv f | ||
| write_field ~opened:true ty1 | ||
| | Tobject f -> write_field ~opened:(opened_object ty) f |
Collaborator
Author
There was a problem hiding this comment.
[bug] Reported by Grok 4.6.
filter_object_field_for_write gates Immutable→Mutable promotion with opened_object, which is true for a private-row terminator (Tconstr of t#row). Unification uses is_Tvar rest and correctly refuses to grant @set on a private type; assignment does not. Because that row is not a generic Tvar, copies share the declaration's mutability cell, so let write = (o: t) => o["x"] = 1 on type t = private {.."x": int} mutates t in place. After that write, t unifies with {..@set "x": int} in the same file and in dependents that read the .cmi (for_saving snapshots the promoted value). A twin module with only a read does not leak. The last commit closed the includecore direction and missed this dual.
Suggestion: Gate promotion with is_Tvar (repr (object_row ty)), matching unify_mutability. Keep type t = private {..@set "x": int} writable. Add super_errors pins for a private readonly write, for t still not unifying with {..@set "x": int} after that error, and for a module interface that does not grant @set being unwritable from outside.
Collaborator
Author
There was a problem hiding this comment.
Response authored by Codex (OpenAI).
Confirmed and fixed in 29c907be0. Assignment now promotes or adds write capability only when the object row terminator is a Tvar, matching unify_mutability; object_row already returns the representative, so the implementation uses is_Tvar (object_row ty). Private rows remain writable when the field is already @set.
The underlying ambiguity is also removed from the API: opened_object is now object_row_is_structurally_open, with a contract stating that structural openness includes Tunivar and private-row Tconstr terminators but does not permit strengthening. The write helper uses ~can_promote rather than ~opened.
Coverage includes direct and signature-mediated private-row write errors and an end-to-end positive private {..@set ...} case. Instead of trying to continue source typing after the expected error, the representation-level test calls filter_object_field_for_write directly and verifies both that the Tconstr row is structurally open and that rejection leaves the declaration cell Immutable; this directly pins the no-leak condition that saving relies on. Compiler build, focused error tests, unit tests, and formatting pass.
Member
|
Is this one ready for review? As you only requested review for part 2 of the stack? |
Collaborator
Author
I had a tab open with the review request menu open but not clicked. |
…cleanup Add behavior-pinning tests for the structural-object mutability semantics (currently encoded via phantom "x#=" setter members), ahead of the staged representation cleanup proposed in #8584. tests/tests/src/object_mutability_pin.res pins the compiling cases: closed mutable-to-immutable covariance, open-source and open-target coercions, assignment- and coercion-driven strengthening of open rows, and a generalized getter used at both mutabilities. Two cases are marked EXPECTED TO FLIP with the rationale in place: the unequal-type coercion and the unrelated-type assignment (getter int acquiring setter string), which today produces a value of declared type int that is the string "hello" at runtime - the type-preservation failure the cleanup closes. Seven super_errors fixtures pin the rejecting directions: closed-row writes, both-open invariance, readonly-to-mutable coercions, mutable-to-mutable invariance with unequal types, read-only callers against strengthened rows, and writes after a closed-source-to-open-target coercion. Part of #8584. Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
Only bare @set remains recognized on object-type fields. The undocumented forms - @get (bare or with the null/undefined/nullable payload) and @set({no_get: ...}) - lose their implementation entirely: they now behave like any other unrecognized attribute. A nullable getter type is written directly (null<int>, undefined<int>, nullable<int>), and @set with a payload no longer marks a field settable, so writes to such fields fail with the standard missing-setter error. process_method_attributes_rev and its config parsing collapse into an 8-line bare-@set recognizer; the No_get branch and null/undefined type lifting disappear from process_getter_setter. Bs_syntaxerr's Unsupported_predicates variant is deleted with its only raisers, along with its ERROR_VARIANTS.md row and fixture. Removed features leave no test trace; mutable_obj_test.res's no_get case becomes bare @set with identical generated JS. Stage 0 of the object-representation cleanup (#8584). Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
Mechanical deletions with no user-visible change, all provably dead since the class system's removal: - Tobject loses its class-abbreviation memo: the (Path.t * type_expr list) option ref second component was never constructed (every creation site built ref None; the surviving writers only propagated an existing Some). Its readers and propagation sites go with it: update_level's abbreviation branch, full_expand's Some-branch (the function is now repr . expand_head), the unify_fields name-propagation postlude, find_cltype_for_path and the class-abbreviation build_subtype arm, normalize_type_rec's name handling, and the nondep/subst/copy plumbing. - Texp_send drops its always-None third field and the single-constructor Tmeth_name wrapper (now a plain string). Typecore's degenerate match, the always-None obj_meths ref, and the dead Meths.fold branch of the undefined-method error handler are removed; the Meths module itself was then unused (Vars stands alone). - The frontend ## handling is deleted (unreachable from the parser); its one producer, @deriving(jsConverter)'s js_field, now builds Pexp_send directly. The #= arm's ##-peeling fallback and the bare-## error go too. - The outcometree class layer had no producers left: out_class_type, Octy_*, Ocsg_*, Osig_class, Osig_class_type, Otyp_class and their oprint and res_outcome_printer arms are deleted. - Stale class-era comments removed alongside. cmi magic Caml1999I025 -> Caml1999I026 (Tobject shape), cmt magic Caml1999T026 -> Caml1999T027 (Texp_send shape). Stage A of the object-representation cleanup (#8584). Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
Property get/set becomes a pair of primitives present identically in both IRs: Pjs_object_get and Pjs_object_set, replacing the Lambda-only Lsend node and the Lam-only Pjs_unsafe_downgrade primitive (whose name described a Js.t coercion removed years ago). Setter recognition moves from lam_convert to translation: translcore matches the applied setter member directly and emits Pjs_object_set with the property name; bare sends emit Pjs_object_get. The "#=" suffix recognition and name surgery disappear from lam_convert, whose two Lsend cases are deleted; the suffix channel now ends at translation and is removed entirely by the mutability stage (#8584 Stage D). Also swept while here: the CamlinternalOO/%sendcache method-table comment archaeology in lam_compile, the Lsend design notes in bs_builtin_ppx.mli, commented-out Lsend lines in lam/lam_analysis, and the editor tooling's Js_OO.unsafe_downgrade heuristic (self-documented as unreachable since compiler 9.0). Generated JavaScript is byte-identical across the test corpus (no .mjs diffs). Lam.t serialization changes shape; cmjs are per-compiler-version artifacts rebuilt on compiler update. Stage B of the object-representation cleanup (#8584). Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
{"a": 1} becomes a first-class node end to end: the parser produces
Pexp_object_literal, typecore types it directly as a closed object row
(fields Tpoly-wrapped exactly as written object types are, duplicates
unified against the first occurrence), and translation emits
Pjs_object_create. Generated JavaScript is byte-identical across the test
corpus.
This deletes the literal's former detour through the frontend: the %obj
extension expansion, record_as_js_object, and the synthetic
letmodule-wrapped external (local_external_obj, pval_prim_of_labels,
from_labels) are gone. The jsConverter deriver builds the node directly.
The %obj shape survives only as the frozen-parsetree encoding: the v0
bridge maps Pexp_object_literal to the reserved %obj-extension-over-record
form and back, covered by a new fixture in
tests/syntax_tests/data/ast-mapping/ (the res_parser -test-ast-conversion
roundtrip infra), and AGENTS.md now documents that infra next to the
parsetree0 rule.
Printer, parens (bracket-access and JSX positions), comments table,
parsetree viewer, AST debugger, depend, pprintast, printast, completion,
and semantic highlighting all handle the node; object-key highlighting and
the object-vs-record error hint (whose rewrite suggestion still matched
the old %obj encoding) are preserved, the latter now also firing in
array-item context. Two error snapshots update to more accurate context
phrasing ("this function argument is expecting").
ast magics ResImpl01301/ResIntf01301 -> ResImpl01302/ResIntf01302.
Stage C of the object-representation cleanup (#8584).
Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
Object rows now carry mutability per field instead of encoding writability
as a phantom mangled member. Tfield becomes a record with a linkable
mutability cell in the field_kind mold:
field_mutability = Mutability_value of mutable_flag
| Mutability_link of field_mutability ref
Mutability_link is pure representation (a union-find edge), never a third
source-level state; Btype.mutability_repr reads the class value. Every
field constrained to share a mutability shares one equivalence class, so a
promotion is seen by all views at once. @set is consumed by typetexp; the
frontend's setter mangling ("x#=" members, the synthetic #= operator
handling, the setter-suffix literals, the # property-name restriction) is
deleted end to end.
Property access becomes structural at every layer: the parser produces
Pexp_object_get/Pexp_object_set (send terminology removed), the typedtree
mirrors them, and translation maps them directly onto the Stage B
primitives. The frozen-parsetree bridge encodes the setter as the
historical #=-application over a send and decodes it back.
Assignment types through Ctype.filter_object_field_for_write: Mutable
fields accept writes; an Immutable field of an open row is promoted on the
class representative (set_mutability, a logged Cmutability change); a
closed row yields the new Object_field_not_mutable error suggesting @set;
a missing field reports the field name rather than a mangled member.
The cell obeys three laws. Classes are merged only by unification
(unify_fields links representatives; both the promotion and the link are
trail-logged, so speculative checks restore value and structure).
Enlargement allocates: build_subtype gives changed fields independent
cells, so trial unification against the approximation can never grant
capability to the declared coercion target; settable payloads stay rigid
under enlarge_type, and subtype_fields encodes mutable-target pairs as
deferred unification constraints over wrapped field fragments. Copying
follows the row variable's sharing law: instantiating a generalized row
(generic terminator, decided once per row via a memoized policy walk)
duplicates each class once per copy so aliases stay correlated within the
instance while the scheme and siblings are untouched; other copies share
the representative so promotions reach every occurrence; saving emits
value cells while preserving class sharing.
Copy-session state (the inherited Tsubst/field_kind restoration lists and
the new mutability memo) moves into an explicit, nestable, exception-safe
session stack in Btype (with_copy_session); instantiation, substitution,
and the nondep entry points all own scoped sessions, which is required
because nondep copying can expand an abbreviation and expansion
instantiates - a nested cleanup of a flat session would split classes
mid-copy. cleanup_types is no longer exported, so a copy path cannot
forget its session.
Four soundness flips from the design land as errors with fixtures:
object_setter_type_mismatch and object_coercion_setter_narrower (payload
rigidity), object_write_alias (a write through an annotated alias
strengthens the shared constraint - agreeing with
object_write_original_after_alias, and order-independent), and
object_write_after_forgetting (a coercion never grants write capability).
Diagnostics and the outcome printer resugar @set (including
{..@set "x": t} demands); gentype reads the flag instead of sniffing
"#="; editor completion, hints, and semantic tokens handle the new nodes.
chain_code_test drops its reliance on the removed getter/setter type
split. Generated JavaScript is byte-identical across the corpus except
the intended flips.
The representation-level unit suite ounit_object_mutability_tests.ml (16
tests) covers class merging and order independence, backtracking of
promotion and links, instance independence and intra-instance aliasing,
structure-generalized sharing, terminator/class sharing across every copy
path, nondep session lifecycle (direct, nested expansion, failure), and
saving (value-only cells, class sharing, a Marshal round trip).
ERROR_VARIANTS.md records the new fixtures.
Magics: cmi Caml1999I028, cmt Caml1999T029, ast ResImpl01303/ResIntf01303.
Stage D of the object-representation cleanup (#8584).
Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
A construction survey showed the presence lattice was dead: Fvar had a
single origin - the Private branch of filter_method_field - and
filter_method's only caller passed the literal Public; Fabsent was set
only in the Tfield-vs-Tnil unification arm behind an Fvar guard, making
it transitively unreachable (settling the reachability question deferred
by the design). A probe confirmed the missing-field error against a
closed row is byte-identical with the arm removed: the Fpresent path was
already unconditional failure.
Tfield loses its presence field and the field_kind type is deleted, along
with everything that existed to serve it: field_kind_repr, copy_kind,
dup_kind, set_kind, the Ckind trail constructor, the kind copy-session
state, unify_kind/moregen_kind/eqtype_kind/mcomp_kind, filter_method's
private_flag parameter, copy's Tfield special arm, repr's Fabsent-skip
arms, and the dummy_method sentinel (unconstructed since Stage A) with
its dead guards in ctype, subst, and printtyp.
The companion Stage E candidate - dropping the Tpoly wrapper on object
field types - is rejected after the same survey: polymorphic object
fields ({"f": 'a. 'a => 'a}) are a live surface feature (the parser reads
field types with parse_poly_type_expr), so Tpoly must stay, and
unwrapping only the empty binder would trade the uniform "field type is
Tvar or Tpoly" invariant for a mixed one. The feature, previously
untested, is now pinned: object_poly_field.res/.resi cover use at two
types, width subtyping, open-row access, a raw JS value, and signature
inclusion over Tpoly fields; the object_literal_for_poly_field fixture
pins that a monomorphic literal cannot satisfy a polymorphic field
(universal-variable escape), whose disappearance would signal an
unsoundness.
Magics: cmi Caml1999I029, cmt Caml1999T030.
Stage E of the object-representation cleanup (#8584).
Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
Saving previously duplicated a field's mutability class only when the row terminator was generic, sharing the resolved source cell otherwise. That left one lifetime exception to the copy invariant: a structure-generalized source (non-generic terminator, share policy) and its saved copy (generic terminator after for_saving relevels, duplicate policy) could own one class with incompatible row-copy classifications - copying both in a single session would then pick a policy depending on which owner was reached first. copy_type_desc gains a fresh_mutability flag and Subst.typexp_rec passes it for for_saving, so a saved graph now owns fresh value cells unconditionally: it shares no mutability class with its source, contains no link chains, and duplication through the session memo still preserves equivalence-class sharing within the saved graph. Ordinary copying alone decides sharing from row-terminator genericity. In passing, the Tlink arm of copy_type_desc now forwards keep_names (and the new flag) instead of silently dropping them on recursion. New pins: for_saving_copy_order_is_irrelevant builds the previously hazardous fixture (source and saved copy with different terminator classifications) and copies both in one session in both orders; for_saving_fresh_copy_preserves_internal_aliasing checks freshness keeps intra-graph sharing; the closed-row saving test now also asserts the saved cell is not the source's. The q1_ test-name prefixes are dropped along with the retracted universal terminator-sharing claim they referred to. Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
Formatting-only: the rebase onto master (which upgraded OCamlFormat from 0.27 to 0.29 in #8591) left the files this branch touches formatted with the old version. Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
Four review findings on #8597, all confirmed by probe or inspection: Variance ignored field mutability. The old phantom "x#=" setter member was an arrow whose contravariant occurrence incidentally made settable fields invariant; removing the phantom left compute_variance treating every field payload with the ambient variance, so an explicitly covariant parameter could annotate a settable field and leak write capability through an abstract type. The Tfield arm now sends a Mutable field's payload through Variance.full, like a mutable record label; Immutable fields keep the ambient variance, preserving read-only covariance. Pinned by object_settable_field_covariant_param (Bad_variance). Writes instantiated polymorphic field schemes. Assigning to {@set "id": 'a. 'a => 'a} typed the value at one instance, so a monomorphic function satisfied the field while reads kept instantiating the unchanged scheme. A field's type is a scheme: reading eliminates it, writing must establish it. The write path now uses the checker's scheme-introduction discipline - fixed instantiation, typing at that instance, check_univars - extracted as type_object_field_value next to its record twin type_label_exp, returning the value at an ordinary instance as both siblings do. type_label_exp's PR#4862 retry is a label-specific completeness recovery and is deliberately not replicated; the helper's comment records that. Pinned by object_write_poly_field_less_general (Less_general) and a positive settable-poly case in object_poly_field. Along the way, instance_poly's positional boolean becomes ~fixed with a contract comment in ctype.mli: the flag controls fixed copying of polymorphic-variant rows; scheme introduction is identified by the whole operation, not by this flag. reanalyze missed Texp_object_literal. Side-effect analysis fell through to the permissive default, so a dead binding whose object literal called effectful code was classified as removable; it now checks every field expression (ObjectLiteralSideEffects deadcode case). Termination analysis crashed on the wildcard; it now compiles the literal as an ordered sequence of its fields - ordered, not unordered, because fields evaluate in source order and crediting a later field's progress past a non-returning earlier field would be unsound (the testObjectLiteralRecursionFirst case is now reported as a possible infinite loop while testObjectLiteralProgressFirst passes). Texp_object_get/Texp_object_set traverse their receiver and value instead of asserting (testObjectAccess). Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
The private/open-object manifest path in includecore compared paired
fields by type only, so a signature could declare
type t = private {..@set "x": int} over an implementation whose field was
not settable - and since access follows the published row, clients could
write straight through the abstraction. In the phantom-setter encoding
this could not happen structurally: granting required an interface
"x#=" member with no implementation partner (rejected by the missing-
field check), while forgetting was the interface simply omitting the
member (absorbed by the ignored implementation-side misses). Stage D
turned the capability into a flag on the field, and nothing had taken
over the width mechanism's job.
The pairing now requires a settable implementation field wherever the
interface field is settable; an implementation's settable field may
still be abstracted to a read-only one. Probe-verified equivalent to the
released (phantom-encoding) compiler in all directions, including that
paired field types remain compared by equality - private rows allow
width and capability forgetting, never depth subtyping.
The @set inclusion matrix is now pinned per comparison arm, since the
flag participates in several independently-changeable relations:
object_private_row_grants_set (the new includecore rule),
object_manifest_set_mismatch (transparent manifests are equations -
eqtype), object_value_signature_set_mismatch (value signatures claim
instances - moregeneral), and the legal forgetting direction compiles in
object_mutability_pin.res.
Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
cristianoc
force-pushed
the
object-stage0
branch
from
August 30, 2026 11:16
29c907b to
a7826ab
Compare
cknitt
approved these changes
Aug 30, 2026
cknitt
left a comment
cknitt
left a comment
Member
There was a problem hiding this comment.
Great cleanup and soundness improvement! 👍
Tested against a large project, no issues.
cristianoc
added a commit
that referenced
this pull request
Aug 31, 2026
Four review findings on #8597, all confirmed by probe or inspection: Variance ignored field mutability. The old phantom "x#=" setter member was an arrow whose contravariant occurrence incidentally made settable fields invariant; removing the phantom left compute_variance treating every field payload with the ambient variance, so an explicitly covariant parameter could annotate a settable field and leak write capability through an abstract type. The Tfield arm now sends a Mutable field's payload through Variance.full, like a mutable record label; Immutable fields keep the ambient variance, preserving read-only covariance. Pinned by object_settable_field_covariant_param (Bad_variance). Writes instantiated polymorphic field schemes. Assigning to {@set "id": 'a. 'a => 'a} typed the value at one instance, so a monomorphic function satisfied the field while reads kept instantiating the unchanged scheme. A field's type is a scheme: reading eliminates it, writing must establish it. The write path now uses the checker's scheme-introduction discipline - fixed instantiation, typing at that instance, check_univars - extracted as type_object_field_value next to its record twin type_label_exp, returning the value at an ordinary instance as both siblings do. type_label_exp's PR#4862 retry is a label-specific completeness recovery and is deliberately not replicated; the helper's comment records that. Pinned by object_write_poly_field_less_general (Less_general) and a positive settable-poly case in object_poly_field. Along the way, instance_poly's positional boolean becomes ~fixed with a contract comment in ctype.mli: the flag controls fixed copying of polymorphic-variant rows; scheme introduction is identified by the whole operation, not by this flag. reanalyze missed Texp_object_literal. Side-effect analysis fell through to the permissive default, so a dead binding whose object literal called effectful code was classified as removable; it now checks every field expression (ObjectLiteralSideEffects deadcode case). Termination analysis crashed on the wildcard; it now compiles the literal as an ordered sequence of its fields - ordered, not unordered, because fields evaluate in source order and crediting a later field's progress past a non-returning earlier field would be unsound (the testObjectLiteralRecursionFirst case is now reported as a possible infinite loop while testObjectLiteralProgressFirst passes). Texp_object_get/Texp_object_set traverse their receiver and value instead of asserting (testObjectAccess). Signed-Off-By: Cristiano Calcagno <ccrisccris@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw
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
2 participants
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.
Implements the object-representation cleanup designed in #8584, as a linear sequence of self-contained commits (pin tests first, then stages 0/A/B/C/D/E and a persistence-copy correction).
What changes for users
obj["x"] = vrequires the field to be settable —@set, or an inferred open row that the write makes settable; a coercion never grants or widens write capability. This fixes the unsoundness where the getter type and the hidden mangled"x#="setter member were tracked independently (a property could be written at a different type than it was read, and a value coerced to a type without@setcould still be written through).@get(bare or withnull/undefined/nullablepayload) and@set({no_get: ...}). Only bare@setmarks a field settable; nullable getter types are written directly.@set; missing-property errors name the field instead of a phantom"x#="member.What changes internally
Tfieldchains; the dead class machinery is gone: the class-abbreviation memo onTobject, method-send typing, and (Stage E) the entirefield_kindpresence lattice, whoseFabsentstate was proven unreachable.Pexp_object_get/set,Texp_object_get/set,Pjs_object_get/set), shared between Lambda and Lam (Stage B); object literals are typed directly, eliminating the%objextension/record_as_js_objectmachinery (Stage C). The frozen parsetree v0 bridge maps the new nodes back to the legacy encodings for PPX compatibility.field_kindmold: unification merges classes by linking representatives, promotion is trail-logged and backtrackable, copying follows the row variable's sharing law, coercion enlargement allocates independent cells, and saving emits fresh link-free cells. Copy-session state (the inheritedTsubstrestoration plus the new cell memos) is an explicit nestable, exception-safe session stack inBtype.Typeslayout changes.Testing
Every commit passed the full battery plus the syntax, analysis, and gentype suites at the time it was made, and the rebased head repeats that: full
make test,make test-syntax,make test-analysis,make test-gentype,make checkformatall green. Behavior is pinned by a representation-level ounit suite (18 tests: class merging and order-independence, backtracking, the two copy-sharing regimes, copy-session nesting and failure paths, saving and a marshal round trip), new super_errors fixtures for the soundness flips, and runtime tests including previously uncovered polymorphic object fields. Generated JavaScript is byte-identical across the test corpus except the intended soundness flips.Closes the implementation checklist of #8584.
🤖 Generated with Claude Code
https://claude.ai/code/session_01PCtQiaDijUqA2fujQXvKUw