Skip to content

Commit 9e67858

Browse files
committed
fix(eval): don't leak the implicit 'use Dynamic' into the global env
evalDynamicVM, evalExpandVM and annotateWithinContextVM installed an implicit 'use Dynamic' on the global env so unqualified dynamic calls resolve during evaluation, but the modified env escaped through the returned context and stuck around permanently. Static symbol qualification then saw the Dynamic module as a used module, so unqualified multisym calls (like 'reduce' under '(use Array)') inside nested modules picked up Dynamic.* candidates, failed to narrow to a single binding, and silently stayed generic -- surfacing later as 'unresolved generic type' errors at call sites that couldn't pin the types either. The implicit use path is now scrubbed from result contexts again (unless the global env legitimately had 'use Dynamic' before), and qualification in annotateWithinContextVM runs on the unpolluted context.
1 parent 0d354aa commit 9e67858

3 files changed

Lines changed: 62 additions & 13 deletions

File tree

src/Env.hs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ module Env
4040
deleteBinding,
4141
addListOfBindings,
4242
addUsePath,
43+
removeUsePath,
4344
-------------------------
4445
-- finds
4546
findPoly,
@@ -426,6 +427,10 @@ addListOfBindings e bindings =
426427
addUsePath :: Environment e => e -> SymPath -> e
427428
addUsePath e path = inj ((prj e) {envUseModules = Set.insert path (envUseModules (prj e))})
428429

430+
-- | Remove a module path from an environment's list of used modules.
431+
removeUsePath :: Environment e => e -> SymPath -> e
432+
removeUsePath e path = inj ((prj e) {envUseModules = Set.delete path (envUseModules (prj e))})
433+
429434
--------------------------------------------------------------------------------
430435
-- Additional binding lookup functions
431436
--

src/EvalVM.hs

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -189,21 +189,40 @@ runEvalIRVMWithPhase phase ctx ir preference =
189189
runEvalCodeWithPhase phase ctx preference compiled
190190

191191
evalDynamicVM :: Context -> XObj -> IO (Context, Either EvalError XObj)
192-
evalDynamicVM ctx xobj =
193-
let ctx' = ensureDynamicUse ctx
194-
in runEvalIRVMWithPhase PhaseExecute ctx' (lowerExpr ctx' xobj) PreferDynamic
192+
evalDynamicVM ctx xobj = do
193+
let hadDynamicUse = Set.member dynamicUsePath (envUseModules (contextGlobalEnv ctx))
194+
ctx' = ensureDynamicUse ctx
195+
(resCtx, res) <- runEvalIRVMWithPhase PhaseExecute ctx' (lowerExpr ctx' xobj) PreferDynamic
196+
pure (scrubDynamicUse hadDynamicUse resCtx, res)
195197

196198
evalExpandVM :: Context -> XObj -> IO (Context, Either EvalError XObj)
197-
evalExpandVM ctx xobj =
198-
let ctx' = ensureDynamicUse ctx
199-
in runEvalIRVMWithPhase PhaseExpand ctx' (lowerExpr ctx' xobj) PreferDynamic
199+
evalExpandVM ctx xobj = do
200+
-- The implicit `use Dynamic` is only for resolving unqualified dynamic
201+
-- calls during evaluation. It must not leak into the persistent global
202+
-- env, where it would pollute static symbol qualification with Dynamic
203+
-- module candidates.
204+
let hadDynamicUse = Set.member dynamicUsePath (envUseModules (contextGlobalEnv ctx))
205+
ctx' = ensureDynamicUse ctx
206+
(resCtx, res) <- runEvalIRVMWithPhase PhaseExpand ctx' (lowerExpr ctx' xobj) PreferDynamic
207+
pure (scrubDynamicUse hadDynamicUse resCtx, res)
200208

201209
macroExpandVM :: Context -> XObj -> IO (Context, Either EvalError XObj)
202210
macroExpandVM ctx xobj = expand MacroExpandOnly evalExpandVM ctx xobj
203211

212+
dynamicUsePath :: SymPath
213+
dynamicUsePath = SymPath [] "Dynamic"
214+
204215
ensureDynamicUse :: Context -> Context
205216
ensureDynamicUse ctx =
206-
replaceGlobalEnv ctx (E.addUsePath (contextGlobalEnv ctx) (SymPath [] "Dynamic"))
217+
replaceGlobalEnv ctx (E.addUsePath (contextGlobalEnv ctx) dynamicUsePath)
218+
219+
-- | Drop the implicit `use Dynamic` from a result context again, unless the
220+
-- global env legitimately had it before the evaluation.
221+
scrubDynamicUse :: Bool -> Context -> Context
222+
scrubDynamicUse hadDynamicUse ctx =
223+
if hadDynamicUse
224+
then ctx
225+
else replaceGlobalEnv ctx (E.removeUsePath (contextGlobalEnv ctx) dynamicUsePath)
207226

208227
data VMClosurePayload
209228
= VMPrecompiled EvalCode
@@ -408,19 +427,18 @@ specialCommandDefineVM ctx xobj = do
408427

409428
annotateWithinContextVM :: Context -> XObj -> IO (Context, Either EvalError (XObj, [XObj]))
410429
annotateWithinContextVM ctx xobj = do
411-
let ctxDyn = ensureDynamicUse ctx
412-
globalEnv = contextGlobalEnv ctxDyn
413-
typeEnv = contextTypeEnv ctxDyn
414-
sig = getSigFromDefnOrDefVM ctxDyn xobj
430+
let globalEnv = contextGlobalEnv ctx
431+
typeEnv = contextTypeEnv ctx
432+
sig = getSigFromDefnOrDefVM ctx xobj
415433
fppl = projectFilePathPrintLength (contextProj ctx)
416434
case sig of
417435
Left err -> pure (ctx, Left err)
418436
Right okSig -> do
419-
(_, expansionResult) <- expandAll evalExpandVM ctxDyn xobj
437+
(_, expansionResult) <- expandAll evalExpandVM ctx xobj
420438
case expansionResult of
421439
Left err -> pure (ctx, Left err)
422440
Right expanded ->
423-
let xobjFullSymbols = qualify ctxDyn expanded
441+
let xobjFullSymbols = qualify ctx expanded
424442
in case xobjFullSymbols of
425443
Left err -> pure (evalError ctx (show err) (xobjInfo xobj))
426444
Right xs ->

test/nested_module_multisym.carp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
(load "Test.carp")
2+
(use Test)
3+
4+
; Regression test: the implicit `use Dynamic` that macro expansion installs
5+
; used to leak into the global env, which made unqualified multisym calls
6+
; (like `reduce` under `(use Array)`) inside nested modules pick up Dynamic
7+
; module candidates and stay generic instead of resolving concretely.
8+
9+
(defmodule O
10+
(deftype CM [values (Array (Pair String String))])
11+
(defmodule CM
12+
(use Array)
13+
(defn to-map [m]
14+
(reduce
15+
&(fn [a v] (Map.put a (Pair.a v) (Pair.b v)))
16+
{}
17+
(CM.values m)))))
18+
19+
(defn stringify []
20+
(str &(Map.get &(O.CM.to-map &(O.CM.init [(Pair.init @"k" @"v")])) "k")))
21+
22+
(deftest test
23+
(assert-equal test
24+
"v"
25+
&(stringify)
26+
"unqualified multisym calls in nested modules resolve concretely"))

0 commit comments

Comments
 (0)