kikito / md5.lua

MD5 sum in pure Lua, with no C and no external dependencies
MIT License
326 stars 151 forks source link

Use bitwise operators for Lua 5.4 #18

Open mattwidmann opened 4 years ago

mattwidmann commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.