Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying or deducing basis configuration #20

Open
shwestrick opened this issue Jan 16, 2023 · 2 comments
Open

Allow specifying or deducing basis configuration #20

shwestrick opened this issue Jan 16, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@shwestrick
Copy link

Problem

Millet generates a 5006 error for the following code; however, this code is accepted by both SML/NJ and MLton.

val x = Time.fromReal (1.0: real)

The error is technically correct, because Time.fromReal has type LargeReal.real -> time, resulting in a clash between real and LargeReal.real.

However, it is common for real = LargeReal.real. Both the SML/NJ and MLton implementations of the basis library do this.

Solution

A couple ideas:

  • Allow the user to specify the basis configuration by providing constraints such as type real = LargeReal.real.
  • Automatically deduce the basis configuration, perhaps by loading the basis library itself. IMO this is the preferable option.

The second option is what smlfmt does, for MLton at least. smlfmt queries the local install of MLton to determine where the basis library lives: see e.g. MLtonPathMap.getPathMap, which this tells us where $(SML_LIB) lives. For SML/NJ, you would need to know where $/basis.cm lives.

@shwestrick shwestrick added the enhancement New feature or request label Jan 16, 2023
@azdavis
Copy link
Owner

azdavis commented Jan 16, 2023

some issues i see with 2:

  1. it requires the user to have a local sml installation. probably many users will anyway, but if they don't their experience will be limited. (or should we fall back to what millet has built in?)
  2. millet would either have to know how to process the std basis, which i'm pretty sure has some… weirdness not seen in regular sml files (since it's defining a bunch of "built in" functions), or know how to specifically ignore the "weirdness" but still deduce the types of bindings, etc.

as for 1, if the user is going to do things themselves anyway, they could also just define val timeFromReal = Time.fromReal o Real.toLarge and use that everywhere. this has the benefit of being portable across any SML implementation and not depending on basis-specific implementation details.

@shwestrick
Copy link
Author

it requires the user to have a local sml installation. probably many users will anyway, but if they don't their experience will be limited. (or should we fall back to what millet has built in?)

Yeah, I was picturing that you can always fall back on the default behavior.

millet would either have to know how to process the std basis, which i'm pretty sure has some... weirdness

Haha yes, you're absolutely right. I ended up adding cases in smlfmt for MLton-specific directives, e.g. _overload which MLton uses for the limited overloading provided by the SML basis. Personally I don't think it's too bad, but certainly it's not ideal.

as for 1, if the user is going to do things themselves anyway, they could also just define val timeFromReal = Time.fromReal o Real.toLarge and use that everywhere. this has the benefit of being portable across any SML implementation and not depending on basis-specific implementation details.

In this particular case, this solution seems not too bad. But more generally, it's common to have implementation-specific code somewhere. For example, mpllib has a compatibility layer that allows it to be compiled with both mpl and mlton. Other projects commit to only using a particular compiler, in which case it's reasonable to rely on specifics of that compiler's ecosystem. A good example might be projects that rely heavily on FFI.

@azdavis azdavis changed the title Specify or deduce basis configuration Allow specifying or deducing basis configuration Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants