Remove parameters from env after visiting function. Fixes #321 (#347)

src/analyzer.fs

@@ -74,10 +74,10 @@ module private VariableInlining =
             if not ident.DoNotInline && not ident.ToBeInlined && not ident.VarDecl.Value.isEverWrittenAfterDecl then
                 match localReferences.TryGetValue(def.Key), allReferences.TryGetValue(def.Key) with
                 | (_, 1), (_, 1) when isConst ->
-                    debug $"inlining variable '{Printer.debugIdent ident}' because it's safe to inline and used only once"
+                    debug $"inlining local variable '{Printer.debugIdent ident}' because it's safe to inline and used only once"
                     ident.ToBeInlined <- true
                 | (_, 0), (_, 0) ->
-                    debug $"inlining variable '{Printer.debugIdent ident}' because it's safe to inline and unused"
+                    debug $"inlining local variable '{Printer.debugIdent ident}' because it's safe to inline and unused"
                     ident.ToBeInlined <- true
                 | _ -> ()
 
@@ -124,7 +124,8 @@ module private VariableInlining =
                         if canBeInlined init then
                             // Never-written locals and globals are inlined when their value is "simple enough".
                             // This can increase non-compressed size but decreases compressed size.
-                            debug $"inlining variable '{Printer.debugDecl def}' because it's never written and has a 'simple' definition"
+                            let varKind = if isTopLevel then "global" else "local"
+                            debug $"inlining {varKind} variable '{Printer.debugDecl def}' because it's never written and has a 'simple' definition"
                             def.name.ToBeInlined <- true
         | _ -> ()
 

src/ast.fs

@@ -196,7 +196,11 @@ type MapEnv = {
     vars: Map<string, Type * DeclElt>
     fns: Map<(string * int), (FunctionType * Stmt) list> // This doesn't support type-based disambiguation of user-defined function overloading
     isInWritePosition: bool // used for findWrites only
-}
+} with
+    member this.withFunction(fct: FunctionType, body, replaceMostRecentOverload) =
+        let oldFnsList = (this.fns.TryFind(fct.prototype) |> Option.defaultValue [])
+        let newFnsList = (fct, body) :: (if replaceMostRecentOverload then oldFnsList.Tail else oldFnsList)
+        {this with fns = this.fns.Add(fct.prototype, newFnsList)}
 
 let mapEnv fe fi = {fExpr = fe; fStmt = fi; vars = Map.empty; fns = Map.empty; isInWritePosition = false}
 
@@ -284,13 +288,24 @@ let mapTopLevel env li =
             let env, res = mapDecl env t
             env, TLDecl res
         | Function(fct, body) ->
-            let oldFns = (env.fns.TryFind(fct.prototype) |> Option.defaultValue [])
-            let envWithFunction = {env with fns = env.fns.Add(fct.prototype, (fct, body) :: oldFns)}
-            let envWithFunctionAndArgs, args = foldList envWithFunction mapDecl fct.args
-            let (fct, body) = { fct with args = args }, snd (mapStmt envWithFunctionAndArgs body)
-            let oldFns = (envWithFunctionAndArgs.fns.TryFind(fct.prototype) |> Option.defaultValue [])
-            let envWithNewFunction = {envWithFunctionAndArgs with fns = env.fns.Add(fct.prototype, (fct, body) :: oldFns.Tail)}
-            envWithNewFunction, Function(fct, body)
+            // Back up the vars without the parameters.
+            let varsWithoutParameters = env.vars
+            // Add the function to env.fns, to have it when transforming the parameters.
+            let env = env.withFunction(fct, body, replaceMostRecentOverload = false)
+            // Transform the parameters and add them to env.vars, to have them when transforming the body.
+            let env, args = foldList env mapDecl fct.args
+            // Update env.fns with the transformed parameters.
+            let fct = { fct with args = args }
+            let env = env.withFunction(fct, body, replaceMostRecentOverload = true)
+
+            // Transform the body. The env modifications (local variables) are discarded.
+            let _, body = mapStmt env body
+            // Update env.fns with the transformed body.
+            let env = env.withFunction(fct, body, replaceMostRecentOverload = true)
+
+            // Remove the parameters from env.vars, so that following functions don't see them.
+            let env = {env with vars = varsWithoutParameters}
+            env, Function(fct, body)
         | e -> env, e)
     res
 

src/options.fs

@@ -92,8 +92,8 @@ module Globals =
     // like printfn when verbose option is set
     let vprintf fmt = fprintf (if options.verbose then stdout else TextWriter.Null) fmt
 
-    let forceDebug = Environment.GetEnvironmentVariable("MINIFIER_DEBUG", EnvironmentVariableTarget.Process ||| EnvironmentVariableTarget.User) <> null
-    let debug str = fprintfn (if options.debug || forceDebug then stdout else IO.TextWriter.Null) "%s" str
+    let forceDebug = Environment.GetEnvironmentVariable("MINIFIER_DEBUG", EnvironmentVariableTarget.User) <> "no"
+    let debug str = if options.debug || forceDebug then printfn "%s" str
 
 open Globals
 

tests/commands.txt

@@ -49,6 +49,7 @@
 --no-remove-unused --format indented -o tests/unit/inline-fn-multiple.expected tests/unit/inline-fn-multiple.frag
 --no-remove-unused --no-renaming --format indented -o tests/unit/simplify.expected tests/unit/simplify.frag
 --no-remove-unused --no-renaming --format indented -o tests/unit/arg-inlining.expected tests/unit/arg-inlining.frag
+--no-remove-unused --no-renaming --format indented -o tests/unit/bug321.expected tests/unit/bug321.frag
 
 # Partial renaming tests
 

tests/unit/bug321.expected

@@ -0,0 +1,13 @@
+int n;
+float a(float s)
+{
+  s=3.;
+  n=2.;
+  return s;
+}
+float main()
+{
+  float t=3.;
+  t-=1.;
+  return a(1.)+a(5.);
+}

tests/unit/bug321.frag

@@ -0,0 +1,14 @@
+int n;
+const float s = 1.;
+float a(float s)
+{
+    s = 3.;
+    n = 2.;
+    return s;
+}
+float main()
+{
+    float t = 3.;
+    t -= s;
+    return a(s)+a(5.);
+}