bakpakin / Fennel

Lua Lisp Language
https://fennel-lang.org
MIT License
2.48k stars 126 forks source link

Key/value table order doesn't seem deterministic #449

Closed mitchellwrosen closed 1 year ago

mitchellwrosen commented 1 year ago

Hi there,

I've noticed that I tend to get a seem every time I compile my neovim fennel configs to lua. Here's one:

-vim.api.nvim_create_autocmd("LspAttach", {callback = _73_, group = "mitchellwrosen"})
+vim.api.nvim_create_autocmd("LspAttach", {group = "mitchellwrosen", callback = _73_})

I can try to provide a smaller repro if desired. Here are some instructions for now:

Fennel version: Fennel 1.3.0 on PUC Lua 5.4

Thanks!

technomancy commented 1 year ago

Yeah, that looks like a regression. Thanks for catching it. Will see if we can get a fix in for the next release.

jaawerth commented 1 year ago

@mitchellwrosen As a temporary workaround until we get a release out, this should be fixable by compiling with luajit or PUC 5.1 instead of 5.4; you can just add a --lua luajit to your compile commands, assuming it's installed on the machine where you do your compiling.

In Lua >= 5.2, every time the Lua VM instantiates, it generates a random nonce to use in the hash part of lua tables to prevent collision attacks, which makes the order of pairs unstable for non-integer keys. What I'm fairly certain you've run into here is a particular edge case in our guard against that, which crop up when a macro modifies a table literal on Lua >= 5.2.

At any rate, once I spotted where the problem I couldn't resist getting a fix going before running out of steam for the evening, so it should be pushed within a few days - just need to wrap it up and make sure there's proper test coverage moving forward.

jaawerth commented 1 year ago

@mitchellwrosen The latest commit on main now has the fix - feel free to let us know if you still have this issue! Otherwise, there are only one or two things outstanding for the next release, so it shouldn't be long before it hits if you don't want to build from source.