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

@@ -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
+0 failures). Items 4 (drop) and 7 (defdata) are still being written. Item 6 —
-held.
+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
+The tag is read in exactly four places in flan_dyn.c: ~class-of~ answers it;
-compares it, so two instances of one class compare by their slots and an
+~dyn_equal~ compares it, so two instances of one class compare by their slots
-instance is never equal to a plain map with the same entries (Clojure's
+and an instance is never equal to a plain map with the same entries
-answer for a record beside a map); and both renderers print it —
+(Clojure's answer for a record beside a map); and *both* renderers write it —
-~#point{ :x 1 :y 2}~, which is Clojure's own spelling for a record.
+~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~
+The tag is built from the class's *qualified* name, and the qualifier is the
-classes are two classes and their instances are never equal.
+**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

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
+   ...)] under [(defgeneric area [self] dyn)] — and a method whose names
-   binds nothing, so the common case adds no [let] at all. *)
+   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 =
+  let pairs = List.combine m.Ast.mfn.Ast.params g.gfn.Ast.params in
-    List.filter_map
+  if
+    List.for_all
       (fun ((mp : Ast.field), (gp : Ast.field)) ->
-         if String.equal mp.Ast.fname gp.Ast.fname then None
+         String.equal mp.Ast.fname gp.Ast.fname)
-         else
+      pairs
-           Some { Ast.bname = mp.Ast.fname; bty = None;
+  then ex loc (Ast.Do m.Ast.mfn.Ast.fbody)
-                  bval = ex mp.Ast.floc (Ast.Var gp.Ast.fname);
+  else begin
-                  bloc = mp.Ast.floc })
+    let tmp i = Printf.sprintf "~arg%d" i in
-      (List.combine m.Ast.mfn.Ast.params g.gfn.Ast.params)
+    let hold =
-  in
+      List.mapi
-  if bs = [] then ex loc (Ast.Do m.Ast.mfn.Ast.fbody)
+        (fun i ((_ : Ast.field), (gp : Ast.field)) ->
-  else ex loc (Ast.Let (bs, m.Ast.mfn.Ast.fbody))
+           { 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,

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);

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.

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\
+       something else\nsomething else\n2\n:circle\n\
-       nil\n100000\n:point\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"

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\