From ac7608f94b096e2ab1809b40e3da5448a7d3d15c Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Sun, 20 Sep 2026 15:36:02 +0700 Subject: [PATCH] A method's parameter names are bound in parallel, not in sequence A let binds in sequence, so binding a method's names pairwise from the generic's reads a name it has just bound. A generic [a b] with a method [b a] -- a swap, which is what renaming parameters most often is -- was handed its first argument twice and could not reach its second at all; [b c] is the same bug one step shorter. Every argument is now copied into a temp in the unspellable ~ namespace first and every method name bound from a temp, uniformly rather than only for the pairs that collide, because a rule that fires on the tangled case alone is one nobody exercises. Both shapes are in dyn-class.flan, where the values are what is wrong rather than the types, and across all three rows. With it, two things the descriptor fix left behind. descriptors_asm wrote the descriptors into .rodata and a descriptor holds the address of its own offset table, so every one of them was a relocation in a read-only section -- a DT_TEXTREL, which ld warns about in a PIE and refuses in a shared object, and which was warning in the new daemon case's own output. They go in .data.rel.ro now, in both the executable and the reload module; readelf -d on a reload module from each backend shows no TEXTREL. And FIX.org: the stale held line for item 6, the fourth read site of the shape tag (say_render, not just print), the warning that a class's qualifier is the importer's alias so a hand-written :a/point is coupled to one import's name, and the gap flagged for the next sweep -- marking through a descriptor an x86 reload module emitted is still unexercised. --- FIX.org | 60 ++++++++++++++++++++++++++++++------ lib/classes.ml | 60 ++++++++++++++++++++++++++++-------- lib/emit.ml | 10 +++++- test/programs/dyn-class.flan | 17 ++++++++++ test/test_acceptance.ml | 10 ++++-- test/test_flan.ml | 13 ++++++++ 6 files changed, 144 insertions(+), 26 deletions(-) diff --git a/FIX.org b/FIX.org index 7c8d20f..ed2d19c 100644 --- a/FIX.org +++ b/FIX.org @@ -185,8 +185,9 @@ against 985ms on LLVM. * Landed on dev-loop Items 1, 2, 3 and 5 are merged and green (dune test --force, 232 elisp checks, -0 failures). Items 4 (drop) and 7 (defdata) are still being written. Item 6 is -held. +0 failures). Items 4 (drop) and 7 (defdata) are still being written. Item 6 — +classes and generic functions — is no longer held: it landed, and the M2 queue +above records it under item 6 with its commits. ** Re-run is merged, and does not work under --x86 Park and re-run live in the merged entry point's main(), and --x86 refuses the @@ -1435,14 +1436,25 @@ before and after — checked, not assumed. It needs no marking either: an interned keyword entry is immortal by construction and is not a collector object, which ~mark_value~ states by following ~BOX_OBJ~ and nothing else. -The tag is visible in exactly three places: ~class-of~ answers it; equality -compares it, so two instances of one class compare by their slots and an -instance is never equal to a plain map with the same entries (Clojure's -answer for a record beside a map); and both renderers print it — -~#point{ :x 1 :y 2}~, which is Clojure's own spelling for a record. +The tag is read in exactly four places in flan_dyn.c: ~class-of~ answers it; +~dyn_equal~ compares it, so two instances of one class compare by their slots +and an instance is never equal to a plain map with the same entries +(Clojure's answer for a record beside a map); and *both* renderers write it — +~render~, which is what ~print~ goes through, and ~say_render~, the 96-byte +sentence a trap prints, so a dyn trap naming an instance says which class it +was. The spelling is ~#point{ :x 1 :y 2}~, Clojure's own for a record. -The tag is built from the *qualified* class name, so two packages' ~point~ -classes are two classes and their instances are never equal. +The tag is built from the class's *qualified* name, and the qualifier is the +**importer's alias** rather than anything the defining package chose — the +same class imported as ~a~ and as ~zz~ tags its instances ~:a/point~ and +~:zz/point~. That falls straight out of [Load]'s rename, and it is right for +the dispatch, which resolves the class name through the same rename and +therefore agrees with it. What it is *not* safe for is a hand-written +dispatch value: ~(defmethod g :a/point ...)~ is a keyword and nobody +qualifies it, so it is coupled to one import's alias and silently answers for +nothing under another. Write the class's name, ~(defmethod g point ...)~, +which is renamed with everything else. Two packages' own ~point~ classes are +two classes either way, which was the property wanted. ** How it is built: a pass, not a macro None of the four forms reaches the checker. ~lib/classes.ml~ rewrites the @@ -1468,9 +1480,21 @@ and evaluating a new one an append; no function is emitted under it. Proved end to end against a real daemon (~test_dev.ml~, "a method added to a running program"), not only at the session's report. +A method's own parameter names are bound from the generic's *in parallel*, +through temporaries in the unspellable ~[~]~ namespace. A [let] binds in +sequence, so the pairwise spelling reads a name it has just bound: a method +[[b a]] under a generic [[a b]] would be handed its first argument twice and +the second would be unreachable. Both the swap and the one-step shift are +pinned in the survey program, where the values are what is wrong rather than +the types. + The cost, recorded rather than hidden: *a method is not separately callable and is not a frame of its own*. A break loop under a method shows the -generic. +generic. And the generic's own parameter names stay in scope inside a method +that renamed them, so a body reaching for ~self~ where it declared ~p~ +silently resolves instead of being refused — small, and closing it would mean +giving the dispatcher unspellable parameter names, which is what the +inspector reads. ** Deferred, each with the reason - *Inheritance.* plan.org's own rule is that method specificity and ambiguity @@ -1516,6 +1540,22 @@ generic. compiler-written body could build. It is not a bad read at run time — a descriptor label is local, so ~ld~ refuses the module with an undefined symbol. Fixed with one line beside the same call in the executable path. + + Emitting them turned up the second half: ~descriptors_asm~ wrote them into + ~.rodata~, and a descriptor holds the address of its own offset table. A + relocation in a read-only section is a ~DT_TEXTREL~ — ld warns about it in + a PIE and refuses it in a shared object — so the section is now + ~.data.rel.ro~, which exists for exactly this and is what both the + executable and the reload module use. Verified with ~readelf -d~ on a + reload module from each backend: no ~TEXTREL~, descriptors in + ~.data.rel.ro~. + + *Still unexercised, and for the next sweep rather than this lane:* marking + THROUGH a descriptor that an x86 reload module emitted. What is proved is + that the module links and runs; what is not is a collection happening while + a live instance of a dyn-holding struct sits in a frame of a body that + module delivered. The LLVM path has been exercised since item 2; this one + has existed for a day. - *A dyn value answered by ~eval-expr~ never reaches the reply's ~:value~.* It renders to the program's own stdout, which arrives on a *later* reply's ~:output~ — the dyn-global rows already read one that way and say so, and diff --git a/lib/classes.ml b/lib/classes.ml index f93c384..7afe0c6 100644 --- a/lib/classes.ml +++ b/lib/classes.ml @@ -193,22 +193,56 @@ let key_expr loc (k : Ast.dispatch) : Ast.expr = (* A method's body, with the method's own parameter names bound to the generic's. The names are the method's to choose — [(defmethod area point [p] - ...)] under [(defgeneric area [self] dyn)] — and a name that already agrees - binds nothing, so the common case adds no [let] at all. *) + ...)] under [(defgeneric area [self] dyn)] — and a method whose names + already agree with the generic's binds nothing, so the common case adds no + [let] at all. + + **The rebinding is parallel, and it has to be.** A [let] here binds in + sequence: each binding is in scope for the next one's value. So the + pairwise spelling — [(let [b a a b] ...)] for a generic [[a b]] and a + method [[b a]] — reads the [b] it has just bound and hands the method its + first argument twice, with the second unreachable. That is a swap, and a + swap is exactly what a method renaming its parameters is most likely to be + doing; the non-swap case [[b c]] is the same bug one step shorter, since + [b] reads the binding above it rather than the parameter. Both are what + [rotatef] and Clojure's destructuring do in parallel, and neither language + would read a name it was in the middle of rebinding. + + So every argument is copied into a temp first and every method name is + bound from a temp, never from a parameter. [~] is a delimiter in the + reader, so the temps cannot collide with a method's names whatever they + are, and one uniform shape is written rather than only the pairs that + actually collide — a rule that fires only on the tangled case is a rule + nobody exercises. *) let method_body (g : generic) (m : Ast.methd) : Ast.expr = let loc = m.Ast.mfn.Ast.nloc in - let bs = - List.filter_map + let pairs = List.combine m.Ast.mfn.Ast.params g.gfn.Ast.params in + if + List.for_all (fun ((mp : Ast.field), (gp : Ast.field)) -> - if String.equal mp.Ast.fname gp.Ast.fname then None - else - Some { Ast.bname = mp.Ast.fname; bty = None; - bval = ex mp.Ast.floc (Ast.Var gp.Ast.fname); - bloc = mp.Ast.floc }) - (List.combine m.Ast.mfn.Ast.params g.gfn.Ast.params) - in - if bs = [] then ex loc (Ast.Do m.Ast.mfn.Ast.fbody) - else ex loc (Ast.Let (bs, m.Ast.mfn.Ast.fbody)) + String.equal mp.Ast.fname gp.Ast.fname) + pairs + then ex loc (Ast.Do m.Ast.mfn.Ast.fbody) + else begin + let tmp i = Printf.sprintf "~arg%d" i in + let hold = + List.mapi + (fun i ((_ : Ast.field), (gp : Ast.field)) -> + { Ast.bname = tmp i; bty = None; + bval = ex gp.Ast.floc (Ast.Var gp.Ast.fname); + bloc = gp.Ast.floc }) + pairs + in + let rename = + List.mapi + (fun i ((mp : Ast.field), (_ : Ast.field)) -> + { Ast.bname = mp.Ast.fname; bty = None; + bval = ex mp.Ast.floc (Ast.Var (tmp i)); + bloc = mp.Ast.floc }) + pairs + in + ex loc (Ast.Let (hold @ rename, m.Ast.mfn.Ast.fbody)) + end (* What a generic answers when no method does. Common Lisp signals here and so does this: it is a condition with a handler-case around it, not a trap, diff --git a/lib/emit.ml b/lib/emit.ml index 6f30937..ca4d8a5 100644 --- a/lib/emit.ml +++ b/lib/emit.ml @@ -4036,7 +4036,15 @@ let descriptors_asm m = "\n# The per-type dyn descriptors — runtime/flan_dyn.h's flan_desc: the\n\ # size of one instance, how many dyn words it holds, and where they are.\n\ # Read by the collector through flan_dyn_root_push_desc and by nothing\n\ - # else; no value points at one.\n\t.section\t.rodata\n"; + # else; no value points at one.\n\ + #\n\ + # .data.rel.ro and not .rodata, because a descriptor holds the address\n\ + # of its own offset table. That is a relocation, and a relocation in a\n\ + # read-only section is one the dynamic linker can only apply by making\n\ + # the section writable — a DT_TEXTREL, which ld warns about in a PIE\n\ + # and refuses outright in a shared object. .data.rel.ro is the section\n\ + # for exactly this: relocated at load and read-only from then on.\n\ + \t.section\t.data.rel.ro,\"aw\",@progbits\n"; List.iter (fun (_, (sym, offs, size)) -> Buffer.add_string b (Printf.sprintf "\t.align\t8\n.L%s.offs:\n" sym); diff --git a/test/programs/dyn-class.flan b/test/programs/dyn-class.flan index 06a9543..5bc6ed4 100644 --- a/test/programs/dyn-class.flan +++ b/test/programs/dyn-class.flan @@ -41,6 +41,17 @@ (defgeneric name-of [self] dyn) (defmethod name-of point [p] "a point") +;; A method's parameter names are its own, and the rebinding that gives it +;; them is parallel. [reorder] names them in the generic's order reversed, which +;; a sequential binding would get wrong in the worst possible way -- it would +;; read the name it had just bound and hand the method its first argument +;; twice. [shift] is the same bug one step shorter: [b] there is the +;; generic's second parameter and must not become the first. +(defmulti reorder [a b] dyn (class-of a)) +(defmethod reorder point [b a] [b a]) +(defmulti shift [a b] dyn (class-of a)) +(defmethod shift point [b c] [b c]) + (defn main [] i32 (let [p (point 3 4) c (circle 2)] @@ -93,6 +104,12 @@ (println (len v)) (println (class-of (at v 1)))) + ;; The renamed parameters, in the generic's own order: the first element + ;; of each answer is the first argument. A sequential rebinding would + ;; print the point twice in the first and lose the 99 in the second. + (println (reorder p 99)) + (println (shift p 99)) + ;; The miss. No method and no :else, so the generic signals NoMethod, and ;; handler-case answers the whole form with a value -- the condition ;; carries the generic's name and the dispatch value that found nothing. diff --git a/test/test_acceptance.ml b/test/test_acceptance.ml index 90e930e..bcc7366 100644 --- a/test/test_acceptance.ml +++ b/test/test_acceptance.ml @@ -3588,6 +3588,11 @@ level "1" is not a claim. The 40 and the 12 are class dispatch over two classes at one call site; the four lines under them are arbitrary dispatch over a keyword, a string, a number and the :else fallback. + The two bracketed lines are a method whose parameter names are the + generic's reversed and one whose names are shifted along: both answer + first-argument-first, which is what says the rebinding is parallel — + a sequential one prints the point twice in the first and loses the 99 + in the second. name-of, then nil, are a dispatch miss answered by handler-case: the generic's name out of the condition, then the dispatch value that found nothing. The 100000 at the end is 50000 instances allocated @@ -3596,8 +3601,9 @@ level "1" "#point{ :x 3 :y 4}\n2\n3\n10\ntrue\nfalse\nnil\n:point\n\ :circle\nnil\nnil\nnil\ntrue\nfalse\nfalse\ntrue\n40\n12\n5\n\ a round thing, keyed by a string\nthe one keyed by a number\n\ - something else\nsomething else\n2\n:circle\na point\nname-of\n\ - nil\n100000\n:point\n" + something else\nsomething else\n2\n:circle\n\ + [ #point{ :x 10 :y 4} 99]\n[ #point{ :x 10 :y 4} 99]\n\ + a point\nname-of\nnil\n100000\n:point\n" in outputs "dyn: classes and dispatch" "programs/dyn-class.flan" dyn_class_out; outputs ~opt:"-O0" "dyn: classes and dispatch, -O0" diff --git a/test/test_flan.ml b/test/test_flan.ml index f271d2f..753bc77 100644 --- a/test/test_flan.ml +++ b/test/test_flan.ml @@ -1778,6 +1778,19 @@ let () = (defgeneric area [self] dyn)\n\ (defmethod area point [whatever] (get whatever :x))\n\ (defn main [] i32 0)"; + (* The rebinding that gives a method its own parameter names is parallel. + A [let] binds in sequence, so the pairwise spelling reads a name it has + just bound: these two type-check either way and the values are what is + wrong, which is why dyn-class.flan is where they are really pinned. What + is pinned here is that both shapes are legal at all. *) + accepts "a method may reverse its generic's parameter names" + "(defmulti g [a b] () (class-of a))\n\ + (defmethod g :else [b a] (println b) (println a))\n\ + (defn main [] i32 0)"; + accepts "a method may shift its generic's parameter names along" + "(defmulti g [a b] () (class-of a))\n\ + (defmethod g :else [b c] (println b) (println c))\n\ + (defn main [] i32 0)"; accepts "a method may be written above its generic" "(defclass point [x y])\n\ (defmethod area point [p] (get p :x))\n\