wrap modules in #%module

[gelisam]

this makes it possible for a #lang to e.g. add some extra steps at the beginning or the end of a module.

Fixes #40

[gelisam]

This is a draft PR because it doesn't work yet. I tweaked the parser so that if readModule sees

(example (+ 2 2))
(example "foo")

it emits the Syntax object for

(#%module
  (example (+ 2 2))
  (example "foo"))

But now every module fail with #[...]<#%module>: Used in a position expecting a module but is valid in a position expecting a top-level declaration or example.

[gelisam]

I get the same error message on the main branch if I add the #%module wrapper myself:

#lang "prelude.kl"

(#%module
  (example (+ 2 2))
  (example "foo"))

But the error message is not quite what I expected in that circumstance. I would have expected to be told that (#%module ...) is valid in a position expecting a module, but is used in a position expecting a declaration or an example. Instead, the error message says the exact opposite!

[gelisam]

One possible explanation is that the error message swaps the expected and actual usages (again). But I tried

#lang "prelude.kl"

(+ 2 2)

and I get the correct error message, in which the actual and expected are NOT swapped:

#[...]<(+ 2 2)>:
  Used in a position expecting a top-level declaration or example
  but is valid in a position expecting an expression

[gelisam]

The error message is thrown here:

https://github.com/gelisam/klister/blob/10163dae560b6db3208fa1660e3760c64fcd622a/src/Expander.hs#L1241

I notice that the other throwError $ WrongMacroContext ... lines nearby use (problemContext prob) as the last argument, but that the line above doesn't, so I that's a mistake and this explains why the error message is backwards.

[gelisam]

That throwError line is the entire implementation when expandOneForm encounters an EPrimModuleMacro. I wonder if that's because this case is not yet implemented (in which case I should implement it as part of this PR), or if it is because module macros are supposed to be handled outside of expandOneForm?

[gelisam]

problemContext converts from MacroDest to MacroContext. It's not too clear to me what's the difference between those two, but they are very similar: they're both a sum type enumerating the possible things which might be expected at a given position, including a declaration, a type, an expression, a pattern, a type pattern, and a context. The only difference between the two types is that context is an element of MacroContext but not of MacroDest. It is thus clearly quite intentional that module positions are treated very differently from the other kinds of positions, so I don't think it's expandOneForm which is supposed to deal with module macros.

[gelisam]

Turns out there's a third one, SyntacticCategory, which only includes module, declaration, and expression contexts. I wonder what's the difference between MacroDest, MacroContext, and SyntacticCategory? I would have expected a single type, not three.

[gelisam]

Wait, MacroDest is more than an enum, it also stores the extra information about the position to be filled. So perhaps I just need to add an entry to MacroDest corresponding to the module context?

[gelisam]

...and SyntacticCategory is just dead code. That answers the question of why there are three different types: there should be only two, one of which is just an enum listing the possible contexts, and the other one giving more details than just that enum about the specific position being filled. Thus, I now think that I should add a module entry to MacroDest and replace the throwError implementation in expandOneForm with a proper implementation.

[gelisam]

I quite find the name "syntactic category" clearer than "MacroContext", perhaps I should rename it?

[gelisam]

I have added ModuleDest to MacroDest, and I've added an entry everywhere we were pattern-matching on MacroDest. Unfortunately, this still doesn't work because the generated #%module is still in a position where a declaration is expected.

I think this is because of this line in expandModule:

https://github.com/gelisam/klister/blob/3bc746b8838eedc1c05c9bcb25433a26c05caf45/src/Expander.hs#L126

It should instead delegate to the new expandModuleForm function, which in turn delegates to expandOneForm, which in turn runs the Syntax -> Expand () function from the implementation of #%module, and that function is makeModule. There are a lot of lines which seem duplicated between expandModule and makeModule, so I probably need to delete those lines from expandModule so that they only run once.

I think a bit more thought is needed though. When #langs provide their own #%module, they do so by writing a macro which produces code using #kernel's #%module. The code in expandModule is very careful to setup the expander's state so that macros in the module execute correctly. I thus need to figure out which code needs to run in expandModule, before the #lang's custom #%module macro runs, and which code needs to run in makeModule, after that macro runs but before the rest of the module is expanded.

[gelisam]

I'll have to revisit, but for now I did the simplest possible thing: I de-duplicated the lines by keeping the makeModule copy, except for newDeclTreePtr, which I had to thread through the ModuleDest because expandModule uses it later on.

[gelisam]

Progress! The #%module macro now runs, but it fails because it expects one extra argument, the name of the module:

(#%module 'module-name
  (example (+ 2 2))
  (example "foo"))

makeModule does not actually use the module name, it just makes sure it's an identifier. This is clearly a placeholder for future functionality... but for simplicity, I'd be inclined to drop it until we need it.

[gelisam]

Now I get TODO throw real error - already expanding a module. This fails when expanderModuleTop is set to a Just; which is weird, because I've deleted the line which was setting it in expandModule, as it was one of the duplicated lines. Maybe I should set it to Nothing instead? What is the code which makes use of this field?

[gelisam]

Maybe I should set it to Nothing instead? What is the code which makes use of this field?

It is only used to produce this TODO throw real error - already expanding a module error. Since the module syntactic category is already making sure that makeModule can't be called anywhere except the top-level of a module, this doesn't seem like a very useful check, shall I delete the check and the field?

[gelisam]

The test finally passes!

[gelisam]

It seems fine to me to be in a state where there's no module right now. I don't think we have any actions in the macro monad that assume a current module, do we?

Setting expanderModuleTop to Nothing is definitely not going to cause trouble, because the only place it is used is in makeModule, which makes sure it is Nothing. We should probably delete expanderModuleTop, it looks to me like it was created in anticipation of being useful, but we didn't end up using it for anything.

Otherwise, this seems similar to a custom declaration macro - as it expands to the core forms that create a declaration, it doesn't exist yet, right? What makes modules special here?

What makes modules special is the fact that expandModule does more than just filling-in a DeclTreePtr, it also modifies the ExpanderState: it clears and restores the fields expanderCurrentBindingTable and expanderDefTypes. It matters whether these global changes to the state are performed in expandModule or makeModule, because it will affect which bindings and types are in those global maps while the custom #%module implementation is running. If it is makeModule which clears the state, then the bindings and types from the module which imports custom-module-test.kl will be accessible while my-module is running, which is bad because that means the behaviour of my-module could be different depending on which module is it imported from.

Okay, I think I have convinced myself that the current code is correct: the global changes to the state must be performed in expandModule, not makeModule.