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 syntax #6982

Merged
merged 4 commits into from
Apr 30, 2018
Merged

Conversation

Simn
Copy link
Member

@Simn Simn commented Apr 26, 2018

Allows enum abstract instead of @:enum abstract and deprecates the latter. We still use @:enum internally for now, so most of the changes here are in .hx files.

I'll follow my usual policy of "merge if nobody complains".

@Justinfront
Copy link
Contributor

if p <> null_pos then ctx.com.warning "`@:enum abstract` is deprecated in favor of `enum abstract`" p;

would this require #if haxe3 code in libraries or will both be supported for awhile?

@Simn
Copy link
Member Author

Simn commented Apr 26, 2018

It's a warning, so it's gonna work either way. I think we have a way to disable warnings now as well. @RealyUniqueName should know more.

# Conflicts:
#	src/typing/typeload.ml
@ncannasse
Copy link
Member

Regarding deprecation warnings: we have several of them for Haxe 4.0 , I wonder how we should handle them. For instance compile with -D haxe3-syntax would disable all these warnings. Whereas compiling with -D haxe4-syntax would turn them into errors.

@ncannasse
Copy link
Member

Question: enum abstract or abstract enum ?

@Simn
Copy link
Member Author

Simn commented Apr 27, 2018

Conceptually, it's still an abstract and comes with all abstract features. So enum abstract is correct.

@Gama11
Copy link
Member

Gama11 commented Apr 27, 2018

I agree that a define to disable Haxe 4 specific warnings would be helpful, all the warnings can get annoying, especially if you can't fix them because they're in third-party libs.

I think enum abstract makes sense because enum in this case is like a modifier for abstract.

@R32
Copy link
Contributor

R32 commented Apr 28, 2018

I think it's possible to use the enum directly.

enum Foo { // when "var" it's must be "enum abstract"
    var A = 1; 
}

enum Bar {// normal enum
    A;    
}

@back2dos
Copy link
Member

@R32 In theory it's not a bad idea. In practice there are multiple differences to consider:

  1. abstracts have more features (implicit casts, operator overloading, methods)
  2. abstracts don't exist at compile time: Type.typeof(Bar.A) is TEnum(Bar) and Type.typeof(Foo.A) is TInt. Corollary: enums unify with EnumValue, abstracts don't.

@Simn
Copy link
Member Author

Simn commented Apr 30, 2018

I'll go ahead and merge this, then disable the deprecation warning for now. We can re-enable it once we know how to deal with the Haxe 4 transition in general.

@Simn Simn merged commit 0edc98e into HaxeFoundation:development Apr 30, 2018
@Simn Simn deleted the enum_abstract_syntax branch April 30, 2018 06:00
@Simn
Copy link
Member Author

Simn commented Apr 30, 2018

It just occurred to me that we can check for the presence of the hx3compat define. This way we can keep some behavior if -lib hx3compat is in place.

Simn added a commit that referenced this pull request Apr 30, 2018
Gama11 added a commit to vshaxe/haxe-TmLanguage that referenced this pull request May 1, 2018
@skial skial mentioned this pull request May 4, 2018
1 task
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