Skip to content

Commit 3ee1bb0

Browse files
committed
fix(validators): scope bound_vars per-transition in arc_validator
Outgoing arc validation previously aggregated incoming-arc variables across ALL transitions in the net, so a t_to_p arc on transition T was allowed to reference variables bound on a different transition T'. CPN semantics scope variable bindings to a single transition's firing. Pre-aggregate `bound_vars_by_transition` once at the top of `validate/1` and look up by `arc.transition` in the t_to_p clause. Replaces the prior per-arc `Enum.flat_map` over the full arc list (O(arcs²) → O(arcs)) and fixes the correctness leak with a regression test. The unreachable `Map.get(outputs, _, [])` default is hardened to `MapSet.new()` so the type matches `MapSet.union/2`'s contract.
1 parent 4c66c1c commit 3ee1bb0

2 files changed

Lines changed: 84 additions & 11 deletions

File tree

lib/coloured_flow/validators/definition/arc_validator.ex

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,19 @@ defmodule ColouredFlow.Validators.Definition.ArcValidator do
1313
alias ColouredFlow.Definition.Arc
1414
alias ColouredFlow.Definition.ColouredPetriNet
1515
alias ColouredFlow.Definition.Transition
16+
alias ColouredFlow.Definition.Variable
1617
alias ColouredFlow.Validators.Exceptions.InvalidArcError
1718

1819
@spec validate(ColouredPetriNet.t()) ::
1920
{:ok, ColouredPetriNet.t()} | {:error, InvalidArcError.t()}
2021
def validate(%ColouredPetriNet{} = cpnet) do
2122
vars_and_consts = build_vars_and_consts(cpnet)
2223
outputs = build_outputs(cpnet)
24+
bound_vars_by_transition = build_bound_vars_by_transition(cpnet)
2325

2426
cpnet.arcs
2527
|> Enum.find_value(fn %Arc{} = arc ->
26-
case validate_arc(arc, vars_and_consts, outputs, cpnet) do
28+
case validate_arc(arc, vars_and_consts, outputs, bound_vars_by_transition) do
2729
:ok -> nil
2830
{:error, reason} -> reason
2931
end
@@ -38,7 +40,7 @@ defmodule ColouredFlow.Validators.Definition.ArcValidator do
3840
%Arc{orientation: :p_to_t} = arc,
3941
{vars, consts},
4042
_outputs,
41-
%ColouredPetriNet{}
43+
_bound_vars_by_transition
4244
) do
4345
arc.expression.vars
4446
|> MapSet.new()
@@ -71,20 +73,14 @@ defmodule ColouredFlow.Validators.Definition.ArcValidator do
7173
%Arc{orientation: :t_to_p} = arc,
7274
{_vars, consts},
7375
outputs,
74-
%ColouredPetriNet{} = cpnet
76+
bound_vars_by_transition
7577
) do
76-
bound_vars =
77-
cpnet.arcs
78-
|> Enum.flat_map(fn
79-
%Arc{orientation: :p_to_t} = arc -> arc.expression.vars
80-
%Arc{} -> []
81-
end)
82-
|> MapSet.new()
78+
bound_vars = Map.get(bound_vars_by_transition, arc.transition, MapSet.new())
8379

8480
vars_and_consts =
8581
bound_vars
8682
|> MapSet.union(consts)
87-
|> MapSet.union(Map.get(outputs, arc.transition, []))
83+
|> MapSet.union(Map.get(outputs, arc.transition, MapSet.new()))
8884

8985
arc.expression.vars
9086
|> MapSet.new()
@@ -124,4 +120,15 @@ defmodule ColouredFlow.Validators.Definition.ArcValidator do
124120
{transition.name, MapSet.new(transition.action.outputs)}
125121
end)
126122
end
123+
124+
@spec build_bound_vars_by_transition(ColouredPetriNet.t()) ::
125+
%{Transition.name() => MapSet.t(Variable.name())}
126+
defp build_bound_vars_by_transition(%ColouredPetriNet{arcs: arcs}) do
127+
arcs
128+
|> Stream.filter(&(&1.orientation == :p_to_t))
129+
|> Enum.group_by(& &1.transition, & &1.expression.vars)
130+
|> Map.new(fn {transition, vars_lists} ->
131+
{transition, vars_lists |> List.flatten() |> MapSet.new()}
132+
end)
133+
end
127134
end

test/coloured_flow/validators/definition/arc_validator_test.exs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,72 @@ defmodule ColouredFlow.Validators.Definition.ArcValidatorTest do
119119
ArcValidator.validate(cpnet)
120120
end
121121

122+
test "outgoing_unbound_vars error when var is bound on a different transition" do
123+
# Two transitions T1 and T2, each with their own input/output places.
124+
# T1 has an incoming arc binding `x`.
125+
# T2 has an outgoing arc referencing `x`, but T2's incoming arc only binds `y`.
126+
# In CPN semantics, variables bind only within a single transition's firing,
127+
# so T2's outgoing arc must NOT see `x` from T1.
128+
cpnet = %ColouredPetriNet{
129+
colour_sets: [
130+
colset(int() :: integer())
131+
],
132+
places: [
133+
%Place{name: "p1_in", colour_set: :int},
134+
%Place{name: "p1_out", colour_set: :int},
135+
%Place{name: "p2_in", colour_set: :int},
136+
%Place{name: "p2_out", colour_set: :int}
137+
],
138+
transitions: [
139+
build_transition!(name: "t1", guard: "true"),
140+
build_transition!(name: "t2", guard: "true")
141+
],
142+
arcs: [
143+
# T1 binds `x` on its incoming arc.
144+
build_arc!(
145+
label: "t1-in",
146+
place: "p1_in",
147+
transition: "t1",
148+
orientation: :p_to_t,
149+
expression: "bind {1, x}"
150+
),
151+
build_arc!(
152+
label: "t1-out",
153+
place: "p1_out",
154+
transition: "t1",
155+
orientation: :t_to_p,
156+
expression: "{1, x}"
157+
),
158+
# T2 binds only `y` — no `x` is in scope here.
159+
build_arc!(
160+
label: "t2-in",
161+
place: "p2_in",
162+
transition: "t2",
163+
orientation: :p_to_t,
164+
expression: "bind {1, y}"
165+
),
166+
# This outgoing arc references `x`, which was bound on a *different*
167+
# transition. Pre-fix, the validator pooled bound vars across all
168+
# transitions and silently accepted this; post-fix, it must reject.
169+
build_arc!(
170+
label: "t2-out",
171+
place: "p2_out",
172+
transition: "t2",
173+
orientation: :t_to_p,
174+
expression: "{1, x}"
175+
)
176+
],
177+
variables: [
178+
%Variable{name: :x, colour_set: :int},
179+
%Variable{name: :y, colour_set: :int}
180+
],
181+
constants: []
182+
}
183+
184+
assert {:error, %InvalidArcError{reason: :outgoing_unbound_vars}} =
185+
ArcValidator.validate(cpnet)
186+
end
187+
122188
defp update_action(%ColouredPetriNet{} = cpnet, %Action{} = action) do
123189
put_in(
124190
cpnet,

0 commit comments

Comments
 (0)