bakpakin/Fennel

Table literals modified by macros don't get all their fields emitted

datwaft opened this issue · 2 comments

Environment

$ fennel --version
Fennel 1.1.0 on PUC Lua 5.4 

How reproduce the issue

1. Create a new file containing the following content:

(macro highlight! [name attributes colors]
  (let [name (tostring name)
        colors (collect [_ v (ipairs attributes) :into colors] (tostring v) true)]
    `(vim.api.nvim_set_hl 0 ,name ,colors)))

(macrodebug (highlight! MatchParen [:underline] {:bg "#262626"}))
(highlight! MatchParen [:underline] {:bg "#262626"})

2. Execute the following command:

fennel --globals vim -c <filename>

Expected result

The compilation result must be equals to the macrodebug result:

print("(vim.api.nvim_set_hl 0 \"MatchParen\" {:bg \"#262626\" :underline true})")
return vim.api.nvim_set_hl(0, "MatchParen", {bg = "#262626" :underline true})

Current result

The compilation result differs from the macrodebug result:

print("(vim.api.nvim_set_hl 0 \"MatchParen\" {:bg \"#262626\" :underline true})")
return vim.api.nvim_set_hl(0, "MatchParen", {bg = "#262626"})

This is a really interesting one! It actually has nothing to do with
macrodebug, other than the fact that the bug happens during
compilation to lua, so macrodebug isn't affected.

What happens is that the parser puts a keys field in a metatable on
every key/value table literal it reads. This allows the table to be
reconstructed exactly as it was originally in the input; even though
key/value tables are un-ordered in their representation, we can emit
them with their keys in the original order.

The problem is that a macro can modify a table in a way that doesn't
actually change the separate keys table. When the compiler emits a
Lua table, currently it only pays attention to keys and if it
disagrees with the actual value of the table itself, the keys table
wins.

The correct behavior is to use keys only for ordering: we must ignore
entries which don't exist in the table, and we must ensure that
legitimate keys which aren't found in keys are still emitted in the Lua.

Anyway the best solution here of course is to not use a macro; this
code is much better suited for a function. But it is a legitimate bug in
the compiler, so I'll put together a fix for it. In the mean time if you
can't replace it with a function without breaking compatibility, you can
work around the bug by avoiding the use of :into and copying fields
manually for the time being.

Thanks for finding this.

Should be fixed in 5596e09.