Skip to content

codegen: two sibling native-instance registration forms are still name-keyed and module-wide (follow-up to #9847) #9858

Description

@proggeramlug

Follow-up to #9847 / #9857, which fixed one of three registration forms in
lower_assign that key a native-instance tag on the identifier text and
scope it to the whole module. The other two, and one fallback, were left alone
deliberately — #9847's A/B does not exercise them, and widening the change would
have put behaviour outside the evidence. This issue records that decision so the
remainder is documented rather than a loose end.

What #9857 fixed, and what it left

#9857 changed the bare-assignment form X = <native module>.<method>(...) to
key on the LocalId the target resolves to. Whole-bundle class-bearing
NativeMethodCall nodes in cli_2.1.112.js went 410 → 98
(child_process::Instance 194 → 0, fs::Instance 40 → 0, path::Instance
92 → 15), with the 70 named-class nodes unchanged as a registered control.

28 catch-all ::Instance sites survive. They split cleanly:

surviving site count assessment
stream::on 4 plausibly a genuine stream handle
https::on 3 plausibly genuine
https::destroy 3 plausibly genuine
stream::pipe, stream::once 2 plausibly genuine
https::write, https::end 2 plausibly genuine
subtotal — look correct 14
path::startsWith 7 wrong
path::slice 3 wrong
path::replace 2 wrong
path::toLowerCase, path::split, path::match 3 wrong
subtotal — still mis-typed 14

path has no instances at all, and every one of those 14 is a
String.prototype method. They are the same defect #9847 described, reaching
the binding through a route #9857 did not change.

The three remaining routes

All in crates/perry-hir/src/lower/expr_assign.rs, in the same
lower_assign prologue:

  1. x = new NativeClass(...) — registers via both
    register_native_instance (scoped, name-keyed) and
    push_module_native_instance (module-wide, name-keyed). The scoped half
    limits the damage, but the module-wide half has the identical homonym
    collision.
  2. x = y propagation — when y is a known native instance, the tag is
    copied to x with push_module_native_instance, module-wide by name. This
    one can also launder a tag past a scope boundary.
  3. The unresolvable-name fallback in fix(hir): scope a bare-assignment native-instance tag to the binding, not the name (#9847) #9857 itself — when the assignment
    target resolves to no local (a bare global, or a module-level binding
    referenced from a body lowered before that binding was pre-registered), the
    tag is still registered and resolved by spelling. This is the documented hole
    fix(hir): scope a bare-assignment native-instance tag to the binding, not the name (#9847) #9857 kept, mirroring the one miscompile: an uncalled function containing new Proxy(arr, {}) makes an unrelated function's indexed read loop run zero iterations #7775 kept for proxies; it is strictly no worse
    than the previous behaviour, but it is not nothing.

I have not determined which of the three produces the 14 path sites.

The obvious shape, untested

The same LocalId treatment #9857 applied — resolve the assignment target with
ctx.lookup_local and register through register_local_id_native_instance,
keeping the name-keyed path only for a target that resolves to no binding. Form
1 would additionally want its register_native_instance half examined rather
than assumed. This is a hypothesis, not a plan: unlike #9847 it has no A/B
behind it yet, and the path/stream/https sites should be traced to their
producing route before anything is changed.

How to measure it

perry compile --no-auto-optimize --trace hir <bundle>, then count
NativeMethodCall { module: "M", class_name: Some("C"), … } occurrences. The
HIR dump is emitted before codegen, so the compile can be stopped as soon as
codegen: <file> appears on stderr — roughly 3 minutes rather than a full
build. Keep the named-class subtotal (70 on this bundle) as a control: a fix to
these routes must not move it.

Note on the catch-all

lower_assign's class-name match ends _ => Some("Instance"), so any method
on any recognised native module tags its target. Narrowing that is a separate
question from scoping — it changes which calls dispatch natively — and is not
proposed here, but it is why a single mis-scoped registration reaches so far.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions