Skip to content

Commit ae9b40f

Browse files
committed
fix(cek): shift free variables in VBuiltin during discharge
VBuiltin values can be stored in an environment and later discharged under additional binders. The previous code returned VBuiltin terms without shifting, causing variable capture. Add shiftTerm helper to shift free variables in discharged VBuiltin terms, add bang pattern to shiftNamedDeBruijn for consistency, and add a VBuiltin test case reproducing basetunnel's example from issue #7526.
1 parent 054a781 commit ae9b40f

3 files changed

Lines changed: 65 additions & 8 deletions

File tree

plutus-core/plutus-core/src/PlutusCore/DeBruijn/Internal.hs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
{-# LANGUAGE BangPatterns #-}
12
{-# LANGUAGE DeriveAnyClass #-}
23
{-# LANGUAGE FlexibleInstances #-}
34
{-# LANGUAGE LambdaCase #-}
@@ -112,7 +113,7 @@ deBruijnInitIndex = 0
112113
The addition is unchecked and will silently wrap on 'Word64' overflow,
113114
which is safe in practice since terms with @2^64@ nested binders cannot be constructed. -}
114115
shiftNamedDeBruijn :: Word64 -> NamedDeBruijn -> NamedDeBruijn
115-
shiftNamedDeBruijn i (NamedDeBruijn t (Index n)) = NamedDeBruijn t (Index (n + i))
116+
shiftNamedDeBruijn !i (NamedDeBruijn t (Index n)) = NamedDeBruijn t (Index (n + i))
116117

117118
-- The bangs gave us a speedup of 6%.
118119

plutus-core/untyped-plutus-core/src/UntypedPlutusCore/Evaluation/Machine/Cek/Internal.hs

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -293,8 +293,10 @@ data CekValue uni fun ann
293293
| VLamAbs !NamedDeBruijn !(NTerm uni fun ann) !(CekValEnv uni fun ann)
294294
| {-| A partial builtin application, accumulating arguments for eventual full application.
295295
We don't need a 'CekValEnv' here unlike in the other constructors, because 'VBuiltin'
296-
values always store their corresponding 'Term's fully discharged, see the comments at
297-
the call sites (search for 'VBuiltin'). -}
296+
values store their corresponding 'Term's fully discharged, see the comments at
297+
the call sites (search for 'VBuiltin'). Note however that a 'VBuiltin' /can/ be stored
298+
in an environment (e.g. when passed as an argument to a lambda), so 'dischargeCekValue'
299+
must still shift its free variables when discharging under additional binders. -}
298300
VBuiltin
299301
!fun
300302
{-^ So that we know, for what builtin we're calculating the cost. We can sneak this into
@@ -673,11 +675,10 @@ dischargeCekValue value0 = DischargeNonConstant $ goValue 0 value0
673675
-- We only return a discharged builtin application when (a) it's being returned by the
674676
-- machine, or (b) it's needed for an error message.
675677
-- @term@ is fully discharged, so we can return it directly without any further discharging.
676-
-- In particular, no @global@ shifting is needed because the @term@ field of 'VBuiltin'
677-
-- is maintained during evaluation as a fully-applied UPLC term whose variables already
678-
-- refer to the correct scope — it is never stored in an environment to be discharged
679-
-- under additional binders later.
680-
VBuiltin _ term _ -> term
678+
-- However, @global@ shifting IS needed because a 'VBuiltin' can be stored in an
679+
-- environment and later discharged under additional binders — e.g. when a partially
680+
-- applied builtin containing free variables is passed as an argument to a lambda.
681+
VBuiltin _ term _ -> shiftTerm global term
681682
VConstr ind args -> Constr () ind . map (goValue global) $ argStackToList args
682683

683684
-- Instantiate all the free variables of a term by looking them up in an environment.
@@ -712,6 +713,27 @@ dischargeCekValue value0 = DischargeNonConstant $ goValue 0 value0
712713
Constr _ ind args -> Constr () ind $ map (go global shift) args
713714
Case _ scrut alts -> Case () (go global shift scrut) $ fmap (go global shift) alts
714715

716+
-- \| Shift all free variables in a fully discharged term by the given amount.
717+
-- Used for 'VBuiltin' terms which have no associated environment.
718+
shiftTerm :: Word64 -> NTerm uni fun () -> NTerm uni fun ()
719+
shiftTerm 0 t = t
720+
shiftTerm amount t = goShift 0 t
721+
where
722+
goShift :: Word64 -> NTerm uni fun () -> NTerm uni fun ()
723+
goShift !depth = \case
724+
Var _ named@(NamedDeBruijn _ (coerce -> idx))
725+
| depth >= idx -> Var () named
726+
| otherwise -> Var () (shiftNamedDeBruijn amount named)
727+
LamAbs _ name body -> LamAbs () name $ goShift (depth + 1) body
728+
Apply _ fun arg -> Apply () (goShift depth fun) (goShift depth arg)
729+
Delay _ term -> Delay () $ goShift depth term
730+
Force _ term -> Force () $ goShift depth term
731+
Constant _ val -> Constant () val
732+
Builtin _ fun -> Builtin () fun
733+
Error _ -> Error ()
734+
Constr _ ind args -> Constr () ind $ map (goShift depth) args
735+
Case _ scrut alts -> Case () (goShift depth scrut) $ fmap (goShift depth) alts
736+
715737
instance (PrettyUni uni, Pretty fun) => PrettyBy PrettyConfigPlc (CekValue uni fun ann) where
716738
prettyBy cfg = prettyBy cfg . dischargeResultToTerm . dischargeCekValue
717739

plutus-core/untyped-plutus-core/testlib/Evaluation/FreeVars.hs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44

55
module Evaluation.FreeVars (test_freevars) where
66

7+
import PlutusCore.Builtin
8+
( BuiltinError (BuiltinEvaluationFailure)
9+
, BuiltinRuntime
10+
, builtinRuntimeFailure
11+
)
712
import PlutusCore.Default
813
import PlutusCore.Evaluation.Machine.ExBudgetingDefaults qualified as PLC
914
import PlutusCore.MkPlc
@@ -81,6 +86,8 @@ testDischargeFree =
8186
("boundaryShiftEqualsIdx", boundaryShiftEqualsIdx)
8287
, -- Constructor arguments containing free variables
8388
("constrWithFreeVars", constrWithFreeVars)
89+
, -- VBuiltin with free variables (basetunnel's example from issue #7526)
90+
("builtinWithFreeVars", builtinWithFreeVars)
8491
]
8592
where
8693
delayWithEmptyEnv =
@@ -322,6 +329,33 @@ testDischargeFree =
322329
Constr () 0 [Delay () (v 2)] -- var 1 shifted by 1
323330
)
324331

332+
builtinWithFreeVars =
333+
-- VLamAbs _ (var 2) [VBuiltin IfThenElse "(force ifThenElse) True (delay (var 1))" _]
334+
-- The VBuiltin term contains free var 1.
335+
-- Under 1 lambda: var 2 looks up env[1] → VBuiltin → term returned.
336+
-- Free var 1 in term should shift by 1 → var 2.
337+
dis
338+
( VLamAbs
339+
(fakeNameDeBruijn $ DeBruijn deBruijnInitIndex)
340+
(toFakeTerm $ v 2)
341+
[ VBuiltin
342+
IfThenElse
343+
( toFakeTerm $
344+
Force () (Builtin () IfThenElse)
345+
@@ [Constant () (someValue True), Delay () (v 1)]
346+
)
347+
dummyRuntime
348+
]
349+
)
350+
@?= DischargeNonConstant
351+
( toFakeTerm . lamAbs0 $
352+
Force () (Builtin () IfThenElse)
353+
@@ [Constant () (someValue True), Delay () (v 2)]
354+
)
355+
356+
dummyRuntime :: BuiltinRuntime val
357+
dummyRuntime = builtinRuntimeFailure BuiltinEvaluationFailure
358+
325359
dis = dischargeCekValue @DefaultUni @DefaultFun
326360
v = Var () . DeBruijn
327361

0 commit comments

Comments
 (0)