Fix for ZAM inlining of nested function calls with the same parameter names

src/script_opt/Expr.cc

@@ -2529,22 +2529,41 @@ ExprPtr InlineExpr::Duplicate() {
 bool InlineExpr::IsReduced(Reducer* c) const { return NonReduced(this); }
 
 ExprPtr InlineExpr::Reduce(Reducer* c, StmtPtr& red_stmt) {
-    // First, reduce each argument and assign it to a parameter.
+    // We have to be careful regarding the order in which we evaluate
-    // We do this one at a time because that will often allow the
+    // the various elements that go into inlining the call. First, the
-    // optimizer to collapse the final assignment.
+    // arguments need to be reduced in the current scope, not the block
+    // we'll create for the inlined code. Second, we need to generate the
+    // identifiers for the formal parameters *after* creating that inner
+    // block scope, so the variables are distinct to that context. Finally,
+    // when done we need to create the return variable within that scope
+    // (so it's unique to the inlined instance) even though we'll use it -
+    // and possibly other locals from the inlining, via optimization - in
+    // the outer scope.
+
+    auto args_list = args->Exprs();
+    std::vector<ExprPtr> red_args; // holds the arguments as singletons
 
     red_stmt = nullptr;
 
-    auto args_list = args->Exprs();
+    // Gather up the reduced arguments.
-
     loop_over_list(args_list, i) {
         StmtPtr arg_red_stmt;
-        auto red_i = args_list[i]->Reduce(c, arg_red_stmt);
+        red_args.emplace_back(args_list[i]->Reduce(c, arg_red_stmt));
-        auto assign_stmt = with_location_of(c->GenParam(params[i], red_i, param_is_modified[i]), this);
+        red_stmt = MergeStmts(red_stmt, arg_red_stmt);
-        red_stmt = MergeStmts(red_stmt, arg_red_stmt, assign_stmt);
     }
 
-    auto ret_val = c->PushInlineBlock(type);
+    // Start the inline block, so the parameters we generate pick up
+    // its naming scope.
+    c->PushInlineBlock();
+
+    // Generate the parameters and assign them to the reduced arguments.
+    loop_over_list(args_list, j) {
+        auto assign_stmt = with_location_of(c->GenParam(params[j], red_args[j], param_is_modified[j]), this);
+        red_stmt = MergeStmts(red_stmt, assign_stmt);
+    }
+
+    // Generate the return variable distinct to the inner block.
+    auto ret_val = c->GetRetVar(type);
     if ( ret_val )
         ret_val->SetLocationInfo(GetLocationInfo());
 

src/script_opt/Reduce.cc

@@ -359,11 +359,27 @@ NameExprPtr Reducer::GenInlineBlockName(const IDPtr& id) {
     return make_intrusive<NameExpr>(GenLocal(id));
 }
 
-NameExprPtr Reducer::PushInlineBlock(TypePtr type) {
+void Reducer::PushInlineBlock() {
     ++inline_block_level;
-
     block_locals.emplace_back(std::unordered_map<const ID*, IDPtr>());
+}
 
+void Reducer::PopInlineBlock() {
+    --inline_block_level;
+
+    for ( auto& l : block_locals.back() ) {
+        auto key = l.first;
+        auto prev = l.second;
+        if ( prev )
+            orig_to_new_locals[key] = prev;
+        else
+            orig_to_new_locals.erase(key);
+    }
+
+    block_locals.pop_back();
+}
+
+NameExprPtr Reducer::GetRetVar(TypePtr type) {
     if ( ! type || type->Tag() == TYPE_VOID )
         return nullptr;
 
@@ -382,21 +398,6 @@ NameExprPtr Reducer::PushInlineBlock(TypePtr type) {
     return GenInlineBlockName(ret_id);
 }
 
-void Reducer::PopInlineBlock() {
-    --inline_block_level;
-
-    for ( auto& l : block_locals.back() ) {
-        auto key = l.first;
-        auto prev = l.second;
-        if ( prev )
-            orig_to_new_locals[key] = prev;
-        else
-            orig_to_new_locals.erase(key);
-    }
-
-    block_locals.pop_back();
-}
-
 ExprPtr Reducer::NewVarUsage(IDPtr var, const Expr* orig) {
     auto var_usage = make_intrusive<NameExpr>(var);
     BindExprToCurrStmt(var_usage);

src/script_opt/Reduce.h

@@ -57,11 +57,15 @@ public:
 
     int NumNewLocals() const { return new_locals.size(); }
 
+    // These should be used as a balanced pair to start and end a
+    // block being inlined.
+    void PushInlineBlock();
+    void PopInlineBlock();
+
     // Returns the name of a temporary for holding the return
     // value of the block, or nil if the type indicates there's
-    // o return value.
+    // no return value. Call before popping the block.
-    NameExprPtr PushInlineBlock(TypePtr type);
+    NameExprPtr GetRetVar(TypePtr type);
-    void PopInlineBlock();
 
     // Whether it's okay to split a statement into two copies for if-else
     // expansion.  We only allow this to a particular depth because