HaxeFoundation / haxe-evolution

Repository for maintaining proposal for changes to the Haxe programming language
111 stars 58 forks source link

Default value for enum parameters #27

Open sebthom opened 7 years ago

sebthom commented 7 years ago

I was surprised to find that while

enum Schedule {
    Hourly(?minute:Int);
    // ...
}

is possible, this is not:

enum Schedule {
    Hourly(minute:Int=1); // characters 18-19 : Unexpected =
    // ...
}

Right now I have to do additional null-checks on enums with optional parameters like so

var instance = new MyClass(Schedule.Hourly());

function MyClass {
    var schedule:Schedule;

    public function new(var schedule:Schedule){
       // validate schedule
       switch (schedule) {
           case Hourly(minute):
                // check for null and apply a default value if found
                if(minute == null) {
                   minute = 1;
                   schedule = Schedule.Hourly(minute); 
                } else if (minute < 0 || minute > 59)  
                   throw "Hourly.Minute must be >= 0 and < 60";
                }
            // ...
       }
       this.schedule = schedule;
    }
}

instead of simply:

var instance = new MyClass(Schedule.Hourly());

function MyClass {
    var schedule:Schedule;

    public function new(var schedule:Schedule) {
       // validate schedule
       switch (schedule) {
           case Hourly(minute):
                if (minute < 0 || minute > 59)  
                    throw "Hourly.Minute must be >= 0 and < 60";
            // ...
       }
       this.schedule = schedule;
    }
}

Therefore I would request adding default value behaviour for enums as specified here https://haxe.org/manual/types-function-default-values.html

delahee commented 7 years ago

I like this, every time we strip null possibilities from core language is a big win and lead to leaner code. +1

nadako commented 7 years ago

Since enum ctors with args are actually functions, maybe we can support this easily?

RealyUniqueName commented 7 years ago

I'd like to see this implemented too.

fullofcaffeine commented 7 years ago

+1 @sebthom You could use tink_lang to relieve the pain for now: https://haxetink.github.io/tink_lang/#/declaration-sugar/complex-default-arguments.

Simn commented 6 years ago

I don't think this could be implemented without significant changes to how we handle default values. In our implementation, the default values are not part of the function type, but of the function itself. Enum constructors to not have actual functions, they are only typed as such. This would only work if we did call-site replacements, but we don't.

The only alternative is to respect this in every generator.

back2dos commented 6 years ago

Enum constructors to not have actual functions, they are only typed as such.

On all platforms I've checked, they are actual functions. What's more, the type system also treats them as such, e.g. Some is of type Unknown<0> -> haxe.ds.Option<Unknown<0>>, meaning there must be a physical function somewhere and its implementation could treat default values accordingly.

Simn commented 6 years ago

Look, I'm not making this up to annoy you or avoid implementing this feature. You can look at haxe/macro/Type.hx and verify for yourself that the only place default values show up is TFunc, and the only place that shows up is the TFunction expression. You will also find that EnumField only has a Type, not an expression. Therefore, adding default values to enum fields would require changes to our enum field representation.

back2dos commented 6 years ago

Sorry, I misunderstood. We're in agreement after all: if the default values are actually known, there is no problem.

You can look at haxe/macro/Type.hx and verify for yourself that the only place default values show up [emphasis added] is TFunc [...]

Regrettably, upon following your invitation I failed to verify your claim. As it turns out you already introduced a compile time representation of default values. It's not a pretty one, but it is sufficient and could be used for enums just as well. Just saying ...

Simn commented 6 years ago

I added that because we wanted to have the original syntax for generating documentation. For generating output, we need a properly typed representation and we cannot store that in metadata. I also wouldn't want to rely on such a hack to generate code in the first place. So the only option is change our representation of enum fields.

I'm not saying it can't be done, but I guarantee you it's more involved than you make it seem.

grepsuzette commented 5 years ago

Enums are now such a proeminent and powerful feature of Haxe (look at all the powerful variants of switch, it really changes the way we code), I definitely would love to see this implemented, one day or another.

Simn commented 6 months ago

There are two main related problems here:

  1. In order for Type.createEnum to work, default values will have to be considered at run-time. This means that the enum constructor functions need the usual if (inValue == null) inValue = defaultValue, for which they'll need the defaultValue expression. This requires some internal refactoring and updates to all target generators and/or run-time support code.
  2. minute:Int = 1 highlights the desire to have values stored as Int, not Null<Int>. However, for 1. we'll need a nullable value at the time of creation. This can easily be a hidden performance trap because a naive implementation that maps Hourly to Hourly would have to wrap the actual integer value in Null<Int> in order to call the creation function. This seems to make it necessary to have two creation functions: One for when we know that the passed value is indeed Int and one which does the null-check and then calls the former.

This might sound like premature optimization, but I really don't think that an implementation which wraps basic values only to then null-check and unwrap them would be acceptable.

Edit: Just to make this clear, all this bullshit is only required in order to support the dynamic/reflection paths. If we did call-site replacement of default values then both problems wouldn't exist. That's not how Haxe is designed though and it then would indeed be surprising if default values didn't work through reflection nonsense.

back2dos commented 6 months ago

I really don't think that an implementation which wraps basic values only to then null-check and unwrap them would be acceptable.

  1. Is that not what we always do for optional primitive args though? How is that any different now?
  2. How is forcing the user to do the null checks themselves any better?

Reflection is not really the problem here (if it were I would suggest solving this in Type.createEnum, by accessing some rtti to pad the arg list to the correct length). Just do var x = SomeEnum.SomeConstructor and the default value is lost. Default values are part of the implementation, not the signature. For better or worse function a(x = true) {} and function b(x = false) {} have the same type.

Of course I'm all for a fast path for call site replacement, if possible.

I was also gonna suggest to use native default values on the platforms that support it, but the more I think about it, the more I wonder how well that can even fit with our beloved arg skipping.

Simn commented 6 months ago

Just do var x = SomeEnum.SomeConstructor and the default value is lost.

You're right, I was somehow thinking that it would turn the type into not having optional arguments, but it doesn't. In that case this is all doomed anyway.

Plus, I don't want to implement something that would change case V(x): V(x); from V(null) to V(default). This is already really strange for normal functions, but with structured data like enums these kind of value changes seem ridiculous...