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

further work on final? #6615

Closed
nadako opened this issue Sep 26, 2017 · 17 comments
Closed

further work on final? #6615

nadako opened this issue Sep 26, 2017 · 17 comments

Comments

@nadako
Copy link
Member

nadako commented Sep 26, 2017

I have a couple suggestions/question regarding the new final feature:

  • How about inlining static final fields when they are initialized with a constant? This would effectively deprecate static inline var, which a lot of users find weird and I think it would make the language easier to learn.

  • final doesn't seem to be supported for anon structures class notation, e.g. {final x:Int;} currently gives Unexpected ; error. Is this intentional or it's a bug?

  • Not really related to final keyword feature, but I think we need a (maybe optional) post-process filter that will add "final" to fields and classes that are not extended/overriden in current compilation. I think it's useful for performance (providing static dispatch instead of vtable in a lot of cases), and IIRC something like this is already done for Hashlink target on generator-level. Maybe we could have this as a general optimization?

@RealyUniqueName
Copy link
Member

Don't forget that haxe-generated code can be used from a 3rd party code. In that case user should be able to cancel "final" optimization on specific fields.

@Simn
Copy link
Member

Simn commented Sep 28, 2017

Yes I don't think we should start inferring inline. This could lead to surprises because even with an explicit field access, the field could be removed by DCE and then not be available through Dynamic/Reflection.

Regarding structure notation, it is supported, but locks you into structure notation. This is a parsing problem because we can't distinguish the two until we see the separator. I'm not sure what we can do here...

Regarding inferring final on methods: Are we sure that nobody wants to run-time extend Haxe-generated code? I think this already wouldn't work on several targets which have is_overridden checks, but we have to define this somehow.

@back2dos
Copy link
Member

Regarding structure notation, it is supported, but locks you into structure notation. This is a parsing problem because we can't distinguish the two until we see the separator. I'm not sure what we can do here...

I think not allowing final in structures would be a better solution then. If you're going to use final it's not like structures are actually shorter: { final x:Int; final y:Int; } vs. { final x:Int, final y:Int, } (granted, you can omit the trailing comma and save 1 token \o/).

To me at least it would be far more useful if a complex type starting with final would expect a structure next and make the whole structure final, so final { x:Int, y:Int } is short for { final x:Int; final y:Int; }. To clarify: final { var x:Int; } is disallowed on a syntactic level, because final MUST be followed by structure, rather than fields.

@nadako
Copy link
Member Author

nadako commented Sep 28, 2017

@Simn

Regarding structure notation, it is supported, but locks you into structure notation. This is a parsing problem because we can't distinguish the two until we see the separator. I'm not sure what we can do here...

Shouldn't it lock you into class notation? Since final is basically a replacement for var, and var is only for class notations, I'd say it should work just like that: encountered final - switch into class notation.

Are we sure that nobody wants to run-time extend Haxe-generated code?

Surely not, finalizing methods/classes should be configurable (maybe not even enabled by default).

@back2dos

it would be far more useful if a complex type starting with final would expect a structure next and make the whole structure final, so final { x:Int, y:Int } is short for { final x:Int; final y:Int; }

I would also love "immutable" type modifiers, but I think that's a different and more complex matter that needs much more design and discussion. In fact I even started writing a proposal about that months ago. The current final keyword is only about immutable references, not immutable structures.

@back2dos
Copy link
Member

Evidently, I am once again failing to express what I mean. I do not wish to inflate the issue towards C++ const or anything even remotely similar. Let me try to illustrate my issue with some code:

typedef StyleObject = {
  @:optional var alignContent(default, never):String;
  @:optional var alignItems(default, never):String;
  @:optional var alignSelf(default, never):String;
  @:optional var alignmentAdjust(default, never):String;
  // ... >300 more fields here
}

