Skip to content

[lua] Add regression test for pcall return wrapping#12740

Open
jdonaldson wants to merge 2 commits into
HaxeFoundation:developmentfrom
jdonaldson:lua-pcall-return-test
Open

[lua] Add regression test for pcall return wrapping#12740
jdonaldson wants to merge 2 commits into
HaxeFoundation:developmentfrom
jdonaldson:lua-pcall-return-test

Conversation

@jdonaldson
Copy link
Copy Markdown
Member

Summary

  • Follow-up to [lua] Remove unnecessary do...end wrappers around tail returns #12601
  • Adds a regression test (Issue7350) for the in_pcall guard that ensures do return end wrapping is preserved inside try-catch blocks (pcall callbacks), where the compiler appends return _hx_pcall_default after user code.
  • Also tests that tail returns use bare return without wrapping.

Test plan

  • make haxe builds successfully
  • All 11,711 Lua unit tests pass

return -1;
}
return 0;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this function, haxe currently generates:

___Main_Main_Fields_.returnInTryCatch = function(x) 
  local _hx_status, _hx_result = pcall(function() 
  
      if (x > 0) then 
        do return x * 2 end;
      end;
    return _hx_pcall_default
  end)
  if not _hx_status then 
    local _g = _hx_result;
    return -1;
  elseif _hx_result ~= _hx_pcall_default then
    return _hx_result
  end;
  return 0
end

from what I can tell in this case the do end doesn't make much of a difference?

Comment thread tests/unit/src/unit/issues/Issue7350.hx Outdated
eq(10, returnInTryCatch(5));
eq(0, returnInTryCatch(-1));

// Tail return should not use do...end wrapper
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't really do much, because even if there is a do ... end it's still going to pass. It's most useful to test if there is a case where the compiler missing out do ... end would result in broken output.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point — rewrote the test to exercise actual pcall return propagation edge cases instead. Now covers returning null/false/zero through pcall (must not be confused with _hx_pcall_default sentinel), fall-through where try body has no return (code after try-catch must execute), nested try-catch, try-catch in loops, and exception handling paths.

Tests that return inside try-catch still uses do...end wrapper
(since pcall callback has appended return _hx_pcall_default),
while tail returns use bare return.
@jdonaldson jdonaldson force-pushed the lua-pcall-return-test branch from 5d8a46e to 90ca592 Compare March 9, 2026 20:02
Address review feedback: old test had returns inside if-blocks where
do...end wrapping was redundant and tests passed either way. New test
covers null/false/zero return propagation through pcall, fall-through
vs return distinction via _hx_pcall_default sentinel, nested try-catch,
try-catch in loops, and exception handling paths.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants