~technomancy/fennel#144: Macro wont output/expand expressions in seq without preceeding expression

(macro wont-output-body []
  ;; let also fails to output
  (local body [`(do (+ 1 1) (values 1))
               `(do (+ 2 2) (values 2))])
  `(fn []
     ,body
     (values :val)))

(macro will-output-body []
  (local body [`(do (+ 1 1) (values 1))
               `(do (+ 2 2) (values 2))])
  `(fn []
     nil ;; can be any expression
     ,body
     ;; (do ,body) also works
     (values :val)))

(comment the following WONT output body statements)
(wont-output-body)

(comment the following WILL output body statements)
(will-output-body)

(macro just-body []
  (local body [`(do (+ 1 1) (values 1))
               `(do (+ 2 2) (values 2))])
  body)

(comment otherwise this normally works)
(just-body)

--[[ the following WONT output body statements ]]
local function _1_()
  return "val"
end
--[[ the following WILL output body statements ]]
local function _2_()
  local _3_
  do
    do local _ = (1 + 1) end
    _3_ = 1
  end
  local function _4_()
    do local _ = (2 + 2) end
    return 2
  end
  do local _ = {_3_, _4_()} end
  return "val"
end
--[[ otherwise this normally works ]]
local _5_
do
  do local _ = (1 + 1) end
  _5_ = 1
end
local function _6_(...)
  do local _ = (2 + 2) end
  return 2
end
return {_5_, _6_(...)}

Status: RESOLVED FIXED
Submitter: ~rktjmp
Assigned to: No-one
Submitted: 6 months ago
Updated: 5 months ago
Labels: No labels applied.

~technomancy added bug 6 months ago

~andreyorst 6 months ago

This has to do with the fact that the sole table right after the argument list is interpreted as a metatable table, e.g.:
(fn foo [...]
  {:fnl/docstring "foo" :fnl/arglist [&optional bar baz]}
  (match ...
    (bar baz) (do bar baz)
    (bar) (do bar)
    _ (do :nothing)))
Because the table itself is just a value and is ignored by the function body completely, similarly to the string as a first value in the function, we treat it specially.

I guess the Fennel compiler should explicitly check that this table is a hash table and not a sequence and that it is not empty, and only then treat it specially.

(We have to treat it specially, because, as can be seen above, :fnl/arglist [&optional bar baz] refers to free symbols, so this also will compile without any error (fn foo [] [abc] nil), while this won't (fn foo [] nil [abc]))

~andreyorst 6 months ago*

On a side note, it's better to construct body as a (do) list, instead of a table literal:
   (macro will-output-body []
     (let [body `(do (do (+ 1 1) (values 1))
                     (do (+ 2 2) (values 2)))]
       `(fn []
          ,body
          (values :val))))
It is still possible to do (table.insert body '(more stuff)) because lists are also sequential tables.

~technomancy 6 months ago

Yeah, this is not a bug, but it is intentional behavior which it's easy to trigger by accident, because this form is somewhat obscure.

I wonder if we could find a way to make this more obvious; maybe flagging it as a compile error if it's clear that it's not intended as metadata? Like perhaps a table which has only numeric keys? (unfortunately we can't use utils.sequence? here because the table might not come from the parser but from a macro like in the case above)

~technomancy removed bug 5 months ago

~xerool 5 months ago

This is surely bug when the table produces side effects.
(fn foo []
  [(print :hello)]
  nil)

(foo) ;; no print

~andreyorst 5 months ago

I wonder if we could find a way to make this more obvious

I guess this is why Clojure put metadata table before arglist

~andreyorst 5 months ago

I've sent a patch that should fix this https://lists.sr.ht/~technomancy/fennel/patches/37451

~technomancy REPORTED FIXED 5 months ago

Applied this patch; so this is fixed now; thanks Andrey!