Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use bitwise operators for Lua 5.4 #18

Open
mattwidmann opened this issue Jul 1, 2020 · 6 comments
Open

Use bitwise operators for Lua 5.4 #18

mattwidmann opened this issue Jul 1, 2020 · 6 comments

Comments

@mattwidmann
Copy link

The bit manipulation library has been removed in Lua 5.4, so md5.lua falls back to its custom bit manipulation functions, using tables. It would be better if md5.lua knew how to use the bitwise operators that were added in Lua 5.3.

@pablomayobre
Copy link
Contributor

The biggest issue I can see with this is that the bitwise operator syntax introduced in Lua 5.3+ is not backwards compatible. You need to wrap any part of the code that uses it in a string, and then load it with loadstring for it to work properly.

@VADemon
Copy link

VADemon commented Jul 2, 2020

wrap any part of the code that uses it in a string, and then load it with loadstring for it to work properly.

This or split the functions in files and load them depending on the detected Lua version. But that'd make the "single library - single file" use a little less convenient.

Input needed I would say.

@mattwidmann
Copy link
Author

Would something like this work?

local bit_53 = loadstring[[
return {
  bor = function (x, y) return x | y end,
  band = function (x, y) return x & y end,
}
]]
if bit_53 then
  bit = bit_53()
end

@VADemon
Copy link

VADemon commented Jul 2, 2020

Yes exactly, that's what the discussion is about, but my criticims:

  1. Functions add a level of indirection which lead to lower perf without JIT inlining - you would gain nothing from bitwise operators and still have a function call.
  2. loadstring may not be available - another "if branch" of code to adapt
  3. After the code is initialized (once!) it should be gone from memory. Doesn't happen without explicit nil in your toy example
  4. If we used require/dofile instead - again, may not be available - and it's a little more inconvenient to use/ship for novice users.

@mattwidmann
Copy link
Author

mattwidmann commented Jul 11, 2020

Those are valid, thoughtful points.

Here's another proposal: MD5 hashes are a known quantity and this particular implementation hasn't seen an update in 2 years. How about, going forward, new versions of md5.lua require 5.3 or later and this project can just use the bitwise operators directly? And prior versions can use the current version of the library?

@pablomayobre
Copy link
Contributor

Lua 5.1 is the most used Lua version out there, due to LuaJIT so I'd say you either make a new library or just make this one support the new syntax somehow, you could potentially make two files with API compatibility.

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

No branches or pull requests

3 participants