From cfbaa6c89d8971c76c0db975e5ea3de02dd0fe32 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Sun, 2 Aug 2026 07:16:49 -0700 Subject: [PATCH] feat(dsl): movement modifiers on the existing runtime actions (p8-s3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `drive()` plus modifiers is the idiomatic DSL scenario — the standard's own style guide says so (§9.1.8) — and until now the generic actions lowered to nothing because there was nothing to shape them with. Now the modifiers are what give them content. Every §8.9 modifier lands on an IR action P2/P5 already implements, or is reported: speed targets absolute and relative, placements and gaps, lane targets and offsets. Nothing modifier-shaped is added to the runtime, which is the same rule ADR-0031 applied to composition. §8.9.1.1.1's `at` anchor decides *how*. Absent, `all` and `start` set the value when the phase begins — a Step. `end` reaches it over the phase, which is what makes p8-s2's durations load-bearing: without a concrete duration there is no interval to spread the change over, and lowering reports rather than inventing one. `position` is the modifier where the anchor changes the kind of action, not just its shape: a TeleportAction placement at the start, a LongitudinalDistanceAction gap over the phase. `keep_speed()` and `keep_position()` lower to no action at all. The runtime already holds speed and relative position between actions, so one that set the current value would be a no-op that still occupies the action domain (§7.5) and could supersede a running action. `keep_lane()` does produce one, because holding a lane is active work. `acceleration`, `along`, `along_trajectory` and `distance` are reported with their reasons and moved to Post in the coverage matrix: there is no acceleration-target action in the IR, a concrete route or trajectory can only come from §8.12.2's map methods (which the standard says may be external, §7.3.7.4), and a distance-bounded phase is not something time-based sequencing expresses. Files #110 rather than absorbing it: §7.3.3 says an overloaded enum literal resolves by "the type requirements of the place it is used in", but Scena reports the ambiguity first — so the specification's own `at: start` is rejected while `at: at!start` works. It blocks nothing and predates this sprint. 1392 gtest (+14), 194 pytest, ADR-0032. --- .../ADR-0032-dsl-movement-modifiers.md | 130 +++++++ docs/roadmap/coverage/osc-dsl-coverage.md | 18 +- frontends/dsl/README.md | 12 + frontends/dsl/src/lower.cpp | 339 +++++++++++++++++- frontends/dsl/tests/dsl_lowering_test.cpp | 271 +++++++++++++- 5 files changed, 751 insertions(+), 19 deletions(-) create mode 100644 docs/architecture/ADR-0032-dsl-movement-modifiers.md diff --git a/docs/architecture/ADR-0032-dsl-movement-modifiers.md b/docs/architecture/ADR-0032-dsl-movement-modifiers.md new file mode 100644 index 0000000..85f75c3 --- /dev/null +++ b/docs/architecture/ADR-0032-dsl-movement-modifiers.md @@ -0,0 +1,130 @@ + + +# ADR-0032 — Lowering the §8.9 movement modifiers + +- **Status:** Accepted +- **Date:** 2026-08-02 +- **Sprint:** p8-s3 (#46) +- **Supersedes:** nothing. Builds on ADR-0030 (lowering to the IR) and ADR-0031 + (the composition operators). + +## Context + +The DSL's generic actions — `drive()`, `move()`, `walk()` — carry no target of +their own. ADR-0030 lowered them to nothing and said why: they exist to be +*shaped* by §8.9's modifiers, and until those lower there is nothing to shape. +The standard's own style guide is explicit that this is the idiomatic form +("prefer using generic actions", §9.1.8), so nearly every scenario in the +specification is `drive()` plus modifiers. + +A modifier is an equality constraint on the invoked behavior's parameters +(§7.3.12.4), and §8.9 gives seventeen of them plus four parameters common to +all. The question is what each one *denotes* in a runtime that was built for +OpenSCENARIO XML. + +## Decision + +### 1. Every modifier lands on an action the runtime already has + +No modifier machinery is added to the runtime. Each one lowers to an IR action +P2 or P5 already implements — a `SpeedAction`, a `TeleportAction`, a +`LaneChangeAction`, a `LaneOffsetAction`, a `LongitudinalDistanceAction`, +a `LateralDistanceAction` — or it is reported. That is the same rule ADR-0031 +applied to composition, and it is what keeps one runtime under two frontends +rather than two runtimes that resemble each other. + +| §8.9 modifier | IR action | +|---|---| +| `speed` (absolute) | `SpeedAction` | +| `speed` (`faster_than`/`slower_than`/`same_as`, `factor`) | `SpeedAction` with a `RelativeTargetSpeed` | +| `change_speed` | `SpeedAction` relative to the actor's own speed | +| `position` at the start | `TeleportAction` to a `RelativeObjectPosition` | +| `position` over the phase | `LongitudinalDistanceAction` | +| `lateral` (with `side_of`) | `LateralDistanceAction` | +| `lateral` (without) | `LaneOffsetAction` | +| `lane` (number) | `LaneChangeAction` to an absolute lane | +| `lane` (`side_of` + `side`) | `LaneChangeAction` to a relative lane | +| `change_lane` | `LaneChangeAction` relative to the actor | +| `keep_lane` | continuous `LaneOffsetAction` at zero | + +### 2. The `at` anchor decides *how*, and the phase duration is what it needs + +§8.9.1.1.1's anchor takes `start`, `end` or `all`, and it is optional. + +- **absent or `all`** — the value holds for the invocation. Realised by setting + it when the phase begins, which is a Step transition. +- **`start`** — the same thing, said explicitly. +- **`end`** — the value must be *reached* by the end of the phase, so it becomes + a transition spread over the phase's length. + +That last case is why p8-s2's durations are load-bearing here: without a +concrete duration there is no interval to spread the change over. Rather than +invent one, lowering reports and sets the value at once — a fabricated duration +would put a number in the trace the scenario never stated. + +`position` is the modifier where the anchor changes the *kind* of action, not +just its shape: at the start it is a placement (`TeleportAction`), over the +phase it is a gap to reach and hold (`LongitudinalDistanceAction`). Both +readings are in §8.9.2; the anchor is what chooses between them. + +### 3. "Keep doing what you are doing" lowers to nothing + +`keep_speed` (§8.9.6) and `keep_position` (§8.9.3) constrain the actor not to +change. The runtime already holds an entity's speed and its relative position +between actions, so the faithful lowering is *no action*: one that set the +current value would be a no-op that nonetheless appears in the trace and +competes for the same action domain (§7.5). + +`keep_lane` (§8.9.16) is different and does produce an action, because holding a +lane is active work — a continuous `LaneOffsetAction` at zero is exactly the +runtime's way of saying it. + +### 4. What is reported, and why + +- **`acceleration` (§8.9.7)** shapes the acceleration of a movement the phase is + already performing. There is no acceleration-target action in the IR — the + coverage matrix defers §8.8's for the same reason — and on its own the + modifier states a rate with nothing to apply it to. +- **`along` and `along_trajectory` (§8.9.11–.12)** need a concrete route or + trajectory *value*. The DSL has no struct constructor (§7.2.2.6.7), so one can + only come from §8.12.2's `map.create_route(...)`, and the standard itself says + map methods may be external implementations (§7.3.7.4) — post-v0.0.1. +- **`distance` (§8.9.13)** bounds a phase by distance travelled rather than by + time, and ADR-0031 sequences phases by time. +- **`yaw`, `orientation`, `physical_movement`, `avoid_collisions`** were already + Post in the coverage matrix and stay there. + +Each is reported by name with its section, never silently dropped. + +## Consequences + +- The idiomatic DSL scenario — `drive()` with modifiers — now produces a + runnable IR, which is what GS-12 and GS-13 will be written in. +- Modifier lowering reads the phase's duration, so it sits after ADR-0031's + time arithmetic and depends on it. That ordering is now a fact of the code. +- **A pre-existing checker gap became visible and was filed as #110.** §7.3.3 + says an overloaded enum literal "will depend on the type requirements of the + place it is used in"; Scena reports the ambiguity before consulting them. `at` + (§8.9.19) and `route_overlap_kind` (§8.12) both declare `start` and `end`, so + the specification's own `at: start` is rejected while `at: at!start` is + accepted. It blocks nothing — the qualified spelling works — but a conforming + scenario copied out of the standard does not check, so it is tracked rather + than absorbed. + +## Alternatives considered + +**Give the runtime a modifier concept.** Rejected: modifiers are a DSL surface, +and a runtime that knew about them would be machinery only one frontend could +reach. + +**Default an `at: end` modifier to some duration.** Rejected: it puts a number +in the trace the scenario never stated, which is the same failure mode ADR-0030 +rejects for performance limits and ADR-0031 for `one_of` selection. + +**Lower `keep_speed` to a speed action holding the current value.** Rejected: it +is a no-op that would still occupy the longitudinal action domain (§7.5) and +could supersede a running action — a behaviour change dressed up as a +constraint. diff --git a/docs/roadmap/coverage/osc-dsl-coverage.md b/docs/roadmap/coverage/osc-dsl-coverage.md index 6caad84..3c8e334 100644 --- a/docs/roadmap/coverage/osc-dsl-coverage.md +++ b/docs/roadmap/coverage/osc-dsl-coverage.md @@ -212,13 +212,17 @@ envelopes belong to the logical level). | Modifier | Section | Exec | Sprint | Notes | |---|---|---|---|---| -| position() / keep_position() | §8.9.2–.3 | In | p8-s3 | ahead_of/behind + distance forms; `*_range`/at_point-with-projection variants Post | -| speed() / change_speed() / keep_speed() | §8.9.4–.6 | In | p8-s3 | Absolute + faster_than/slower_than/same_as relative forms | -| acceleration() | §8.9.7 | In | p8-s3 | Scalar form | -| lateral() | §8.9.8 | In | p8-s3 | t-axis offset; measure_by default | -| lane() / change_lane() / keep_lane() | §8.9.14–.16 | In | p8-s3 | `change_lane` without an explicit side is diagnosed, not randomized (determinism) | -| along() / along_trajectory() | §8.9.11–.12 | In | p8-s3 | Route/path/trajectory binding | -| distance() | §8.9.13 | In | p8-s3 | Traveled-distance bound | +| position() | §8.9.2 | In | p8-s3 | **Landed** (`dsl_lowering_test.cpp`): `ahead_of`/`behind` with a concrete distance. The anchor changes the *kind* of action — at the start a `TeleportAction` placement, over the phase a `LongitudinalDistanceAction` gap (ADR-0032). Point forms and `*_range` Post | +| keep_position() | §8.9.3 | In | p8-s3 | **Landed**: lowers to *no* action. The runtime already holds relative position between actions, so one that set the current value would be a no-op that still occupies the action domain (§7.5) | +| speed() / change_speed() | §8.9.4–.5 | In | p8-s3 | **Landed**: absolute, and the `faster_than`/`slower_than`/`same_as` relative forms with `factor` → `RelativeTargetSpeed`. `change_speed` is relative to the actor's own speed. Ranges are reported | +| keep_speed() | §8.9.6 | In | p8-s3 | **Landed**: lowers to no action, same reason as `keep_position` | +| acceleration() | §8.9.7 | Post | — | Reported: it shapes an acceleration the phase is already performing, and the IR has no acceleration-target action — the same reason §8.8's are deferred | +| lateral() | §8.9.8 | In | p8-s3 | **Landed**: `side_of` → `LateralDistanceAction`, otherwise a `LaneOffsetAction` from the lane centre. Positive offsets are to the left (§7.4.1.4), so `side: right` is negative | +| lane() / change_lane() / keep_lane() | §8.9.14–.16 | In | p8-s3 | **Landed**: a lane number → an absolute lane target, `side_of`+`side` → a relative one, `keep_lane` → a continuous zero `LaneOffsetAction`. `change_lane` without an explicit side is diagnosed, not chosen (determinism) | +| along() / along_trajectory() | §8.9.11–.12 | Post | — | Reported: both need a concrete route or trajectory value, and the DSL has no struct constructor (§7.2.2.6.7) — one can only come from §8.12.2's map methods, which the standard says may be external (§7.3.7.4) | +| distance() | §8.9.13 | Post | — | Reported: it bounds a phase by distance travelled, and phase sequencing is by time (ADR-0031) | +| `at` phase anchoring | §8.9.1.1.1, §8.9.19 | In | p8-s3 | **Landed**: absent/`all`/`start` set the value when the phase begins (a Step); `end` reaches it over the phase, which needs the phase length p8-s2's duration fixes. Without one it is reported, never invented | +| Overloaded enum literal resolved by context | §7.3.3 | **Gap** | — | §7.3.3 says the literal "will depend on the type requirements of the place it is used in"; Scena reports the ambiguity first, so the standard's own `at: start` is rejected while `at: at!start` works. Tracked as #110; blocks nothing | | yaw() / orientation() | §8.9.9–.10 | Post | — | Orientation-target modifiers deferred (teleport orientation covers placements) | | physical_movement() | §8.9.17 | Post | — | Single documented default profile in v0.0.1 | | avoid_collisions() | §8.9.18 | Post | — | No collision-avoidance controller in v0.0.1 (engine executes what the scenario says; Collision condition detects) | diff --git a/frontends/dsl/README.md b/frontends/dsl/README.md index 7462ec8..94f81b1 100644 --- a/frontends/dsl/README.md +++ b/frontends/dsl/README.md @@ -260,6 +260,18 @@ expected. It also keeps the dependency list unchanged. by label from `LowerOptions::alternative`, defaulting to the first: the engine has no seed machinery, and a hidden input is what determinism forbids. `wait elapsed(d)` lowers to nothing but the offset. +- **Every §8.9 modifier lands on an action the runtime already has** (ADR-0032). + `drive()` plus modifiers is the idiomatic scenario, and the modifiers are what + give it content: speed targets (absolute and relative), placements and gaps, + lane targets and offsets. Nothing modifier-shaped exists in the runtime. +- **The `at` anchor decides how, and needs the phase's length.** Absent, `all` + and `start` set the value when the phase begins; `end` reaches it over the + phase, which is what makes p8-s2's durations load-bearing. Without a duration + it is reported, never invented. `position` is the one where the anchor changes + the *kind* of action: a placement at the start, a gap to hold over the phase. +- **`keep_speed()` and `keep_position()` lower to nothing, on purpose.** The + runtime already holds both between actions, so an action setting the current + value would be a no-op that still occupies the action domain (§7.5). - **`..` has to beat a float that starts with a dot.** §7.2.2.6.7 spells the range constructor `[a '..' b]` while §7.2.1.5.2 makes a float's leading digits optional, so `[2..4]` is a race the operator must win. It did not until p8-s2; diff --git a/frontends/dsl/src/lower.cpp b/frontends/dsl/src/lower.cpp index fe363ad..5defe63 100644 --- a/frontends/dsl/src/lower.cpp +++ b/frontends/dsl/src/lower.cpp @@ -609,12 +609,337 @@ struct Behaviors { return true; } - /// Reports the parts of an invocation that p8-s2 and p8-s3 will lower. - void report_deferred(const DoMember& member) { - for (const ModifierApplication& modifier : member.with.modifiers) { - warn(modifier.range, - "movement modifier '" + modifier.name + "' is lowered in p8-s3 (#46), §8.9"); + // --- §8.9 movement modifiers ------------------------------------------- + + /// §8.9.19's phase anchor: where in the phase the modifier's value holds. + /// + /// The standard leaves `at` optional. An unstated anchor means the value + /// holds for the invocation rather than at one edge of it, which is `all` — + /// and both `all` and `start` are realised the same way, by setting the + /// value when the phase begins. Only `end` needs the value *reached* over + /// the phase, which is what makes a duration load-bearing. + enum class Anchor { Start, End, All }; + + [[nodiscard]] Anchor anchor_of(const ArgumentMap& arguments) { + const std::string written = argument_enum(program, arguments, "at"); + if (written == "end") { + return Anchor::End; + } + if (written == "start") { + return Anchor::Start; + } + return Anchor::All; + } + + /// How a value anchored at `at` is reached over a phase of `span` seconds. + /// + /// `start`/`all` is a Step: the value is simply true from the phase's first + /// evaluation. `end` is a transition over the phase, which needs a phase + /// whose length is a constant — without one there is no interval to spread + /// the change over, and inventing a duration would put a number in the + /// trace the scenario never stated. + [[nodiscard]] ir::TransitionDynamics anchored_dynamics(Anchor at, + const std::optional& span, + const SourceRange& range, + const std::string& modifier) { + ir::TransitionDynamics dynamics; + if (at != Anchor::End) { + return dynamics; // Step, the default + } + if (!span.has_value() || *span <= 0.0) { + warn(range, "'" + modifier + + "' is anchored at the end of a phase whose length no duration fixes, " + "so the value is set at once instead (§8.9.1.1.1)"); + return dynamics; + } + dynamics.shape = ir::DynamicsShape::Linear; + dynamics.dimension = ir::DynamicsDimension::Time; + dynamics.value = *span; + return dynamics; + } + + /// The §8.9 modifiers applied to one invocation, as IR actions. + /// + /// A modifier is an equality constraint on the invoked behavior's + /// parameters (§7.3.12.4), and the generic actions exist to carry them — so + /// this is where `drive() with: speed(...)` becomes something the runtime + /// can execute. Nothing new appears in the runtime: every modifier lands on + /// an action P2/P5 already implements, or is reported. + void modifier_actions(const DoMember& member, const std::string& actor, TypeId actor_type, + const std::optional& span, + std::vector>& out) { + for (const ModifierApplication& application : member.with.modifiers) { + const TypeId modifier = lookup_modifier(actor_type, application.name); + if (modifier == kInvalidType) { + continue; // the resolver already said why + } + const ArgumentMap arguments = + bind_arguments(program, modifier, application.arguments, context); + lower_modifier(application, program.types[modifier].simple_name, arguments, actor, span, + out); + } + } + + /// The modifier a name resolves to: through the receiver's chain for an + /// actor-associated one (§7.3.12.2), or plainly for the three §8.9 names + /// the standard had to leave unassociated. + [[nodiscard]] TypeId lookup_modifier(TypeId receiver, const std::string& name) const { + for (TypeId current = receiver; current != kInvalidType; + current = program.types[current].base) { + const auto found = program.types_by_name.find(program.types[current].name + "." + name); + if (found != program.types_by_name.end() && + program.types[found->second].kind == TypeKind::Modifier) { + return found->second; + } + } + const auto plain = program.types_by_name.find("std::" + name); + if (plain != program.types_by_name.end() && + program.types[plain->second].kind == TypeKind::Modifier) { + return plain->second; } + return kInvalidType; + } + + void lower_modifier(const ModifierApplication& application, const std::string& name, + const ArgumentMap& arguments, const std::string& actor, + const std::optional& span, + std::vector>& out) { + const Anchor at = anchor_of(arguments); + const SourceRange& range = application.range; + + if (name == "movable_object.speed") { + lower_speed_modifier(arguments, actor, at, span, range, out); + return; + } + if (name == "change_speed") { + // §8.9.5 changes the speed *by* an amount, which is a relative + // target against the actor's own speed (§RelativeTargetSpeed). + const std::optional delta = argument_number(arguments, "speed"); + if (!delta.has_value()) { + warn(range, "'change_speed' needs a concrete 'speed' to change by (§8.9.5)"); + return; + } + ir::RelativeTargetSpeed target; + target.entity_ref = actor; + target.value = *delta; + out.push_back(std::make_shared( + actor, target, anchored_dynamics(at, span, range, name))); + return; + } + if (name == "keep_speed" || name == "movable_object.keep_position") { + // Both say "do not change what you are doing". The runtime already + // holds an entity's speed and relative position between actions, so + // the faithful lowering is no action at all — an action that set + // the current value would be a no-op with a trace of its own. + return; + } + if (name == "vehicle.keep_lane") { + // §8.9.16 holds the lane, which is the lane offset the runtime + // keeps when a continuous LaneOffsetAction targets zero. + out.push_back(std::make_shared( + actor, ir::AbsoluteTargetLaneOffset{0.0}, /*continuous=*/true, + ir::DynamicsShape::Step)); + return; + } + if (name == "movable_object.position") { + lower_position_modifier(arguments, actor, at, range, out); + return; + } + if (name == "movable_object.lateral") { + lower_lateral_modifier(arguments, actor, span, at, range, out); + return; + } + if (name == "vehicle.lane") { + lower_lane_modifier(arguments, actor, span, at, range, out); + return; + } + if (name == "change_lane") { + const std::string side = argument_enum(program, arguments, "side"); + const std::optional count = argument_number(arguments, "lane"); + if (side != "left" && side != "right") { + error(range, "'change_lane' needs an explicit 'side' of left or right (§8.9.15); " + "an unstated side would have to be chosen"); + return; + } + ir::RelativeTargetLane target; + target.entity_ref = actor; + const int lanes = count.has_value() ? static_cast(*count) : 1; + target.value = side == "left" ? lanes : -lanes; + out.push_back(std::make_shared( + actor, target, anchored_dynamics(at, span, range, name))); + return; + } + if (name == "movable_object.acceleration") { + // §8.9.7 constrains the acceleration of the movement the phase is + // already performing. There is no acceleration-target action in the + // IR (the coverage matrix defers §8.8's), and on its own the + // modifier states a rate with nothing to apply it to. + warn(range, "'acceleration' shapes a speed change rather than standing alone, and the " + "runtime has no acceleration-target action in v0.0.1 (§8.9.7)"); + return; + } + if (name == "movable_object.along" || name == "movable_object.along_trajectory") { + // Both need a concrete route or trajectory *value*. The DSL has no + // struct constructor (§7.2.2.6.7), so one can only come from + // §8.12.2's `map.create_route(...)` — a method the standard itself + // says may be an external implementation (§7.3.7.4), which is + // post-v0.0.1. + warn(range, "'" + application.name + + "' needs a concrete route or trajectory, which only §8.12.2's map " + "methods produce and those are external (§7.3.7.4)"); + return; + } + if (name == "movable_object.distance") { + // §8.9.13 bounds the distance travelled during the phase. That is a + // phase *boundary*, not an action, and the composition machinery + // sequences phases by time (ADR-0031). + warn(range, "'distance' bounds a phase by distance travelled rather than by time, " + "which p8-s2's sequencing does not express (§8.9.13)"); + return; + } + warn(range, "movement modifier '" + application.name + + "' has no runtime counterpart in v0.0.1 (§8.9); see the DSL coverage " + "matrix"); + } + + void lower_speed_modifier(const ArgumentMap& arguments, const std::string& actor, Anchor at, + const std::optional& span, const SourceRange& range, + std::vector>& out) { + const ir::TransitionDynamics dynamics = anchored_dynamics(at, span, range, "speed"); + // §8.9.4's relative forms name another actor; `factor` scales its speed + // and a bare `speed` offsets it (§RelativeTargetSpeed's two value types). + for (const char* relation : {"faster_than", "slower_than", "same_as"}) { + const std::string reference = argument_reference(arguments, relation); + if (reference.empty()) { + continue; + } + if (entities.count(reference) == 0) { + error(range, "'speed' needs '" + std::string(relation) + + "' to name a §8.7 participant of this scenario (§8.9.4)"); + return; + } + ir::RelativeTargetSpeed target; + target.entity_ref = reference; + if (const std::optional factor = argument_number(arguments, "factor"); + factor.has_value()) { + target.value_type = ir::SpeedTargetValueType::Factor; + target.value = *factor; + } else { + const std::optional offset = argument_number(arguments, "speed"); + target.value = offset.value_or(0.0); + if (std::string(relation) == "slower_than") { + target.value = -target.value; + } + } + out.push_back(std::make_shared(actor, target, dynamics)); + return; + } + const std::optional speed = argument_number(arguments, "speed"); + if (!speed.has_value()) { + warn(range, "'speed' fixes no single value here, so nothing is lowered; a range " + "constrains accepted traces rather than fixing a speed (§8.9.4)"); + return; + } + out.push_back(std::make_shared(actor, *speed, dynamics)); + } + + void lower_position_modifier(const ArgumentMap& arguments, const std::string& actor, Anchor at, + const SourceRange& range, + std::vector>& out) { + // §8.9.2 places the actor relative to another one. `ahead_of` puts the + // actor ahead of the reference, `behind` behind it. + std::string reference = argument_reference(arguments, "ahead_of"); + bool ahead = true; + if (reference.empty()) { + reference = argument_reference(arguments, "behind"); + ahead = false; + } + if (reference.empty() || entities.count(reference) == 0) { + warn(range, "'position' is lowered in its 'ahead_of'/'behind' form; the point forms " + "need a concrete position_3d declaration (§8.9.2)"); + return; + } + const std::optional distance = argument_number(arguments, "distance"); + if (!distance.has_value()) { + warn(range, "'position' needs a concrete 'distance' (§8.9.2); a time gap or a range " + "is not lowered"); + return; + } + if (at == Anchor::Start) { + // A placement: the actor *is* there when the phase begins. + ir::RelativeObjectPosition placement; + placement.entity_ref = reference; + placement.dx = ahead ? *distance : -*distance; + out.push_back(std::make_shared(actor, ir::Position{placement})); + return; + } + // A gap to reach and, for `all`, to hold. + out.push_back(std::make_shared( + actor, reference, *distance, std::nullopt, /*freespace=*/true, + /*continuous=*/at == Anchor::All, ir::CoordinateSystem::Entity, + ahead ? ir::LongitudinalDisplacement::LeadingReferencedEntity + : ir::LongitudinalDisplacement::TrailingReferencedEntity)); + } + + void lower_lateral_modifier(const ArgumentMap& arguments, const std::string& actor, + const std::optional& span, Anchor at, + const SourceRange& range, + std::vector>& out) { + const std::optional distance = argument_number(arguments, "distance"); + if (!distance.has_value()) { + warn(range, "'lateral' needs a concrete 'distance' (§8.9.8)"); + return; + } + // §8.9.8's `side` is which side of the reference the actor is on; + // positive lateral offsets are to the left (§7.4.1.4). + const std::string side = argument_enum(program, arguments, "side"); + const double offset = side == "right" ? -*distance : *distance; + const std::string reference = argument_reference(arguments, "side_of"); + if (!reference.empty() && entities.count(reference) != 0) { + out.push_back(std::make_shared( + actor, reference, *distance, /*freespace=*/true, + /*continuous=*/at == Anchor::All)); + return; + } + (void)span; + out.push_back(std::make_shared( + actor, ir::AbsoluteTargetLaneOffset{offset}, /*continuous=*/at == Anchor::All, + at == Anchor::End ? ir::DynamicsShape::Linear : ir::DynamicsShape::Step)); + } + + void lower_lane_modifier(const ArgumentMap& arguments, const std::string& actor, + const std::optional& span, Anchor at, const SourceRange& range, + std::vector>& out) { + const std::string reference = argument_reference(arguments, "side_of"); + const std::string side = argument_enum(program, arguments, "side"); + if (!reference.empty() && entities.count(reference) != 0) { + if (side != "left" && side != "right") { + error(range, "'lane' with 'side_of' needs an explicit 'side' of left or right " + "(§8.9.14)"); + return; + } + ir::RelativeTargetLane target; + target.entity_ref = reference; + target.value = side == "left" ? 1 : -1; + out.push_back(std::make_shared( + actor, target, anchored_dynamics(at, span, range, "lane"))); + return; + } + const std::optional lane = argument_number(arguments, "lane"); + if (!lane.has_value()) { + warn(range, "'lane' needs a concrete lane number or a 'side_of' participant (§8.9.14)"); + return; + } + // §8.9.14 counts lanes by road-network identity, which only a road + // backend can resolve — an absolute lane id, exactly as XML spells it. + ir::AbsoluteTargetLane target; + target.value = std::to_string(static_cast(*lane)); + out.push_back(std::make_shared( + actor, target, anchored_dynamics(at, span, range, "lane"))); + } + + /// Reports the parts of an invocation a later sprint will lower. + void report_deferred(const DoMember& member) { for (const EventSpec& until : member.with.until) { (void)until; // §7.6.2.5.4 ends the invocation *exactly* at an event, and the @@ -926,6 +1251,10 @@ struct Behaviors { "does not bound an absolute time (§7.6.2.4.1)"); } } + // The modifiers come after the action's own contribution, because they + // *tune* it — and they need the phase's length, which the duration just + // established (§8.9.1.1.1's `at: end`). + modifier_actions(member, actor, actor_type, duration, actions); if (actions.empty()) { // Nothing to run — a generic action, or one already reported. The // phase still takes its duration, because the actor is still doing diff --git a/frontends/dsl/tests/dsl_lowering_test.cpp b/frontends/dsl/tests/dsl_lowering_test.cpp index 7d9c097..8ec281e 100644 --- a/frontends/dsl/tests/dsl_lowering_test.cpp +++ b/frontends/dsl/tests/dsl_lowering_test.cpp @@ -828,19 +828,276 @@ TEST(DslLoweringTest, ANonDefaultParallelOverlapIsReported) { EXPECT_TRUE(sink_says(result.sink, "overlap 'equal'")); } -TEST(DslLoweringTest, ModifiersAreReportedAsDeferred) { - // A modifier is a real construct of the invocation that belongs to a later - // sprint. Reporting beats silently running a scenario that says more than - // the engine was told. +// --- §8.9 movement modifiers ------------------------------------------------ + +// §8.9.19's `at` and §8.12's `route_overlap_kind` both declare `start` and +// `end`, so the anchors are written qualified here. §7.3.3 says an overloaded +// literal should resolve by the type the place expects, which Scena does not do +// yet — #110. The qualified spelling is the one that works today. + +TEST(DslLoweringTest, ASpeedModifierAtTheStartSetsTheSpeedAtOnce) { + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " speed(speed: 36kph, at: at!start)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const std::vector lowered = phases(result.lowered.scenario); + ASSERT_EQ(lowered.size(), 1U); + const scena::ir::SpeedAction* action = only_action(lowered[0]); + ASSERT_NE(action, nullptr); + // 36 kph folded once, at check time, through §8.14.1.3's printed factor. + EXPECT_NEAR(action->target_speed(), 10.0, 1e-6); + EXPECT_EQ(action->dynamics().shape, scena::ir::DynamicsShape::Step); +} + +TEST(DslLoweringTest, ASpeedModifierAtTheEndIsReachedOverThePhase) { + // This is what makes p8-s2's durations load-bearing: `at: end` needs an + // interval to spread the change over, and the duration is that interval. + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive(duration: 4s) with:\n" + " speed(speed: 0kph, at: at!start)\n" + " speed(speed: 36kph, at: at!end)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const std::vector lowered = phases(result.lowered.scenario); + ASSERT_EQ(lowered.size(), 1U); + ASSERT_EQ(lowered[0].event->actions.size(), 2U); + const auto* ramp = + dynamic_cast(lowered[0].event->actions[1].get()); + ASSERT_NE(ramp, nullptr); + EXPECT_EQ(ramp->dynamics().shape, scena::ir::DynamicsShape::Linear); + EXPECT_EQ(ramp->dynamics().dimension, scena::ir::DynamicsDimension::Time); + EXPECT_EQ(ramp->dynamics().value, 4.0); +} + +TEST(DslLoweringTest, AnEndAnchorWithoutADurationIsReportedNotInvented) { + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " speed(speed: 36kph, at: at!end)\n"), + result); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + EXPECT_TRUE(sink_says(result.sink, "no duration fixes")); +} + +TEST(DslLoweringTest, ARelativeSpeedModifierNamesTheOtherActor) { + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " lead: vehicle\n" + " do a: ego.drive() with:\n" + " speed(speed: 5mps, faster_than: lead)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const scena::ir::SpeedAction* action = + only_action(phases(result.lowered.scenario)[0]); + ASSERT_NE(action, nullptr); + ASSERT_TRUE(action->is_relative()); + EXPECT_EQ(action->relative_target()->entity_ref, "lead"); + EXPECT_EQ(action->relative_target()->value, 5.0); + EXPECT_EQ(action->relative_target()->value_type, scena::ir::SpeedTargetValueType::Delta); +} + +TEST(DslLoweringTest, SlowerThanIsTheSameTargetWithTheSignTurnedRound) { + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " lead: vehicle\n" + " do a: ego.drive() with:\n" + " speed(speed: 5mps, slower_than: lead)\n"), + result); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const scena::ir::SpeedAction* action = + only_action(phases(result.lowered.scenario)[0]); + ASSERT_NE(action, nullptr); + ASSERT_TRUE(action->is_relative()); + EXPECT_EQ(action->relative_target()->value, -5.0); +} + +TEST(DslLoweringTest, ChangeSpeedIsRelativeToTheActorsOwnSpeed) { + // §8.9.5 changes the speed *by* an amount, which is what a relative target + // against the actor itself says. + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " change_speed(speed: 2mps)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const scena::ir::SpeedAction* action = + only_action(phases(result.lowered.scenario)[0]); + ASSERT_NE(action, nullptr); + ASSERT_TRUE(action->is_relative()); + EXPECT_EQ(action->relative_target()->entity_ref, "ego"); + EXPECT_EQ(action->relative_target()->value, 2.0); +} + +TEST(DslLoweringTest, APositionModifierPlacesAtTheStartAndKeepsAGapAfterwards) { + // The same modifier says two different things depending on its anchor: at + // the start it is a placement, over the phase it is a gap to hold. + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " lead: vehicle\n" + " do a: ego.drive() with:\n" + " position(distance: 20m, behind: lead, at: at!start)\n" + " position(distance: 10m, behind: lead, at: at!all)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const std::vector lowered = phases(result.lowered.scenario); + ASSERT_EQ(lowered.size(), 1U); + ASSERT_EQ(lowered[0].event->actions.size(), 2U); + const auto* placement = + dynamic_cast(lowered[0].event->actions[0].get()); + ASSERT_NE(placement, nullptr); + const auto* relative = std::get_if(&placement->position()); + ASSERT_NE(relative, nullptr); + EXPECT_EQ(relative->entity_ref, "lead"); + EXPECT_EQ(relative->dx, -20.0); + const auto* gap = dynamic_cast( + lowered[0].event->actions[1].get()); + ASSERT_NE(gap, nullptr); + EXPECT_EQ(gap->entity_ref(), "lead"); + ASSERT_TRUE(gap->distance().has_value()); + EXPECT_EQ(*gap->distance(), 10.0); + EXPECT_TRUE(gap->continuous()); + EXPECT_EQ(gap->displacement(), scena::ir::LongitudinalDisplacement::TrailingReferencedEntity); +} + +TEST(DslLoweringTest, ALaneModifierTargetsALaneOfTheRoadNetwork) { + // §8.9.14 counts lanes by road-network identity, which is exactly what an + // absolute lane target is on the XML side. + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " lane(lane: 2)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const auto* action = + only_action(phases(result.lowered.scenario)[0]); + ASSERT_NE(action, nullptr); + ASSERT_FALSE(action->is_relative()); + EXPECT_EQ(action->absolute_target()->value, "2"); +} + +TEST(DslLoweringTest, ChangeLaneModifierCountsLanesFromTheActor) { + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " change_lane(lane: 2, side: side_left_right!left)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const auto* action = + only_action(phases(result.lowered.scenario)[0]); + ASSERT_NE(action, nullptr); + ASSERT_TRUE(action->is_relative()); + EXPECT_EQ(action->relative_target()->entity_ref, "ego"); + EXPECT_EQ(action->relative_target()->value, 2); +} + +TEST(DslLoweringTest, ChangeLaneModifierNeedsASide) { + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " change_lane(lane: 1)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + EXPECT_EQ(result.status, Status::ValidationError); + EXPECT_TRUE(sink_says(result.sink, "explicit 'side'")); +} + +TEST(DslLoweringTest, KeepLaneHoldsTheLaneOffsetContinuously) { + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " keep_lane()\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const auto* action = + only_action(phases(result.lowered.scenario)[0]); + ASSERT_NE(action, nullptr); + EXPECT_TRUE(action->continuous()); +} + +TEST(DslLoweringTest, ALateralModifierOffsetsFromTheLaneCentre) { + Lowered result; + run(std::string(kPrelude).append( + "scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " lateral(distance: 1m, side: side_left_right!right)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + const auto* action = + only_action(phases(result.lowered.scenario)[0]); + ASSERT_NE(action, nullptr); + // Positive lane offsets are to the left (§7.4.1.4), so `right` is negative. + ASSERT_FALSE(action->is_relative()); + EXPECT_EQ(action->absolute_target()->value, -1.0); +} + +TEST(DslLoweringTest, KeepSpeedAndKeepPositionLowerToNothingOnPurpose) { + // Both say "do not change what you are doing", and the runtime already + // holds speed and relative position between actions — so an action that set + // the current value would be a no-op with a trace of its own. + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " keep_speed()\n" + " keep_position()\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + EXPECT_TRUE(result.lowered.scenario.storyboard.stories.empty()); + EXPECT_TRUE(result.sink.diagnostics().empty()); +} + +TEST(DslLoweringTest, AModifierWithNoRuntimeCounterpartIsReported) { + Lowered result; + run(std::string(kPrelude).append("scenario go:\n" + " ego: vehicle\n" + " do a: ego.drive() with:\n" + " acceleration(acceleration: 2.0)\n" + " distance(distance: 100m)\n"), + result); + ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); + ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); + EXPECT_TRUE(sink_says(result.sink, "acceleration-target action")); + EXPECT_TRUE(sink_says(result.sink, "distance travelled")); +} + +TEST(DslLoweringTest, ARouteModifierSaysWhyItCannotBeMadeConcrete) { + // §8.12.2's map methods are what produce a route, and the standard says + // they may be external implementations (§7.3.7.4) — post-v0.0.1. Lowered result; run(std::string(kPrelude).append("scenario go:\n" " ego: vehicle\n" - " do phase: ego.drive() with:\n" - " speed(speed: 30kph)\n"), + " r: route\n" + " do a: ego.drive() with:\n" + " along(route: r)\n"), result); ASSERT_EQ(result.check_status, Status::Ok) << first_message(result.check_sink); ASSERT_EQ(result.status, Status::Ok) << first_message(result.sink); - EXPECT_TRUE(sink_says(result.sink, "p8-s3 (#46)")); + EXPECT_TRUE(sink_says(result.sink, "external")); } // --- §8.5.4 the map file ------------------------------------------------------