[tlively]

When entering a nested control flow scope whose input parameters are scratch local.gets from the enclosing scope's inputs, reuse the enclosing scope's scratch locals directly instead of allocating new scratch locals. Share the movement logic in pushScope with moving legacy EH pops into nested unnamed blocks.

[gemini-code-assist]

Code Review

This pull request optimizes input parameter handling in IRBuilder::pushScope by allowing pops to move into unnamed blocks and reusing scratch locals from enclosing scopes instead of allocating new ones. It also simplifies pop validation in IRBuilder::makePop and adds comprehensive tests for block tunneling and misplaced pop parse errors. The review feedback points out a critical correctness bug in pushScope where isInputGet fails to verify that the LocalGet index matches the parent local's index, which could trigger an assertion failure or cause incorrect optimization. Additionally, the reviewer suggests wrapping the parameter matching loop in an if (hasInput) block to prevent potential out-of-bounds indexing of Type::none when hasInput is false.

[github-actions]

🤖 Google Antigravity Code Review

I have reviewed the changes in this Pull Request and left inline feedback.

[aheejin]

It looks this also contains changes in #9336. Are you going to rebase?

[tlively]

Yes, merged latest changes now.

[tlively]

Friendly ping @aheejin

[aheejin]

1. Can we split the pop handling code and the scratch local tunneling code? They don't share much other than pushSynthetic at the end, and to me merging the two code paths makes it harder to read.

2. Currently this code is accepted without an error:

(module
  (tag $e (param i64))
  (func $test
    try
    catch $e
      block (param i32)  ;; This should error out because the tag type is i64
        drop
      end
    end
  )
)

3. Can we have a test case that tests double tunneling too, so that we can confirm we only create a single scratch local for the below?

(func $block-double-tunnel (result i32)
  i32.const 0
  block (param i32) (result i32)
    block (param i32) (result i32)
      block (param i32) (result i32)
      end                                                                                                                                                                                                                         
    end                                                                                                                                                                                                                           
  end                                                                                                                                                                                                                             
)    

[tlively]

1. Can we split the pop handling code and the scratch local tunneling code? They don't share much other than pushSynthetic at the end, and to me merging the two code paths makes it harder to read.

Yes, this makes things much simpler. Good idea.

3. Currently this code is accepted without an error:
   (module
   (tag $e (param i64))
   (func $test
   try
   catch $e
   block (param i32) ;; This should error out because the tag type is i64
   drop
   end
   end
   )
   )

Good catch, fixed.

3. Can we have a test case that tests double tunneling too, so that we can confirm we only create a single scratch local for the below?

(func $block-double-tunnel (result i32)
i32.const 0
block (param i32) (result i32)
block (param i32) (result i32)
block (param i32) (result i32)
end
end
end
)

Done, and added some other interesting test cases as well.