Skip to content

[lua] Port _hx_bit_clamp to haxe code#12600

Open
tobil4sk wants to merge 14 commits intoHaxeFoundation:developmentfrom
tobil4sk:fix/lua-clamp-haxe
Open

[lua] Port _hx_bit_clamp to haxe code#12600
tobil4sk wants to merge 14 commits intoHaxeFoundation:developmentfrom
tobil4sk:fix/lua-clamp-haxe

Conversation

@tobil4sk
Copy link
Member

@tobil4sk tobil4sk commented Feb 12, 2026

We can now rely on normal dce so that the code for this function isn't generated unless required. It also provides more control over what code is generated, which has allowed adding checks like lua_ver >= 5.3 to avoid redundant compatibility implementations.

This also allows compile time abstractions that aren't supported on lua, like inline function calls, which reduce duplicate code.

The generic implementation checks what is supported to decide which implementation to use, then patches the method to avoid having to rerun the feature detection every time. This can't go into static init because bitwise operations are needed in regex constructors, which are called in some static initialisers. I think this way is also fairly self contained which is nice.

Currently a test is failing because __lua_Boot.clampInt32 is nil, I think my check @:ifFeature("use._bitop") isn't sufficient.

Below is the generated lua code, which could be improved, but I'm surprised at how close it is to the original!

Expand to view
__lua_Boot.clampInt32 = 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 
        do return _G.math.floor(v) end;
      else
        do return _G.math.ceil(v) end;
      end;
    end;
    if ((v ~= v) or not ((v > -_G.math.huge) and (v < _G.math.huge))) then 
      do return nil end;
    end;
    local v = v;
    if (v > 2251798999999999) then 
      v = v * 2;
    end;
    do return (v & 0x7FFFFFFF) - (v & 0x80000000) end;
  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 
          do return _G.math.floor(v) end;
        else
          do return _G.math.ceil(v) end;
        end;
      end;
      if ((v ~= v) or not ((v > -_G.math.huge) and (v < _G.math.huge))) then 
        do return nil end;
      end;
      local v = v;
      v = (v % 4294967296);
      if (v >= 2147483648) then 
        v = v - 4294967296;
      end;
      do return v end;
    end; else 
    _hx_2 = function(v) 
      if ((v <= 2147483647) and (v >= -2147483648)) then 
        if (v > 0) then 
          do return _G.math.floor(v) end;
        else
          do return _G.math.ceil(v) end;
        end;
      end;
      if ((v ~= v) or not ((v > -_G.math.huge) and (v < _G.math.huge))) then 
        do return nil end;
      end;
      local v = v;
      if (v > 2251798999999999) then 
        v = v * 2;
      end;
      local band = _hx_bit_raw.band;
      do return band(v, 2147483647) - _G.math.abs(band(v, 2147483648)) end;
    end; end
    return _hx_2
  end )();
  __lua_Boot.clampInt32 = clampImpl;
  do return clampImpl(v) end;
end

Related to #8852.

We can now rely on normal dce so that the code for this function isn't
generated unless required. It also provides more control over what code
is generated.

This also allows compile time abstractions that aren't supported on lua,
like inline function calls, which reduce duplicate code.
This is a simpler operation which makes more sense here given the range
of values this.length can have.

Array fails to be dce'd in hello world, so removing this avoids clamp
from being included.
In clampInt32, these are used before static init runs, which means they
are set to nil
If lua 5.3+ is being targetted, we know that native bitwise operators
are available, so we can use them.

On lua 5.2, we know that bit32 is available so we don't need to generate
a fallback for it being null.
@skial skial mentioned this pull request Feb 12, 2026
1 task
@jdonaldson
Copy link
Member

I looked into why @:ifFeature("use._bitop") doesn't work here. The root cause is that use._bitop has two separate systems that don't connect:

Code generation time (after DCE): genlua.ml's gen_bitop calls add_feature ctx "use._bitop" when compiling a & b_hx_bit.band(a, b). This tells genlua.ml to include _hx_bit.lua, but DCE has already finished by this point.

DCE time: @:ifFeature("use._bitop") is only triggered when DCE encounters __define_feature__("use._bitop", ...). The only place that exists is Bit.__init__() — which only runs if someone explicitly uses the lua.Bit class. Normal Haxe bitwise operators (a & b, a | b) are compiled directly by genlua.ml and never touch lua.Bit, so DCE never sees the feature being defined.

Fix: Remove @:ifFeature from clampInt32. Normal DCE tracing should keep it alive through its callers:

  • Std.int()Boot.clampInt32()
  • Boot.__instanceof()clampInt32()

If nothing calls clampInt32, DCE removes it naturally — which is correct.

Also worth noting: _hx_bit.lua (raw Lua) calls the global _hx_bit_clamp directly, not __lua_Boot.clampInt32. So if _hx_bit_clamp.lua is removed in favor of the Haxe port, the bit32 fallback path in _hx_bit.lua would need updating too (either assign _hx_bit_clamp = __lua_Boot.clampInt32, or port _hx_bit.lua to Haxe as well).

Previously use._bitop was being set during the generator phase, at which
point dce already occurred so @:ifFeature on lua.Boot.clampInt32 didn't
work.

This patch moves use._bitop detection to the dce phase, which means it
will be set in time to make sure clampInt32 is included when needed.
-Min_Int32 is "optimised" into -2147483648 due to overflow
@tobil4sk
Copy link
Member Author

Code generation time (after DCE): genlua.ml's gen_bitop calls add_feature ctx "use._bitop" when compiling a & b → _hx_bit.band(a, b). This tells genlua.ml to include _hx_bit.lua, but DCE has already finished by this point.

This analysis seems correct, but claude was a bit too confident about the solution.

I did some digging and it seems like python already deals with the same problem: #11427. I lifted the solution from there, and the test seems to pass now.

The clamp port caused a regression in underflow, but it wasn't caught
due to this missing test case.
Haxe modulo generates as math.fmod, which rounds towards 0 instead of
minus infinity. This breaks underflow.

This requires adding lua.Syntax.modulo. Documentation is taken from:
https://www.lua.org/manual/5.5/manual.html#3.4.1
Otherwise, the _hx_bit file will always be omitted when dce is off.
This allows the incomplete clamp implementations to be marked as extern
inline, which prevents them from being generated outside of clampInt32.
This is important especially for clampNativeOperator, which does not
parse on lua < 5.3 so it cannot be generated outside
testFunctionSupport.

@:noPackageRestrict is needed here for the macro to work within the
lua.Boot module.
@jdonaldson
Copy link
Member

Heya, I like your DCE fix and the haxe-side clampInt32. I can adapt to it on my work for #12599. I'm going to mark that one as draft, expecting this to get merged first, FWIW.

@tobil4sk
Copy link
Member Author

I tried to avoid making conflicts with that PR, because I was hoping they can be merged together. It would be good if we also port _hx_bit to haxe, but having the native operators as they are now in #12599 would be a good start as it unblocks lua 5.5 support, and they shouldn't conflict.

So maybe we can merge both of these for now and then handle further _hx_bit clean up in a new PR?

As an aside, I've also been thinking in general about int behaviour. Int32 is the type that needs to comply with strict overflow rules, but for regular Int, haxe describes that as a type with platform specific behaviour so I wonder if we can avoid some of the overhead by making the behaviour closer to native lua. Ideally, we should be able to generate native operators directly in most cases, with these wrappers mainly for when strict int32 behaviour is needed.

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.

4 participants