So there's @:optional (default, never) written over 300 times. If I changed the 150th field to (default, default) would that be easy to spot? Nope. Or if I had a typo like @:optoinal somewhere? Nope. It's just noise that buries actual information. In total, the 807 lines of code of js-virtual-dom contain never 474 times, because the library largely defines data structures where every field of every structure is immutable and Haxe provides no explicit way of actually communicating that to the reader. In attempt to improve this, I would suggest to allow writing the following:

typedef StyleObject = final {
   ?alignContent:String,
   ?alignItems:String,
   ?alignSelf:String,
   ?alignmentAdjust:String,
   // ... 300 more fields here
}

(Actually, I would still argue that different syntax should have different semantics and therefore structures should be "final" and for anything else you should use class field notation, but whatever ...).

And yes, every field may hold mutable data. This is perfectly fine, because sometimes it's actually desired. In other words:

typedef HasIntArray = final {
  array:Array<Int>
}
//Is fully equivalent to
typedef HasIntArray = {
  final array:Array<Int>;
}
//and given
var o:HasIntArray = { array: [1, 2, 3] };
//this compiles:
o.array.push(12);
//and this doesn't
a.array = [1, 2, 3, 12];

I hope this makes a bit clearer what I want and why.

@nadako
Copy link
Member Author

nadako commented Sep 28, 2017

So there's @:optional (default, never) written over 300 times.

Well that's what final is there to help with (not the @:optional part, obviously, which is a yet another different topic):

typedef StyleObject = {
  @:optional final alignContent:String;
  @:optional final alignItems:String;
  @:optional final alignSelf:String;
  @:optional final alignmentAdjust:String;
  // ... >300 more fields here
}

And yes, every field may hold mutable data. This is perfectly fine, because sometimes it's actually desired.

Sometimes yes, sometimes no. In my project I actually want C++-like recursive const. My point is that I think it's a bit short-sighted to rush into adding final { ...struct type... } syntax that will cover one particular use-case with not repeating the final keyword for anonymous structure type declaration, since it really looks like a type "modifier" which surely will bring up discussions about const types in general.


By the way, regarding these pesky @:optional metas, maybe we should think of something to shorten that as well. For example, we could have final?/var? syntax for fields which would parse into @:optional final/var, so your example would look like:

typedef StyleObject = {
  final? alignContent:String;
  final? alignItems:String;
  final? alignSelf:String;
  final? alignmentAdjust:String;
  // ... >300 more fields here
}

The final? looks like a dumb question tho, so maybe ?final/?var is better. Anyway, that's a different topic as well...

@back2dos
Copy link
Member

Sometimes yes, sometimes no. In my project I actually want C++-like recursive const. My point is that I think it's a bit short-sighted to rush into adding final { ...struct type... } syntax that will cover one particular use-case with not repeating the final keyword for anonymous structure type declaration, since it really looks like a type "modifier" which surely will bring up discussions about const types in general.

Yes, that is precisely why we chose final as opposed to const. These are two fundamentally different things. I want exactly the thing I described and what you propose instead is not only way more complex, but also doesn't cover my use case, except in the very special case where all fields also happen to hold immutable data. But this is not always the case. For example, I want to write something like this:

typedef User = final {
  id:Int,
  name:Observable<String>,
  blocked:State<Bool>,
}

So yes, the id is constant, but the name may change over time. In other words, it's not immutable and I don't want it to be. The State in blocked is not even readonly. I can do user.blocked.set(true). Whoever handed me the user object might actually be observing changes on that state object. Therefore for them it's important that I cannot do user.blocked = new State(true), because that would break up the object graph in an unintended manner.

Right now, I have to write this:

typedef User = {
  var id(default, never):Int;
  var name(default, never):Observable<String>;
  var blocked(default, never):State<Bool>;
}

This is a bit of an improvement:

typedef User = {
  final id:Int,
  final name:Observable<String>,
  final blocked:State<Bool>,
}

But really, this is what I want:

typedef User = final {
  id:Int,
  name:Observable<String>,
  blocked:State<Bool>,
}

