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

Bogus diagnostics in macro modules?

Open kwshi opened this issue 2 years ago • 4 comments

The Fennel manual describes using macro modules to export commonly re-used macros. In macro modules, macros are defined not using (macro) but rather as plain (fn) functions. This leads to hotpot mistakenly treating macro code as "run-time" (rather than "compile-time") code, reporting errors such as

  • "unknown-identifier: assert-compile"
  • "symbols may only be used at compile time"

etc.

Is there some way to configure hotpot so that it knows how to distinguish between regular (run-time) modules vs. macro (compile-time) modules?

kwshi avatar Aug 19 '22 20:08 kwshi

There is https://github.com/rktjmp/hotpot.nvim/blob/b942e8760ea26f6ff3782f675a8d6c1323f3e7d4/fnl/hotpot/api/diagnostics.fnl#L118-L126 (under :h hotpot.api.diagnostics.set-options but the docs are lacking, PR's welcome).

You should be able to call that with macro compiler options and it will call the handler internally to update the current diagnostics, the options are persisted until detachment so you should only need to call this once. I haven't tested this rigorously, but that's the idea from memory.

I would either create a bind, or auto-cmd that calls that for files you consider "macro files".

See also :h hotpot-setup for the default compiler options to pass for macros.

The options will override what's set here:

https://github.com/rktjmp/hotpot.nvim/blob/b942e8760ea26f6ff3782f675a8d6c1323f3e7d4/fnl/hotpot/api/diagnostics.fnl#L71-L72

So you may want to pass in a filename and allowed globals in addition to any env options.

I don't think there is a really safe way to detect "macro modules", besides perhaps if they are called init-macros.fnl which is why we don't set anything like this automatically currently.

rktjmp avatar Aug 20 '22 05:08 rktjmp

Thanks! This set-options thing seems useful for sidestepping the "unknown identifier: assert-compile" error, but it seems to have no effect on the "symbols may only be used at compile time" issue (presumably, since that's not an unresolved global variable issue).

For reference, here's what I ran (as a vim command, via lua):

lua require 'hotpot.api.diagnostics'['set-options'](0, { env = '_COMPILER' })

Which, per the Fennel API docs, "will cause the code to be run/compiled in a context which has all compiler-scoped values available. This can be useful for macro modules or compiler plugins."

Upon running that, the "unknown identifier: assert-compile" error did indeed go away, but "lists/symbols may only be used at compile time" errors remained.

Perhaps this is a Fennel-side issue that I should take up with them?

kwshi avatar Aug 20 '22 23:08 kwshi

Hmm, it's because macros are kind of always eval-at-compile-time but not technically compiled.

When you compile a regular module that uses a macro, the macro module is evaluated during compile and the functions are basically added to the Fennel runtime, there is never any resulting lua code for the macro module.

To get the diagnostics for normal modules we give the code to the compiler and show any errors, but for macros that wont work.

Swapping compile-buffer to eval-buffer (with the env alteration) will let it work but:

  • it can only detect errors in unquoted code because the quoted code just compiles out to a list, it's never evaluated, so (bad-let ,(bad call) will show an error for (bad call) but not (bad-let.
  • there's the potential for harmful side effects as eval'ing the code will actually run it, which might accidentally run code that you don't want to run!
  • you could get around this probably by passing in an env that has compiler symbols and dead stubs for stuff like fopen etc.

AFAIK Fennel upstream doesn't provide any way to "check a macro compiles" as we are seeing. Unsure how hard that would be to implement. I know that Andrey Listopadov wrote some "eval-safely" stuff for his fennel-doc project.

I can add an additional function, perhaps (set-mode buf :compile|:eval) which would solve the issue but it wont ever be turned on by default due to the potentially unsafe nature.

rktjmp avatar Aug 21 '22 04:08 rktjmp

I've added an extra argument to set-options, (set-options buf opts :compile|:eval|nil) which effects how code is run, as above, be very careful you are not writing potentially damaging code and evaluating it unexpectedly.

Probably there is room in the API for diagnostics.check-once where you can explicitly force a diagnostic run on a buffer when you know its safe, if you didn't want it checked after each insert leave.

Changes: https://github.com/rktjmp/hotpot.nvim/commit/0c192b64addebfdc97d92803ce1e73d8600a2b03

rktjmp avatar Aug 21 '22 04:08 rktjmp

Should be fixed, requires macros to be in a file that ends in macro.fnl or macros.fnl, doc: https://github.com/rktjmp/hotpot.nvim/blob/e77fc345e3f7b8bdcc18f7da22edb4cde01613f4/doc/hotpot-api.txt#L79-L80

rktjmp avatar Nov 12 '22 11:11 rktjmp