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

Final #6596

Merged
merged 6 commits into from Sep 22, 2017

Conversation

Projects
None yet
8 participants
@Simn
Member

Simn commented Sep 19, 2017

This adds final as a keyword and allows using it in two places:

  • As modifier to a method, like in Java.
  • As replacement for var when declaring fields.

The semantics of the latter are as follows:

  • static final vars have to be initialized immediately
  • if a class has non-static final vars which are not initialized immediately, it requires a constructor which has to assign to all such fields
  • static inline final works, but has the same semantics as static inline var
  • visibility is orthogonal and not affected
  • property syntax is not supported

Implementation details

I added a new var_access variant AccCtor which is used as the v_write value for non-static final vars. For static vars, we simply use AccNo. We internally add the @:final metadata to final fields (both vars and methods) in order to recognize final fields for the XML export. This also makes it automatically work with the already implemented @:final parts of the compiler.

Parsing is a bit funky: We can't parse final as a field modifier because it is also used for final x fields. We also have to consider that field modifiers may appear both before and after the final, which requires a call to parse_cf_rights twice.

@ncannasse

This comment has been minimized.

Member

ncannasse commented Sep 19, 2017

When discussing it, we also suggested to allow { final x : Int } as a replacement for { var x(default,never) : Int }

@Simn

This comment has been minimized.

Member

Simn commented Sep 19, 2017

Done

Note that there's a decision to be made there: Anonymous structures allow both class an structure notation, so when seeing the final we kind of have to make a choice in the parser. It works with structure notation only, so you can't do e.g. { final x : Int; var y : String; } (it wants comma instead of semicolon as the separator at that point).

@Gama11

This comment has been minimized.

Member

Gama11 commented Sep 19, 2017

static inline final works, but has the same semantics as static inline var

Is there a good reason for allowing this then? It seems like this is just adds one more thing to worry about in code-style debates / one additional check in haxe-checkstyle. :)

@nadako

This comment has been minimized.

Member

nadako commented Sep 19, 2017

What about simply always inlining static final when possible (same rules as current inline vars), which basically deprecates static inline var and reduces "cognitive burden" by a bit?

@RealyUniqueName

This comment has been minimized.

Member

RealyUniqueName commented Sep 19, 2017

Honestly i feel this final for vars is like abstract for decorators.
Why can't we just have const for vars? )

We already have (default,never) for static vars, which will now be duplicated with static final. And static inline var for real constants, which will also have a duplicate static inline final.

@Simn

This comment has been minimized.

Member

Simn commented Sep 19, 2017

Is there a good reason for allowing this then? It seems like this is just adds one more thing to worry about in code-style debates / one additional check in haxe-checkstyle. :)

Technically, you'll have to worry about it anyway because if anything, it would be a typing error. That means that it's still valid syntax either way.

@Simn

This comment has been minimized.

Member

Simn commented Sep 20, 2017

We already have (default,never) for static vars, which will now be duplicated with static final. And static inline var for real constants, which will also have a duplicate static inline final.

You would have the exact same situation with const or any other keyword.

@RealyUniqueName

This comment has been minimized.

Member

RealyUniqueName commented Sep 20, 2017

So let's think a bit more about reducing such duplications, not introducing them :)
It's a major version change anyway.

@Simn

This comment has been minimized.

Member

Simn commented Sep 20, 2017

We could remove (default, never), but I don't really see a good reason to break people's code like that...

@RealyUniqueName

This comment has been minimized.

Member

RealyUniqueName commented Sep 20, 2017

Can we mark some syntax as deprecated and remove it in 4.1?

@skial skial referenced this pull request Sep 21, 2017

Closed

Haxe Roundup 400 #432

@Simn Simn merged commit 8a076bf into HaxeFoundation:development Sep 22, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Simn Simn deleted the Simn:final branch Sep 22, 2017

@wiggin77

This comment has been minimized.

wiggin77 commented Sep 22, 2017

Will this eventually be expressed in target output? For example would "final s = new Something();" be marked as final in the Java target? This is important for generating thread safe code in cases where locks (constructors) are difficult.

@jgranick

This comment has been minimized.

jgranick commented Sep 22, 2017

Can this can be overridden using @:hack?

@EricBishton

This comment has been minimized.

Member

EricBishton commented Sep 22, 2017

It can be easily overridden using "search and replace." ;-)

@Simn

This comment has been minimized.

Member

Simn commented Sep 22, 2017

Since final is just @:final for methods, @:hack should work accordingly. That's obviously not gonna work on targets which actually generate final natively though.

I feel like if you have to use @:hack, something went wrong somewhere...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment