fix: make set! work with dynamic args (thanks @hellerve) (#1151)

* fix: make set! work with dynamic args (thanks @hellerve)

Like `let` before it, we used to bind function arguments to their values
only, which wasn't accounted for in `set!` such that one could not
`set!` `i` in `defndynamic [i] ...`. To fix this for let bindings we
introduced a `LetDef` form for consistency with Def forms. This commit
renames `LetDef` to `LocalDef` and uses it as a value for function
arguments in addition to let bindings, ensuring `set!` works on function
arguments too. Big thanks to @hellerve for the suggestion!

* test: add regression test for set! on dynamic function args

src/Emit.hs

@@ -168,7 +168,7 @@ toC toCMode (Binder meta root) = emitterSrc (execState (visit startingIndent roo
             (Interface _ _) -> dontVisit
             (Dict _) -> dontVisit
             (Fn _ _) -> dontVisit
-            LetDef -> dontVisit
+            LocalDef -> dontVisit
             (Match _) -> dontVisit
             With -> dontVisit
             MetaStub -> dontVisit

src/Eval.hs

@@ -142,7 +142,7 @@ eval ctx xobj@(XObj o info ty) preference resolver =
         Right (val, _) -> (nctx, Right val)
   where
     resolveDef (XObj (Lst [XObj DefDynamic _ _, _, value]) _ _) = value
-    resolveDef (XObj (Lst [XObj LetDef _ _, _, value]) _ _) = value
+    resolveDef (XObj (Lst [XObj LocalDef _ _, _, value]) _ _) = value
     resolveDef x = x
     eval' form =
       case form of
@@ -296,7 +296,7 @@ eval ctx xobj@(XObj o info ty) preference resolver =
                   (newCtx, res) <- eval ctx' x preference resolver
                   case res of
                     Right okX -> do
-                      let binder = Binder emptyMeta (XObj (Lst [XObj LetDef Nothing Nothing, XObj (Sym (SymPath [] n) Symbol) Nothing Nothing, okX]) Nothing (xobjTy okX))
+                      let binder = Binder emptyMeta (toLocalDef n okX)
                           Just e = contextInternalEnv newCtx
                       pure $ Right (newCtx {contextInternalEnv = Just (envInsertAt e (SymPath [] n) binder)})
                     Left err -> pure $ Left err
@@ -545,8 +545,8 @@ apply ctx@Context {contextInternalEnv = internal} body params args =
       let n = length proper
           insideEnv = Env Map.empty internal Nothing [] InternalEnv 0
           insideEnv' =
-            foldl'
-              (\e (p, x) -> extendEnv e p x)
+           foldl'
+              (\e (p, x) -> extendEnv e p (toLocalDef p x))
               insideEnv
               (zip proper (take n args))
           insideEnv'' =
@@ -1220,7 +1220,7 @@ setStaticOrDynamicVar path env binder value =
       envReplaceBinding path (Binder meta (XObj (Lst [def, sym, value]) (xobjInfo value) t)) env
     (Binder meta (XObj (Lst (defdy@(XObj DefDynamic _ _) : sym : _)) _ _)) ->
       envReplaceBinding path (Binder meta (XObj (Lst [defdy, sym, value]) (xobjInfo value) (Just DynamicTy))) env
-    (Binder meta (XObj (Lst (lett@(XObj LetDef _ _) : sym : _)) _ t)) ->
+    (Binder meta (XObj (Lst (lett@(XObj LocalDef _ _) : sym : _)) _ t)) ->
       envReplaceBinding path (Binder meta (XObj (Lst [lett, sym, value]) (xobjInfo value) t)) env
     -- shouldn't happen, errors are thrown at call sites.
     -- TODO: Return an either here to propagate error.

src/Obj.hs

@@ -130,7 +130,7 @@ data Obj
   | Fn (Maybe SymPath) (Set.Set XObj) -- the name of the lifted function, the set of variables this lambda captures, and a dynamic environment
   | Do
   | Let
-  | LetDef
+  | LocalDef
   | While
   | Break
   | If
@@ -352,7 +352,7 @@ getSimpleNameWithArgs _ = Nothing
 getPath :: XObj -> SymPath
 getPath (XObj (Lst (XObj (Defn _) _ _ : XObj (Sym path _) _ _ : _)) _ _) = path
 getPath (XObj (Lst (XObj Def _ _ : XObj (Sym path _) _ _ : _)) _ _) = path
-getPath (XObj (Lst (XObj LetDef _ _ : XObj (Sym path _) _ _ : _)) _ _) = path
+getPath (XObj (Lst (XObj LocalDef _ _ : XObj (Sym path _) _ _ : _)) _ _) = path
 getPath (XObj (Lst (XObj Macro _ _ : XObj (Sym path _) _ _ : _)) _ _) = path
 getPath (XObj (Lst (XObj Dynamic _ _ : XObj (Sym path _) _ _ : _)) _ _) = path
 getPath (XObj (Lst (XObj DefDynamic _ _ : XObj (Sym path _) _ _ : _)) _ _) = path
@@ -426,7 +426,7 @@ pretty = visit 0
         While -> "while"
         Do -> "do"
         Let -> "let"
-        LetDef -> "let"
+        LocalDef -> "local-binding"
         Mod env -> fromMaybe "module" (envModuleName env)
         Deftype _ -> "deftype"
         DefSumtype _ -> "deftype"
@@ -491,7 +491,7 @@ prettyUpTo lim xobj =
         While -> ""
         Do -> ""
         Let -> ""
-        LetDef -> ""
+        LocalDef -> ""
         Mod _ -> ""
         Deftype _ -> ""
         DefSumtype _ -> ""
@@ -1080,3 +1080,7 @@ instance Semigroup Context where
             contextInternalEnv = internal <> internal',
             contextTypeEnv = TypeEnv (typeEnv' <> typeEnv)
           }
+
+toLocalDef :: String -> XObj -> XObj
+toLocalDef var value =
+  (XObj (Lst [XObj LocalDef Nothing Nothing, XObj (Sym (SymPath [] var) Symbol) Nothing Nothing, value]) (xobjInfo value) (xobjTy value))

test/regression.carp

@@ -28,6 +28,12 @@
   (Piff [String])
   (Puff [String]))
 
+; set! works on arguments (issue #1144)
+(defndynamic set-args [i]
+  (do (set! i (+ i 2)) i))
+
+(defmacro call-set-args [] (set-args 2))
+
 (defn match-ref-1 []
   (let [xs [(StrangeThings.Puff @"ABCD")]]
     (match-ref (Array.unsafe-nth &xs 0)
@@ -66,6 +72,10 @@
                 2
                 (nested-lambdas)
                 "test that nested lambdas can use captured values")
+  (assert-equal test
+                4
+                (call-set-args)
+                "test that set! works on dynamic function arguments")
   (assert-true test
                (let-and-set)
                "test that nested let bindings and set! interplay nicely")