From fd49cc4bc78d3929450a148c54ac31ddfc3212b9 Mon Sep 17 00:00:00 2001 From: Brent Yorgey Date: Sun, 5 Jan 2025 17:46:16 -0600 Subject: [PATCH 1/5] *evaluate* argument to instant/atomic atomically, as a special case --- data/scenarios/Challenges/_gallery/setup.sw | 12 ++++-- data/scenarios/Testing/2270-instant-defs.yaml | 42 +++++++++++++++++++ src/swarm-engine/Swarm/Game/Step.hs | 9 ++++ test/integration/Main.hs | 1 + 4 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 data/scenarios/Testing/2270-instant-defs.yaml diff --git a/data/scenarios/Challenges/_gallery/setup.sw b/data/scenarios/Challenges/_gallery/setup.sw index 72ccc5a87..9a5ad4b2e 100644 --- a/data/scenarios/Challenges/_gallery/setup.sw +++ b/data/scenarios/Challenges/_gallery/setup.sw @@ -1,3 +1,5 @@ +instant ( + def doN = \n. \f. if (n > 0) {f; doN (n - 1) f} {}; end; def isDivisibleBy = \dividend. \divisor. @@ -99,7 +101,7 @@ def length : (rec l. Unit + a * l) -> Int = \l. end def busts : (rec l. Unit + Text * l) = tagmembers "bust" end -def bustCount : {Int} = {length busts} end +def bustCount : Int = length busts end def placeThing = \entIdx. let entName = index entIdx busts in @@ -125,7 +127,7 @@ bust in the base's inventory increases monotonically. */ def populateInventory = \baseCount. \idx. - if (idx < force bustCount) { + if (idx < bustCount) { let item = index idx busts in @@ -150,14 +152,16 @@ def populateInventory = \baseCount. \idx. def setup = populateInventory 0 0; - naiveRandomStack placeEntByIndex (force bustCount) 0 (force bustCount); + naiveRandomStack placeEntByIndex bustCount 0 bustCount; turn back; move; create "bitcoin"; end; def go = - instant setup; + setup; end; go; + +) diff --git a/data/scenarios/Testing/2270-instant-defs.yaml b/data/scenarios/Testing/2270-instant-defs.yaml new file mode 100644 index 000000000..fc6498f39 --- /dev/null +++ b/data/scenarios/Testing/2270-instant-defs.yaml @@ -0,0 +1,42 @@ +version: 1 +name: Instant wrapped defs +description: | + `instant` should work when wrapped around definitions +creative: false +objectives: + - goal: + - Grab the rock + condition: | + as base { has "rock" } +robots: + - name: base + dir: east + devices: + - logger + - treads + - grabber + - name: judge + dir: east + system: true + program: | + instant ( + def fib : Int -> Int = \n. + if (n <= 1) {n} {fib (n-1) + fib (n-2)} + end + + def x = fib 10 end + + create "rock"; place "rock" + ) +solution: | + move; grab +known: [rock] +world: + dsl: | + {grass} + palette: + 'B': [grass, null, base] + 'j': [grass, null, judge] + upperleft: [0, 0] + map: | + Bj diff --git a/src/swarm-engine/Swarm/Game/Step.hs b/src/swarm-engine/Swarm/Game/Step.hs index 6ac85ce2c..857bbdf88 100644 --- a/src/swarm-engine/Swarm/Game/Step.hs +++ b/src/swarm-engine/Swarm/Game/Step.hs @@ -603,6 +603,15 @@ stepCESK cesk = case cesk of Out v2 s (FFst v1 : k) -> return $ Out (VPair v1 v2) s k -- Lambdas immediately turn into closures. In (TLam x _ t) e s k -> return $ Out (VClo x t e) s k + -- Special case for evaluating an application of Instant or Atomic: + -- set the runningAtomic flag and push a stack frame to unset it + -- when done evaluating. We do this here so that even *evaluating* + -- the argument to instant/atomic will happen atomically (#2270). + -- *Execution* will also happen atomically; that is handled in execConst. + In (TApp (TConst c) t2) e s k + | c `elem` [Atomic, Instant] -> do + runningAtomic .= True + return $ In t2 e s (FApp (VCApp c []) : FFinishAtomic : k) -- To evaluate an application, start by focusing on the left-hand -- side and saving the argument for later. In (TApp t1 t2) e s k -> return $ In t1 e s (FArg t2 e : k) diff --git a/test/integration/Main.hs b/test/integration/Main.hs index ca4966fd1..2b835a534 100644 --- a/test/integration/Main.hs +++ b/test/integration/Main.hs @@ -509,6 +509,7 @@ testScenarioSolutions rs ui key = not (any ("- treads" `T.isInfixOf`) msgs) && any ("- tank treads" `T.isInfixOf`) msgs , testSolution Default "Testing/2253-halt-waiting" + , testSolution Default "Testing/2270-instant-defs" ] where -- expectFailIf :: Bool -> String -> TestTree -> TestTree From 1695e6d7eb03ad36e9f16172414af93b6979449e Mon Sep 17 00:00:00 2001 From: Brent Yorgey Date: Sun, 5 Jan 2025 20:34:12 -0600 Subject: [PATCH 2/5] apply restyling --- src/swarm-engine/Swarm/Game/Step.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/swarm-engine/Swarm/Game/Step.hs b/src/swarm-engine/Swarm/Game/Step.hs index 857bbdf88..66e759997 100644 --- a/src/swarm-engine/Swarm/Game/Step.hs +++ b/src/swarm-engine/Swarm/Game/Step.hs @@ -605,9 +605,9 @@ stepCESK cesk = case cesk of In (TLam x _ t) e s k -> return $ Out (VClo x t e) s k -- Special case for evaluating an application of Instant or Atomic: -- set the runningAtomic flag and push a stack frame to unset it - -- when done evaluating. We do this here so that even *evaluating* + -- when done evaluating. We do this here so that even /evaluating/ -- the argument to instant/atomic will happen atomically (#2270). - -- *Execution* will also happen atomically; that is handled in execConst. + -- Execution will also happen atomically; that is handled in execConst. In (TApp (TConst c) t2) e s k | c `elem` [Atomic, Instant] -> do runningAtomic .= True From dc86f54b30a6bfdbe58446cd37fb797ed498a307 Mon Sep 17 00:00:00 2001 From: Brent Yorgey Date: Sun, 5 Jan 2025 20:43:39 -0600 Subject: [PATCH 3/5] add new testing scenario to 00-ORDER --- data/scenarios/Testing/00-ORDER.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/data/scenarios/Testing/00-ORDER.txt b/data/scenarios/Testing/00-ORDER.txt index 67efafd9f..8e5cd929e 100644 --- a/data/scenarios/Testing/00-ORDER.txt +++ b/data/scenarios/Testing/00-ORDER.txt @@ -73,3 +73,4 @@ Achievements 2239-custom-entity.yaml 2240-overridden-entity-capabilities.yaml 2253-halt-waiting.yaml +2270-instant-defs.yaml From 941c3a1740acc80fb54ab79bf5c035caa062e402 Mon Sep 17 00:00:00 2001 From: Brent Yorgey Date: Sun, 5 Jan 2025 20:51:56 -0600 Subject: [PATCH 4/5] fix test by putting in another special case and a bunch of comments --- src/swarm-engine/Swarm/Game/Step.hs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/swarm-engine/Swarm/Game/Step.hs b/src/swarm-engine/Swarm/Game/Step.hs index 66e759997..31a91ed5d 100644 --- a/src/swarm-engine/Swarm/Game/Step.hs +++ b/src/swarm-engine/Swarm/Game/Step.hs @@ -607,11 +607,29 @@ stepCESK cesk = case cesk of -- set the runningAtomic flag and push a stack frame to unset it -- when done evaluating. We do this here so that even /evaluating/ -- the argument to instant/atomic will happen atomically (#2270). - -- Execution will also happen atomically; that is handled in execConst. + -- Execution will also happen atomically; that is handled in + -- execConst. In (TApp (TConst c) t2) e s k | c `elem` [Atomic, Instant] -> do runningAtomic .= True - return $ In t2 e s (FApp (VCApp c []) : FFinishAtomic : k) + case k of + -- In the (common) special case that we will immediately + -- execute the atomic/instant command next, don't bother + -- pushing an FFinishAtomic frame. That way, runningAtomic + -- will remain set, and evaluation + execution together will + -- all happen in a single tick. + FExec : _ -> return $ In t2 e s (FApp (VCApp c []) : k) + + -- Otherwise, in general, other evaluation may take place in + -- between evaluating the argument to atomic/instant and + -- executing it, so we must push an FFinishAtomic frame so + -- that intermediate evaluation will not happen atomically. + -- For example, consider something like `f (instant c)`, + -- where `f : Cmd Unit -> Cmd Unit`. After evaluating `c` + -- atomically, `instant c` is then passed to `f`, which may + -- do some (non-atomic) computation before executing its + -- argument (if it is executed at all). + _ -> return $ In t2 e s (FApp (VCApp c []) : FFinishAtomic : k) -- To evaluate an application, start by focusing on the left-hand -- side and saving the argument for later. In (TApp t1 t2) e s k -> return $ In t1 e s (FArg t2 e : k) From 0e6b378cdc50243299a792cc15889503088c8b63 Mon Sep 17 00:00:00 2001 From: Brent Yorgey Date: Sun, 5 Jan 2025 21:06:37 -0600 Subject: [PATCH 5/5] apply restyling --- src/swarm-engine/Swarm/Game/Step.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/swarm-engine/Swarm/Game/Step.hs b/src/swarm-engine/Swarm/Game/Step.hs index 31a91ed5d..5fdf74ebd 100644 --- a/src/swarm-engine/Swarm/Game/Step.hs +++ b/src/swarm-engine/Swarm/Game/Step.hs @@ -619,7 +619,6 @@ stepCESK cesk = case cesk of -- will remain set, and evaluation + execution together will -- all happen in a single tick. FExec : _ -> return $ In t2 e s (FApp (VCApp c []) : k) - -- Otherwise, in general, other evaluation may take place in -- between evaluating the argument to atomic/instant and -- executing it, so we must push an FFinishAtomic frame so