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

enum abstract over enum constructors #87

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

kevinresol
Copy link

@kaikoga
Copy link

kaikoga commented Mar 31, 2021

Looks like you are editing the template file, instead of copying the template into proposals directory?

@skial skial mentioned this pull request Mar 31, 2021
1 task
@montibbalt
Copy link

For some more inspiration, I recently learned Ada has a handy feature related to this which lets you specify a subtype as a range of enum values (see "Enumeration subtypes"). As I understand it their enums are more like "traditional" ones rather than ADTs but... enum abstract is already Haxe's version of that :)

@RealyUniqueName
Copy link
Member

I like the idea.
It would be better for readability if enum parameters were mentioned in the syntax of enum abstract fields somehow.

@Simn
Copy link
Member

Simn commented Apr 2, 2021

enum abstract Status<T>(Option<T>) to Option<T> {
	final Continue = Some;
	final End = None;
}

This violates the abstraction because Some is not Option<T>, it's actually T -> Option<T>. Things like this have to be considered in the detailed design.

There are also some questions regarding pattern matching. For instance, given this enum from the proposal:

enum abstract MaybeValue<T>(Option<T>) to Option<T> {
	final One = Some(1);
	final Two = Some(2);
	final Other = Some;
	final Nothing = None;
}

What should happen if we have:

switch (maybeValue) {
    case Other(x):
    case One:
    case Nothing:
}

Here, case One is useless because it is covered by case Other(x), but we can only detect that by inspecting the underlying enum. We have to define what exactly we want to check here, and also what exactly we want to report. This should be a new case as far as exhaustiveness is concerned because up until now all enum abstracts were defined over infinite sets, while now we have to consider subsets of finite sets.

Overall, I'm not opposed to this idea, but the detailed design of this proposal is quite weak and needs some work before I could see myself approving this.

@kevinresol
Copy link
Author

kevinresol commented Apr 2, 2021

This violates the abstraction because Some is not Option<T>, it's actually T -> Option<T>. Things like this have to be considered in the detailed design.

From my understanding, enum abstract's non-static var/final are specially handled and they become the "constructors" of that abstract, and they become "static" too, accessed via Status.Continue just like its underlying enum Options.Some. Here, the two expressions has type T->Status<T> and T->Option<T> respectively. What I mean is that the behaviour should just be consistent with ordinary enum.

If I understand correctly, for primitive enum abstract the compiler expects literal values of the underlying type. So in this new feature, when assigning the value, as in final Continue = Some, the compiler just needs to check if Some is a "literal" constructor of the underlying type Option<T>.

We have to define what exactly we want to check here, and also what exactly we want to report.

The original idea of this proposal is really just to create a subset of the constructors of the underlying enum, without partial/full application. The example of MaybeValue is just to redirect people to #86. Because the exhaustiveness problem will exist there too.

@Simn
Copy link
Member

Simn commented Apr 2, 2021

Here, the two expressions has type T->Status and T->Option respectively. What I mean is that the behaviour should just be consistent with ordinary enum.

I don't follow where that T->Status<T> comes from. None is Option<T>, which is the underlying type. Some is T -> Option<T>, which is not the underlying type. This is the equivalent of having an abstract over String where one of the constructors is var FromInt = (i:Int) -> "" + i;. Yes it produces the underlying type, but it isn't the underlying type. This seems like a separate feature altogether.

The original idea of this proposal is really just to create a subset of the constructors of the underlying enum, without partial/full application. The example of MaybeValue is just to redirect people to #86. Because the exhaustiveness problem will exist there too.

But that doesn't mean that the problem doesn't have to be addressed. The cases you create aren't necessarily distinct, e.g. One == Other(1) in that example. This causes overlap, which should be considered when pattern matching. In particular, you cannot tell which constructors essentially shadow other constructors without looking at the implementation of the abstract, which is a terrible abstraction.

@kevinresol
Copy link
Author

