hotpot.nvim icon indicating copy to clipboard operation
hotpot.nvim copied to clipboard

Unknown behaviour for when calling require(:macros) inside import-macros call

Open rktjmp opened this issue 2 years ago • 5 comments

Related: #76

Work draft:

  • create test case
    • a.fnl (import-macros b.fnl) b.fnl (require c.fnl) c.fnl (x ,some ,mac)?
      • what is the goal? to use a macro from c, inside b? or to use a module c inside b?
        • did we not solve this with libdonut previously? check how we call internal macros there.
      • validate cljlibs intention, do you understand the goal & problem? cljlib/init-macros.fnl#858
        • 28/8: Imports lazy-seq macro to call that macro inside a macro to output the output of the imported macro :)
          • import-macros x lazy-seq; mac-a [] `(... ,(x ...)`
            
    • how does this behave in fennel cli?
      • 1.1.0
      • 1.2.0
        • eval:yes, compile: yes
    • how does this behave in hotpot?
      • different?
    • Is the behaviour expected, useful or aberrant?
      • previous discussion from technomancy on matrix implied that requiring modules from macros was unsupported (due to unknown behaviour with sandboxing?)

rktjmp avatar Aug 23 '22 06:08 rktjmp

;; specials.fnl#1099
;; If the compiler sandbox is disabled, we need to splice in the searcher
;; so macro modules can load other macro modules in compiler env.
(fn dofile-with-searcher [fennel-macro-searcher filename opts ...]
  (let [searchers (or package.loaders package.searchers {})
        _ (table.insert searchers 1 fennel-macro-searcher)
        m (utils.fennel-module.dofile filename opts ...)]
    (table.remove searchers 1)
    m))

(fn fennel-macro-searcher [module-name]
  (let [opts (doto (utils.copy utils.root.options)
               (tset :module-name module-name)
               (tset :env :_COMPILER)
               (tset :requireAsInclude false)
               (tset :allowedGlobals nil))]
    (match (search-module module-name utils.fennel-module.macro-path)
      filename (values (if (= opts.compiler-env _G)
                           (partial dofile-with-searcher fennel-macro-searcher
                                    filename opts)
                           (partial utils.fennel-module.dofile filename opts))
                       filename))))

"if opts == _G, then use macro searcher otherwise eval as fennel", but sandbox should be enabled by default? So should not hit that path? confirmed.

rktjmp avatar Aug 28 '22 04:08 rktjmp

fennel-macro-searcher   module-name     clj-req
fennel-macro-searcher   module-name     clj-req.lazy-seq.init-macros
user.fnl:: calling clj-lib function
clj-req/init.fnl:: Hello from clj-req.init.function, I will call clj-mac next.
clj-req/init-macros.fnl:: clj-req.init-macros.clj-req-mac called with i-am-from-function
lazy-seq over a,b =>    10      20

rktjmp avatar Aug 28 '22 04:08 rktjmp

;; specials.fnl#1086
(fn make-searcher [?options]
  "This will allow regular `require` to work with Fennel:
table.insert(package.loaders or package.searchers, fennel.searcher)"
  (fn [module-name]
    (let [opts (utils.copy utils.root.options)]
      (each [k v (pairs (or ?options {}))]
        (tset opts k v))
      (set opts.module-name module-name)
      (match (search-module module-name)
        filename (values (partial utils.fennel-module.dofile filename opts)
                         filename)
        (nil error) error))))

Possibly because fennel.bin always patches lua search path, and this searcher dofiles instead of compiles, some options are inherited from the previous macro search?

rktjmp avatar Aug 28 '22 04:08 rktjmp


fennel-macro-searcher-fn-is     function: 0x55eb88b56f70
using-make-searcher     clj-req nil
fennel-macro-searcher   module-name     clj-req {:opts {:env "_COMPILER"
        :filename "./clj-req/init.fnl"
        :ignore-options true
        :indent "  "
        :module-name "clj-req"
        :plugins {}
        :requireAsInclude false}
 :package.searchers [#<function: 0x55eb88a9f930>
                     #<function: 0x55eb88a9f970>
                     #<function: 0x55eb88a9f9b0>
                     #<function: 0x55eb88a9f9f0>
                     #<function: 0x55eb88bb5ef0>]}
fennel-macro-searcher   module-name     clj-req.lazy-seq.maccas {:opts {:env "_COMPILER"
        :filename "./clj-req/init.fnl"
        :ignore-options true
        :indent "  "
        :module-name "clj-req.lazy-seq.maccas"
        :plugins {}
        :requireAsInclude false}
 :package.searchers [#<function: 0x55eb88a9f930>
                     #<function: 0x55eb88a9f970>
                     #<function: 0x55eb88a9f9b0>
                     #<function: 0x55eb88a9f9f0>
                     #<function: 0x55eb88bb5ef0>]}
fennel-macro-searcher   module-name     clj-req.lazy-seq.module {:opts {:env "_COMPILER"
        :filename "./clj-req/init.fnl"
        :ignore-options true
        :indent "  "
        :module-name "clj-req.lazy-seq.module"
        :plugins {}
        :requireAsInclude false}
 :package.searchers [#<function: 0x55eb88a9f930>
                     #<function: 0x55eb88a9f970>
                     #<function: 0x55eb88a9f9b0>
                     #<function: 0x55eb88a9f9f0>
                     #<function: 0x55eb88bb5ef0>]}
just a normal fn
user.fnl:: calling clj-lib function
clj-req/init.fnl:: Hello from clj-req.init.function, I will call clj-mac next.
clj-req/init-macros.fnl:: clj-req.init-macros.clj-req-mac called with i-am-from-function
lazy-seq over a,b =>    10      20

_COMPILER env is retained through all requires inside an import-macros call. This "works" because macros are functions that output a list, so you can mix and match "normal" functions in that context and call either kind interchangeably.

rktjmp avatar Aug 28 '22 05:08 rktjmp

Probably:

  • We only insert a module searcher into lua runtime
  • When we find a fnl module, we compile it and insert the macro searcher into fennel.macro-searcher but not into the package searchers. The module is compiled, any import-macros calls end up hitting the macro searcher, which evals as normal, but the eval will hit a require which only uses the regular searcher, which tries to compile the file, and we get the error.
  • Fix is probably to inject the macro searcher into the normal searchers when evalling (then pop it off afterwards), this should allow the macro searcher to find files first and kick through to eval again.
    • Side effects:
      • Wont compile out any module that a macro requires, wont track dependencies of these modules
      • Watch for recursive eval loops too
  • Options:
    • Reconsider "module vs macro" searcher for "caching vs non-caching" searcher which draws a clearer distinction in behaviour
      • Though the "non-caching/macro" has different options (at least env = _COMPILER) so ... :shrug:
    • Confirm that this is actually intended behaviour, esp with regards to the propagation of env option into normal modules (probably?).

rktjmp avatar Aug 28 '22 05:08 rktjmp