Module Methods

[FlandiaYingman]

Module methods are the methods defined within a module.
Module parameters are function parameters that are prefixed with module keyword.
Module values are module literals or applications of functions returning modules.

Rules

• Module parameters must have an explicit type.
• Module parameters must have a concrete type.
• Function returning module values must have an explicit return type.
• Function returning module values must have a concrete return type.
• The return type of functions returning module values must be prefixed with module keyword.
• Only module arguments (values) may be passed to module parameters.
• Only module methods may return module values.
• Function returning module values must not have multiple parameter lists.

[LPTK]

Ok, so currently we are trying to see if we can get away with not elaborating imported files. So your approach of accessing the defn won't work if the function comes from a different file. Based on this, I would suggest adding an explicit module modifier for parameters: fun foo(module m, arg2, arg3). I actually like it better than some subtle type-based check.

[LPTK]

So is this ready for review? (BTW pls read https://github.com/hkust-taco/mlscript/blob/mlscript/CONTRIBUTING.md, if you haven't)

Please notify me once you're addressed all our discussions and updated your branch. Also, why not check your TODOs in #233 (comment)?

[FlandiaYingman]

@LPTK

Things are getting messy as we come up with new ideas and rules and changing previous ones. I'm trying to refactor the code to ensure everything looks good and concise. That TODO list was intended to track the new rules that have been refactored.

This PR should be ready for review by tonight. I'll notify you when I'm done. 🫡

[LPTK]

Ok sure, no worries!

[LPTK]

Did you consider the following scenario?

module M
fun foo() = M

I don't think I saw it in the tests.

[LPTK]

The signature was already elaborated above in s. Looks like this will redo the work!

Though I guess the signature might be different from the explicitly-ascribed result type.

Anyway, you shouldn't manually retrieve this type by pattern matching the tree, duplicating logic in Tree.scala. There should only be one source of truth about what is the result type.

[FlandiaYingman]

What. I thought that's the problem we've just discussed earlier.

In this #233 (comment) you said "signatures are only picked up from externally-given declaration"; and at

mlscript/hkmc2/shared/src/main/scala/hkmc2/syntax/Tree.scala

Line 234 in e385993

(symbName, R(id), paramLists, S(typeParams), N)

it does not always pick up the result type from the parsed tree. That's why I did another parsing to make sure result types are always parsed.

[LPTK]

Ok, let me be completely explicit, so we don't get hung up on details and terminology:

• There should be only one place that determines what is the inline-annotated result type in a definition (if there is any). The logic for finding it by analyzing trees should not be duplicated (as you are doing now), as it means that any update to it must also be duplicated everywhere, which is impractical and error-prone.
  Currently, this is done in TypeOrTermDef and the result is confusingly called signature (we could change it to annotatedResultType).
• In the elaborator, if we see there is no inline-annotated result type, we should use the one given by the separate signature. This could be implemented later, though.
• Such explicitly-annotated result types must not be elaborated several times. Indeed, currently that's not the case, but we should be careful that this does not happen whenever we add the logic to achieve the previous point.
  This is more of a comment for the future and not immediately actionable.

it does not always pick up the result type from the parsed tree. That's why I did another parsing to make sure result types are always parsed.

If the implementation in TypeOrTermDef is missing some cases, you should fix that implementation, instead of leaving it broken and reimplementing it separately!

[FlandiaYingman]

@LPTK Everything's done! I

• removed Term.Mod
• fixed (maybe) a bug that our parser didn't pick up result types from generic functions (c874fe3, hope that's not intended)

so now only Tree.scala will parse the Tree as the single truth

[LPTK]

• fixed (maybe) a bug that our parser didn't pick up result types from generic functions (c874fe3, hope that's not intended)

Thanks, this was clearly a typo in the original code.

@LPTK Everything's done!

Actually you also need to update the branch and fix its conflicts with the main branch. If you choose to rebase, pls make sure that the tests still work on each individual commit: git rebase -i --exec "sbtn hkmc2JVM/test". If you don't rebase and have a dirty history, we can later squash all commits together when merging.

[LPTK]

Thanks! I've committed some minor last changes. Notably, there was some dead code that I removed.