I don't follow where that T->Status<T> comes from.

enum abstract Status<T>(Option<T>) to Option<T> {
	final Continue = Some;
	final End = None;
}

$type(Status.Continue); // T -> Status<T>
$type(Option.Some); // T -> Option<T>

What I really after is to define a subset of enum constructors, not a subset of enum instance values which should go #86 IMO. Perhaps we need some special metadata to express that intention.

@kevinresol
Copy link
Author

kevinresol commented Oct 6, 2021

I think this proposal should be renamed as "abstract enum over enum constructors" to avoid confusion. In the following I am trying to think aloud my understanding of the concepts around enum and enum abstracts and their relationship with this proposal. Any comments welcome.


First of all, regarding the enum type, there are two sets of values around:

  1. the set of enum constructors, which is finite
  2. the set of enum instances, which is potentially infinite

and this proposal is about the 1st set. For the 2nd set, IMO #86 is more relevant.


Secondly, regarding the enum abstract type, there are a few properties:

  1. there is an underlying type
  2. each member in the abstract is considered a entry of the finite set of the "enum", and they must be:
    2a) a constant expression
    2b) of the underlying type

I think 2a is fine because enum constructor itself is considered constant regardless of its parameter count, i.e. the following code already compiles:

enum abstract Foo(Dynamic) {
	final A = haxe.ds.Option.None;
	final B = haxe.ds.Option.Some;
}

I think it is 2b that is the major blocker of this proposal as enum constructors can be of different type depending on their parameters.


Thirdly, regarding the typing of enum constructors and enum abstract entries:

var value:TheEnumAbstract = TheEnumAbstract.AnyMember; // always hold
var value:TheEnum = TheEnum.AConstructorWithoutParam; // ok
var value:TheEnum = TheEnum.AConstructorWithParam; // not ok

But since currently we can't declare a enum abstract entry with parameter, so I consider the behavior between enum and enum abstract consistent.


Conclusion:

I think we want to keep the restriction that each entry in enum abstract is of the underlying type, such that proposals like #86 is still allowed. But since this restriction blocks this proposal as mentioned, I suggest introducing a specially handled enum abstract over enum constructor type. Inside such type each member will have to be a constant value which is a constructor to produce an instance of the underlying enum type. I think this combination will leave us only one option that is to declare the entry by assigning an enum constructor directly (as in the example in the 2nd section). Furthermore, the compiler should handle this new type the same way as ordinary enum in other aspects such as pattern matching and exhaustiveness check.

As for how to actually declare an enum abstract over enum constructor type, I can think of a few ways:

  1. metadata
    @:enumConstructor enum abstract Status<T>(Option<Int>)
  2. a special underlying type
    enum abstract Status<T>(EnumConstructor<Option<Int>>)
  3. stuff the enum keyword into the underlying type
    enum abstract Status<T>(enum Option<Int>)
  4. or even introduce a new "abstract enum" type (looks cleanest but might be confusing with abstract class)
    abstract enum Status<T>(Option<T>)

@kevinresol kevinresol changed the title enum abstract over enum enum abstract over enum constructors Oct 6, 2021
@skial skial mentioned this pull request Oct 6, 2021
1 task
@Simn Simn added this to the 2021-12 milestone Nov 8, 2021
@ncannasse
Copy link
Member

Alternate proposal is to allowing defining constructor function for enum abstracts that would work with top down inference and exhaustiveness check just like normal enums:

enum abstract Option<T>( Null<T> ) {
    var None = null;
    @:constructor public static function Some<T>( v : T ) : Option<T> { return cast v; }
}

@Simn
Copy link
Member

Simn commented Nov 9, 2021

We didn't reach a conclusion on this proposal in the haxe-evolution meeting today.

We feel like we should look closer into the problems that are meant to be solved here and then find solutions for those. There have been some alternative suggestions already, but so far it is unclear what to do and how to go about it.

@Simn Simn removed this from the 2021-12 milestone Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants