Skip to content

[lua] Remove unnecessary do...end wrappers around tail returns#12601

Merged
Simn merged 2 commits intoHaxeFoundation:developmentfrom
jdonaldson:genlua-return-cleanup
Mar 4, 2026
Merged

[lua] Remove unnecessary do...end wrappers around tail returns#12601
Simn merged 2 commits intoHaxeFoundation:developmentfrom
jdonaldson:genlua-return-cleanup

Conversation

@jdonaldson
Copy link
Copy Markdown
Member

@jdonaldson jdonaldson commented Feb 13, 2026

Summary

  • Partial fix for Unnecessary output emitted for lua #7350 (addresses do return end wrappers; other codegen issues in that issue remain)
  • When return is the last statement in a function body, the do...end wrapper is unnecessary. This adds a wrap parameter to gen_return and a shared gen_func_block helper that emits bare return at tail position in all four function body handlers.
  • Non-tail returns outside pcall also use bare return.
  • Returns inside pcall callbacks (try-catch) still use do return end since the compiler appends return _hx_pcall_default after user code.

Test plan

  • make haxe builds successfully
  • All 11,711 Lua unit tests pass
  • Verified bare return at function tail positions in generated output
  • Verified do return end still used for returns inside try-catch (pcall)
  • Added regression test (Issue7350) covering both pcall return wrapping and tail return cleanup
  • ~80% reduction in unnecessary do...end wrappers (5,897 → 1,198 in test output)

@jdonaldson
Copy link
Copy Markdown
Member Author

This was mostly a cosmetic issue, and I put it off because it would require refactoring how I render function blocks... everywhere. However, the changes here aren't really so complex, the code winds up being much simpler, so I think it's a good change to make even if the only real benefit is cosmetic.

@tobil4sk
Copy link
Copy Markdown
Member

This looks good! It would give some nice cleanup in #12600.

Early returns (inside if/loop bodies) still use do return end as before.

Could we also deal with these? There are still a lot of cases where it is not needed, for example something like:

final tmp = 10;
if (tmp == 10) {
    return;
}
local tmp = 10;
if (tmp == 10) then 
  do return end;
end;

@skial skial mentioned this pull request Feb 19, 2026
1 task
@jdonaldson
Copy link
Copy Markdown
Member Author

I added a mutable pcall flag to check the status for that situation. It made the change even simpler, how does it look on your end?

@tobil4sk
Copy link
Copy Markdown
Member

how does it look on your end?

Looks much cleaner! :)

function(v) 
  local _hx_1_result_status, _hx_1_result_value = _G.pcall(_G.load, [[return function(v) 
    if ((v <= 2147483647) and (v >= -2147483648)) then 
      if (v > 0) then 
        return _G.math.floor(v);
      else
        return _G.math.ceil(v);
      end;
    end;
    if ((v ~= v) or not ((v > -_G.math.huge) and (v < _G.math.huge))) then 
      return nil;
    end;
    local v = v;
    if (v > 2251798999999999) then 
      v = v * 2;
    end;
    return (v & 0x7FFFFFFF) - (v & 0x80000000);
  end]]);
  local nativeOperators;
  if (_hx_1_result_status and (_hx_1_result_value ~= nil)) then 
    local fn = _hx_1_result_value;
    nativeOperators = fn();
  else
    nativeOperators = nil;
  end;
  local clampImpl = (function() 
    local _hx_2
    if (nativeOperators ~= nil) then 
    _hx_2 = nativeOperators; elseif (_hx_bit_raw == nil) then 
    _hx_2 = function(v) 
      if ((v <= 2147483647) and (v >= -2147483648)) then 
        if (v > 0) then 
          return _G.math.floor(v);
        else
          return _G.math.ceil(v);
        end;
      end;
      if ((v ~= v) or not ((v > -_G.math.huge) and (v < _G.math.huge))) then 
        return nil;
      end;
      local v = v;
      v = (v % 4294967296);
      if (v >= 2147483648) then 
        v = v - 4294967296;
      end;
      return v;
    end; else 
    _hx_2 = function(v) 
      if ((v <= 2147483647) and (v >= -2147483648)) then 
        if (v > 0) then 
          return _G.math.floor(v);
        else
          return _G.math.ceil(v);
        end;
      end;
      if ((v ~= v) or not ((v > -_G.math.huge) and (v < _G.math.huge))) then 
        return nil;
      end;
      local v = v;
      if (v > 2251798999999999) then 
        v = v * 2;
      end;
      local band = _hx_bit_raw.band;
      return band(v, 2147483647) - _G.math.abs(band(v, 2147483648));
    end; end
    return _hx_2
  end )();
  __lua_Boot.clampInt32 = clampImpl;
  return clampImpl(v);
end

I don't really understand the mutable pcall case, perhaps we could add a test for this to demonstrate it and make sure it doesn't regress? (if there isn't one already)

Apart from that this looks good, but it might be good to keep #7350 open (right now it's marked to autoclose) as there are some other generation issues mentioned there unrelated to return end, like the semicolon use.

@Simn
Copy link
Copy Markdown
Member

Simn commented Feb 26, 2026

This needs conflict resolution.

@jdonaldson
Copy link
Copy Markdown
Member Author

Yeah good idea.

@jdonaldson jdonaldson force-pushed the genlua-return-cleanup branch from b07df7a to 6eff185 Compare February 26, 2026 06:45
When a return is the last statement in a function body, it doesn't
need to be wrapped in do...end. Add a wrap parameter to gen_return
and pass false at tail position in all four function body handlers.
Track pcall context with in_pcall flag so do...end is only used where
Lua's grammar requires it (inside pcall bodies with sentinel returns).
@jdonaldson jdonaldson force-pushed the genlua-return-cleanup branch from 6eff185 to fa72152 Compare March 1, 2026 17:33
@Simn Simn merged commit b9a82d4 into HaxeFoundation:development Mar 4, 2026
50 checks passed
@Simn Simn deleted the genlua-return-cleanup branch March 4, 2026 07:32
@tobil4sk
Copy link
Copy Markdown
Member

tobil4sk commented Mar 4, 2026

I don't really understand the mutable pcall case, perhaps we could add a test for this to demonstrate it and make sure it doesn't regress? (if there isn't one already)

Could we still add this test ?

Also could we reopen the issue at #7350, this only addresses part of it.

@jdonaldson
Copy link
Copy Markdown
Member Author

Follow-up: added the regression test for the pcall return case in #12740. Also reopened #7350 since this PR only addressed the do return end wrappers.

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.

3 participants