fix(dsl): resolve modifier applications through the receiver (#100)

docs/dev/stdlib-worksheets/08-09-movement-modifiers.md

@@ -109,13 +109,12 @@ Association per §7.3.12.3 — see note 1. `+4` marks the four common parameters
    not decoration, it is what makes the chapter declarable.
 4. **§8.9.14's `lane` counts lanes as `uint`, §8.9.15's `change_lane` as
    `int`.** Both carried as printed.
-5. **Applying any of these is issue #100**, in both of §7.3.12.4.1's positions:
-   as a scenario member, an actor-associated modifier is not found by simple
-   name; inside a `with:` block, modifier applications are not validated at all
-   (a nonsense name is accepted silently). The declarations here are
-   well-formed and are pinned; nothing pins application, because neither
-   current behaviour is one to keep. p8-s3 (#46) needs #100 fixed first, and
-   §8.9 is the surface that fix should be tested against.
+5. **Applying any of these was issue #100 when this was written — since
+   fixed**, in both of §7.3.12.4.1's positions: as a scenario member, an
+   actor-associated modifier was not found by simple name; inside a `with:`
+   block, applications were not validated at all. §8.9 was the surface the fix
+   was tested against, as this note asked, and both positions are now pinned in
+   `dsl_stdlib_test.cpp`. p8-s3 (#46) is unblocked.
 6. **§8.9.1.4's scalar/range pairs are two separate fields.** `speed` and
    `speed_range`, `distance` and `distance_range`, `angle` and `angle_range`:
    "at most one of them is used within an invocation", which is a constraint on

docs/dev/stdlib-worksheets/08-12-02-map.md

@@ -94,7 +94,7 @@ scenario or an action (43b finding).
    list argument.** The declared prototype has three parameters and prints no
    defaults, so the library declares three. A caller supplying one argument is a
    caller-side concern (§7.2.2.5.2 positional arguments), not a library one.
-6. **These twelve modifiers cannot yet be applied — issue #100.** Writing
+6. **These twelve modifiers could not be applied when this was written — issue #100, since fixed.** Writing
    `my_map.lane_side(a, side_left_right!left, b, 1, ls)` reports "unknown
    modifier 'lane_side'". The cause is in the resolver, not in the library: the
    parser keeps the whole qualified behavior name, so the declaration

docs/roadmap/coverage/osc-dsl-coverage.md

@@ -123,11 +123,11 @@ included, so a conforming implementation and Scena agree.
 | `stdtypes` — string methods | §8.13 | In | p7-s5 | **Landed**: declared on the `string` primitive. §8.13 heads them under the types sub-module and then names the `std` namespace; declared under the section they appear in, and nothing observable turns on it — a method on a primitive is reached through a value, never through a namespace |
 | `std` — physical-object actors, structs and enums | §8.7 | In | p7-s5 | **Landed** (`dsl_stdlib_test.cpp`): the `osc_actor` → `physical_object` → `movable_object` → `traffic_participant` → `vehicle` → `trailer` chain with `stationary_object`, `person` and `animal`; `bounding_box`, `axle`, `hitch_receiver`, `hitch_coupler`; all 11 enums including the backward-compatibility spellings (`truck = heavy_truck`, `fire = fire_brigade`, …) sharing their replacement's value; the §8.7.6.1 measurement methods. §8.7.26 (traffic-participant groups) is non-normative and excluded |
 | `std` — road abstraction classes | §8.12.3–§8.12.41 | In | p7-s5 | **Landed** (`dsl_stdlib_test.cpp`): the `route` → `route_element` hierarchy with `road`, `lane_section`, `lane`, `crossing`, the four point structs, `path`/`trajectory` and their relative variants, `compound_route`/`compound_lane`, `junction`, and 15 enums. The road-dependent `physical_object` and `traffic_participant` methods arrive here as `extend` blocks, which is how the standard prints their prototypes |
-| `std` — the `map` actor | §8.12.2 | In | p7-s5 | **Landed** (`dsl_stdlib_test.cpp`): the top-level road-network actor with its six fields, its 18 conversion and creation methods, and its 12 search-space modifiers. *Applying* an actor-associated modifier is issue #100 — the parser keeps the actor prefix in the declared name, so the application site cannot find it; the declarations are well-formed and are pinned. Worksheet: `docs/dev/stdlib-worksheets/08-12-02-map.md` |
+| `std` — the `map` actor | §8.12.2 | In | p7-s5 | **Landed** (`dsl_stdlib_test.cpp`): the top-level road-network actor with its six fields, its 18 conversion and creation methods, and its 12 search-space modifiers. Applying them works as of the #100 fix: the modifier name lives in the actor scope (§7.3.12.2) and is reached through the receiver. Worksheet: `docs/dev/stdlib-worksheets/08-12-02-map.md` |
 | `std` — action hierarchy and the environment | §8.8.1, §8.10, §8.11 | In | p7-s5 | **Landed** (`dsl_stdlib_test.cpp`): `osc_action` with `action_for_environment` and `action_for_movable_object`; the `environment` actor with `local_to_unix_time`; the `weather`/`air`/`precipitation`/`wind`/`fog`/`clouds`/`celestial_light_source` structs; the seven §8.11 environment actions. Their *execution* stays Post (see the actor table) — the DSL environment actions are checked, not run, in v0.0.1 |
 | `std` — traffic lights | §8.15 | In | p7-s5 | **Landed** (`dsl_stdlib_test.cpp`): `traffic_light_bulb`, `traffic_light` and `traffic_light_group` with their state methods, `traffic_light_stop_line` (a `route_element`), `traffic_light_phase`/`traffic_light_cycle`, the `traffic_light_controller` actor and its seven §8.15.9 actions, and 5 enums. Their *execution* stays Post (see the actor table). Worksheet: `docs/dev/stdlib-worksheets/08-15-traffic-lights.md` |
 | `std` — movement actions | §8.8.2–§8.8.4 | In | p7-s5 | **Landed** (`dsl_stdlib_test.cpp`): 15 actions for `movable_object`, 13 for `vehicle` and `walk` for `person`, plus `dynamic_profile`, `lane_change_side`, `gap_direction` and `headway_direction`. `action_for_vehicle` and `action_for_person` are declared here — §8.8.3/§8.8.4 name them as parents but only §8.8.1's prose defines them. Their *execution* stays Post except where the roadmap's action table says otherwise. Worksheet: `docs/dev/stdlib-worksheets/08-08-movement-actions.md` |
-| `std` — movement modifiers | §8.9 | In | p7-s5 | **Landed** (`dsl_stdlib_test.cpp`): the `any_shape`/`common_*_shape` hierarchy, the seven §8.9.19–§8.9.25 enums, and 17 modifiers each carrying §8.9.1.1's four common parameters (`at`, `movement_mode`, `track`, `shape`). Fourteen are actor-associated per §7.3.12.3; `change_speed`, `keep_speed` and `change_lane` are unassociated because §8.8 already declares actions of those names on those actors and a qualified behavior name identifies one declaration — the collision is the standard's. *Applying* a modifier is issue #100 in both of §7.3.12.4.1's positions. Worksheet: `docs/dev/stdlib-worksheets/08-09-movement-modifiers.md` |
+| `std` — movement modifiers | §8.9 | In | p7-s5 | **Landed** (`dsl_stdlib_test.cpp`): the `any_shape`/`common_*_shape` hierarchy, the seven §8.9.19–§8.9.25 enums, and 17 modifiers each carrying §8.9.1.1's four common parameters (`at`, `movement_mode`, `track`, `shape`). Fourteen are actor-associated per §7.3.12.3; `change_speed`, `keep_speed` and `change_lane` are unassociated because §8.8 already declares actions of those names on those actors and a qualified behavior name identifies one declaration — the collision is the standard's. Applying them works as of the #100 fix, in both of §7.3.12.4.1's positions. Worksheet: `docs/dev/stdlib-worksheets/08-09-movement-modifiers.md` |
 
 Coverage of the individual §8 declarations: all physical types and units
 (§8.14.1), compound structs (§8.14.2), string methods (§8.13), all actors

frontends/dsl/README.md

@@ -174,11 +174,17 @@ expected. It also keeps the dependency list unchanged.
 - **An actor's modifier uses the prefixed form.** §8.7.4.1.1 writes
   `stationary_object.location()`, which is `modifier stationary_object.location`
   — not `modifier location of stationary_object`, because §7.3.12.2's `of`
-  names a scenario or an action. Note that such a modifier cannot yet be
-  *applied* — issue #100: the parser keeps the actor prefix in the declared
-  name, so `check_modifier_application` looks up a name no scope holds.
-  Unassociated modifiers are unaffected, and a modifier application inside a
-  `with:` block is not checked at all — same issue.
+  names a scenario or an action.
+- **An actor-associated modifier is reached through its receiver, not through
+  the namespace.** §7.3.12.2 puts its name "in the actor scope", so
+  `modifier vehicle.keep_lane` interns under the actor's qualified name and
+  ordinary lookup will never find `keep_lane` on its own. Application
+  (§7.3.12.4.1) resolves the actor expression's type and walks its inheritance
+  chain, probing the name table once per step — an exact lookup, not a scan.
+  With the actor omitted the receiver is what the site already implies: the
+  enclosing declaration in a member position, the invoked behavior's actor
+  inside a `with:` block. This was issue #100; before it, such a modifier could
+  not be applied at all, and a `with:` block accepted any name whatsoever.
 - **Association is what makes §8.9 declarable.** §7.3.12.3's own example is
   `modifier vehicle.keep_lane()`, annotated as being §8.9.16's, so the movement
   modifiers are actor-associated. That is not decoration: an unassociated

frontends/dsl/src/resolve.cpp

@@ -229,8 +229,10 @@ class Resolver {
     void add_field(TypeId id, const PendingStructured& pending, const Field& field);
     void add_method(TypeId id, const PendingStructured& pending, const MethodDecl& method);
     void add_event(TypeId id, const PendingStructured& pending, const EventDecl& event);
-    void check_modifier_application(TypeId id, const PendingStructured& pending,
-                                    const ModifierApplication& application);
+    void check_modifier_application(const ModifierApplication& application,
+                                    const ExpressionContext& context, TypeId implicit_receiver);
+    [[nodiscard]] TypeId lookup_modifier_on(TypeId receiver, const std::string& name) const;
+    [[nodiscard]] TypeId find_modifier_named(const std::string& name) const;
     void check_literal_default(const FieldInfo& info, const Expr& value);
     [[nodiscard]] bool resolve_declarator(const TypeRef& declarator, const Scope& scope,
                                           TypeId& type, TypeId& element);
@@ -1064,25 +1066,105 @@ void Resolver::add_event(TypeId id, const PendingStructured& pending, const Even
     type.events.emplace(event.name, std::move(info));
 }
 
-void Resolver::check_modifier_application(TypeId container, const PendingStructured& pending,
-                                          const ModifierApplication& application) {
-    const TypeId id = lookup_declared(application.name, pending.scope);
+/// Finds a modifier associated with `receiver` or one of its supertypes.
+///
+/// §7.3.12.2 puts an actor-associated modifier's name "in the actor scope", and
+/// the declaration `modifier vehicle.keep_lane` interns it under the qualified
+/// name `<the actor's qualified name>.keep_lane`. So the lookup is an exact map
+/// probe per step of the inheritance chain — no scan, and deterministic.
+TypeId Resolver::lookup_modifier_on(TypeId receiver, const std::string& name) const {
+    for (TypeId current = receiver; current != kInvalidType;) {
+        const auto found = out_.types_by_name.find(out_.types[current].name + "." + name);
+        if (found != out_.types_by_name.end() &&
+            out_.types[found->second].kind == TypeKind::Modifier) {
+            return found->second;
+        }
+        current = out_.types[current].base;
+    }
+    return kInvalidType;
+}
+
+/// The first actor-associated modifier whose bare name is `name`.
+///
+/// Only reached after lookup has already failed, to turn "unknown modifier"
+/// into "belongs to another actor" — so an O(n) walk of the name table is the
+/// right cost, and iterating a std::map keeps it deterministic.
+TypeId Resolver::find_modifier_named(const std::string& name) const {
+    for (const auto& [qualified, id] : out_.types_by_name) {
+        (void)qualified;
+        const TypeInfo& type = out_.types[id];
+        if (type.kind != TypeKind::Modifier || type.actor_type == kInvalidType) {
+            continue;
+        }
+        const std::size_t dot = type.simple_name.rfind('.');
+        if (dot != std::string::npos && type.simple_name.substr(dot + 1) == name) {
+            return id;
+        }
+    }
+    return kInvalidType;
+}
+
+void Resolver::check_modifier_application(const ModifierApplication& application,
+                                          const ExpressionContext& context,
+                                          TypeId implicit_receiver) {
+    // §7.3.12.4.1: `[<actor-expression>.]<modifier-name>(...)`. A written actor
+    // expression names the receiver; omitting it means the receiver is the one
+    // the application site already implies — the enclosing declaration or the
+    // actor of the invocation the `with:` block belongs to.
+    TypeId receiver = implicit_receiver;
+    if (application.actor != nullptr) {
+        // May grow Program::types (a list or range constructor interns a type),
+        // so nothing may hold a TypeInfo& across it.
+        receiver = check_expression(*application.actor, context);
+        if (receiver == kInvalidType) {
+            return; // the actor expression already reported why
+        }
+    }
+
+    const Scope scope{context.name_space, context.uses, context.file};
+
+    // An unassociated or scenario-associated modifier is named plainly
+    // (§7.3.12.4.2's first example); an actor-associated one lives in the actor
+    // scope and is only reachable through the receiver. A written actor
+    // expression means the second kind, so ordinary lookup is skipped.
+    TypeId plain = kInvalidType;
+    if (application.actor == nullptr) {
+        plain = lookup_declared(application.name, scope);
+    }
+    TypeId id = (plain != kInvalidType && out_.types[plain].kind == TypeKind::Modifier)
+                    ? plain
+                    : kInvalidType;
+    if (id == kInvalidType && receiver != kInvalidType) {
+        id = lookup_modifier_on(receiver, application.name);
+    }
+
     if (id == kInvalidType) {
+        // Nothing applies. Say what is actually wrong rather than "unknown",
+        // because at library scale the interesting case is a name that exists
+        // and is the wrong thing.
+        if (plain != kInvalidType) {
+            error(application.range, "'" + application.name + "' is " +
+                                         std::string(spelling_of(out_.types[plain].kind)) +
+                                         ", not a modifier (§7.3.12.4)");
+            return;
+        }
+        const TypeId elsewhere = find_modifier_named(application.name);
+        if (elsewhere != kInvalidType && receiver != kInvalidType) {
+            error(application.range,
+                  "modifier '" + application.name + "' belongs to '" + out_.types[elsewhere].actor +
+                      "' and cannot be applied to '" + out_.types[receiver].name + "' (§7.3.12.4)");
+            return;
+        }
         error(application.range, "unknown modifier '" + application.name + "' (§7.3.12.4)");
         return;
     }
+
     const TypeInfo& modifier = out_.types[id];
-    if (modifier.kind != TypeKind::Modifier) {
-        error(application.range, "'" + application.name + "' is " +
-                                     std::string(spelling_of(modifier.kind)) +
-                                     ", not a modifier (§7.3.12.4)");
-        return;
-    }
     if (modifier.modifies_type != kInvalidType) {
         // §7.3.12.2: a scenario-associated modifier "can be applied only to an
         // invocation of the specified scenario or as a member of that
         // scenario".
-        if (!out_.is_derived_from(container, modifier.modifies_type)) {
+        if (!out_.is_derived_from(context.self, modifier.modifies_type)) {
             error(application.range, "modifier '" + modifier.simple_name + "' belongs to '" +
                                          modifier.modifies +
                                          "' and cannot be applied here (§7.3.12.2)");
@@ -1133,7 +1215,8 @@ void Resolver::add_members(const PendingStructured& pending) {
             add_event(pending.id, pending, member.event);
             break;
         case Member::Kind::ModifierApplication:
-            check_modifier_application(pending.id, pending, member.modifier);
+            // Checked in pass 4 instead: resolving the receiver means typing
+            // the actor expression, which needs an ExpressionContext.
             break;
         case Member::Kind::Behavior: {
             ++behaviors;
@@ -1450,8 +1533,11 @@ void Resolver::check_do_member(const DoMember& member, const ExpressionContext&
             (void)check_expression(*argument.value, context);
         }
     }
+    // The invoked behavior's actor is also the receiver a `with:`-block modifier
+    // application falls back to when it omits one (§7.3.12.4.1).
+    TypeId invoked_on = out_.types[context.self].actor_type;
     if (member.actor != nullptr) {
-        (void)check_expression(*member.actor, context);
+        invoked_on = check_expression(*member.actor, context);
     }
     if (member.kind == DoMemberKind::Wait) {
         check_event_spec(member.event, context);
@@ -1465,6 +1551,10 @@ void Resolver::check_do_member(const DoMember& member, const ExpressionContext&
                 (void)check_expression(*argument.value, context);
             }
         }
+        // Until this landed, a `with:` block accepted any name at all — and it
+        // is where the domain model expects nearly every movement modifier to
+        // be applied (#100).
+        check_modifier_application(application, context, invoked_on);
     }
     for (const EventSpec& until : member.with.until) {
         check_event_spec(until, context);
@@ -1577,6 +1667,14 @@ void Resolver::check_expressions() {
                         (void)check_expression(*argument.value, context);
                     }
                 }
+                // §7.3.12.4.1's member position. With the actor omitted the
+                // receiver is the declaration itself — an actor applying its
+                // own modifier (§7.3.12.4.2's third example) — or, for a
+                // behavior, the actor it is declared on.
+                check_modifier_application(member.modifier, context,
+                                           out_.types[pending.id].actor_type == kInvalidType
+                                               ? pending.id
+                                               : out_.types[pending.id].actor_type);
                 break;
             case Member::Kind::Behavior:
                 if (member.behavior != nullptr) {