An yes, as the system grows, the User may have 20 fields or more. Seeing a final right at the start, that tells me that the structure itself is immutable (but not necessarily the data that it holds) is - to me at least - very useful. And either way, const will not solve that problem for me.

@nadako
Copy link
Member Author

nadako commented Sep 28, 2017

So you basically just want to have final before { as a part of the short structure notation? I understand what you're after, but I also think this feature would be a bit "random". FWIW you can already have a @:genericBuild type that would give you pretty much the same thing:

typedef User = Final<{
  id:Int,
  name:Observable<String>,
  blocked:State<Bool>,
}>;

Anyway, I still think that final in anonymous structures should indicate the class notation and simply be used instead of var, just like in classes. This looks consistent and makes a lot of sense to me. @Simn what do you think?

@Simn
Copy link
Member

Simn commented Sep 28, 2017

Makes sense

@back2dos
Copy link
Member

I also think this feature would be a bit "random"

Look, if you have no use for this, ok fine. That's your prerogative. But to say it's "random" is just non-sense, because such final structures are pretty much exactly how records work in standard ML. They are immutable, but they may hold refs (which themselves are mutable). Even Haskell records can hold mutable stuff like MArray.

FWIW you can already have a @:genericBuild type that would give you pretty much the same thing.

I know that. You know I know that. I know you know I know that. And we also both know you have made your argument against resorting to such workarounds in your const proposal. So let's stop playing silly games, can we?

@nadako
Copy link
Member Author

nadako commented Sep 28, 2017

Uhm, I didn't say I don't have use for immutable records, the "randomness" point was about this particular syntax extension for short structure notation syntax. I don't have a strong position on that, just sharing my opinion. I even added "a bit", you know. Why such drama?

Regarding my arguments against macro approach: most of them apply only to the recursive const I was proposing specifically. The Final macro implementation of what you want is much much simpler and won't suffer from any of the issues I listed, except maybe macro performance (which is also something I'm not sure about since the macro is simpler and macro interpreter is faster now).

@back2dos
Copy link
Member

Uhm, I didn't say I don't have use for immutable records, the "randomness" point was about this particular syntax extension for short structure notation syntax.

That's not how I would read this:

I also think this feature [emphasis mine] would be a bit "random"

But fair enough. If what you're saying is all about the syntax, we fully agree. It is random, as is final itself (that has totally unrelated meanings for fields and methods \o/). Indeed, I want to have immutable records without having to jump through hoops. I have proposed interpreting structure types as that. That was dismissed. Now it seems that final is what we get, so I'm basing my further suggestions on that.

Regarding my arguments against macro approach: most of them apply only to the recursive const I was proposing specifically. The Final macro implementation of what you want is much much simpler and won't suffer from any of the issues I listed, except maybe macro performance (which is also something I'm not sure about since the macro is simpler and macro interpreter is faster now).

If this is your implementation, then the recursion is expensive because you're not caching sufficiently, e.g. in { a: { x: Int }, b: { x: Int }} you do the work for { x :Int } twice. If you cached everything, you're gonna land at O(F) cost, where F is the total number of fields involved (plus it also works for circular types \o/). So ultimately @:genericBuild is either suitable for both or for none.

@markknol
Copy link
Member

markknol commented Feb 20, 2018

I think we also should consider final in expression blocks.
And also allow final class and add deprecate warning for @:final class metadata.

@Simn Simn added this to the Design milestone Apr 17, 2018
@ncannasse
Copy link
Member

We have added both final in typedefs and in statics.

@RblSb
Copy link
Member

RblSb commented May 9, 2018

If you deprecate @:enum and @:extern, may do the same thing with @:final? :)

@ncannasse ncannasse reopened this May 18, 2018
@ncannasse
Copy link
Member

Replacing @:final for fields and classes with proper final keyword is still left todo.

@ncannasse ncannasse modified the milestones: Design, Release 4.0 May 18, 2018
@Simn
Copy link
Member

Simn commented Jun 5, 2018

I'll close this to focus on #6584.

@Simn Simn closed this as completed Jun 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants