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

Allow arbitrary expressions in place of property identifiers #7129

Closed
wants to merge 1 commit into from

Conversation

Simn
Copy link
Member

@Simn Simn commented Jun 5, 2018

This change allows parsing any expression where we currently only allow property identifiers. If any non-identifier is used, it fails while loading the type.

This breaks macros that inspect FProp, which is unfortunate, but I believe we should break this properly instead of trying to maintain backwards compatibility through some hacks.

Obviously, I have a follow-up planned once this is merged.

@back2dos
Copy link
Member

back2dos commented Jun 5, 2018

This breaks macros that inspect FProp, which is unfortunate, but I believe we should break this properly instead of trying to maintain backwards compatibility through some hacks.

Hmm. My long standing hopes for FProp were that the first two params would become a proper enum, instead of fiddling with strings. This step goes in the opposite direction though, so I'm personally not that big a fan.

@nadako
Copy link
Member

nadako commented Jun 5, 2018

I once mentioned that it would be a good idea for macro-powered sugar, but nowadays I'm not sure how well will it play.

For example one usage idea for this would be something like var prop(_prop, _prop = value), right? Now if we have some get/set methods that we want use from within a (g/s)etter we'd do var prop(get("..."), set("...", value)) which is confusingly close to normal var prop(get,set) which will have a different meaning. Of course this example is a bit artificial, but I'd love to see some good use-case examples that doesn't give me that weird gut feeling :)

@Simn
Copy link
Member Author

Simn commented Jun 5, 2018

I wanted to go for var prop(() -> _prop, value -> _prop = value), i.e. normal functions (with or without short lambda).

@Gama11
Copy link
Member

Gama11 commented Jun 5, 2018

I'd prefer that property syntax over the current one, but it raises one question: how would you override accessors now? Would that simply not be supported, or would some override var syntax be allowed?

I could imagine something like this:

// parent type
var prop(() -> _prop, value -> _prop = value):Int;

// child type - only the getter is overriden
override var prop(() -> _prop * 2, super):Int;

super would allow you to override a property's accessors without needing to redefine both methods if you only want to change one of them (by using the super type's accessor). _ might work too.

@Simn
Copy link
Member Author

Simn commented Jun 5, 2018

Since it would still generate get_prop and set_prop fields, you could simply override these using the current syntax. I don't think it's very common to do that anyway.

@Gama11
Copy link
Member

Gama11 commented Jun 5, 2018

That doesn't sound very clean to me. I think if a new syntax is introduced, it should properly support all features, not have some strange in-between solution... With the new syntax, the get_ / set_ fields should become an implementation detail the user doesn't need to know about.

You might be surprised how common overriding accessors is in object-oriented code. A quick Ctrl+Shift+F on flixel shows 47 results for override function set_ and 11 for get_.

@Simn
Copy link
Member Author

Simn commented Jun 5, 2018

I only wanted a short syntax for the current property accessors, but from the comments so far I can already tell that this would have to go through 20 years of haxe-evolution only to then be turned down by Nicolas.

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.

None yet

4 participants