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

[lua] add special markup + handling for extern closure instances #6415

Closed
wants to merge 3 commits into from

Conversation

jdonaldson
Copy link
Member

@jdonaldson jdonaldson commented Jun 28, 2017

Here's a simple POC for supporting closure-style extern instances, as described in #6411.

The main idea is to provide special metadata that changes how an extern behaves:

@:luaClosureInstance
extern class Bar {
	public function new(){ }
	public function bar(a:Int, b:String) : Int;
}

Field method invocations on this instance will be simple "dot" style, rather than the colon.

There's a lot of advantages to this approach... It's restricted to extern only, and I can keep most of the code nicely isolated. Still, I don't think I can make this perfect.

There's ways this could break... it mostly would involve situations with dynamic, reflect, etc... also I guess this breaks if you pass it as a structural type, or try to use inheritance with this. Some of that I can guard against.

The other alternative is to wrap each instance in some kind of proxy that substitutes a dummy "self" argument where necessary. This makes the extern instances easier to pass around through the other type signatures. However, I couldn't think of a good way of doing this automatically (abstracts, etc.). @Simn you have ideas there? Also, I'm guessing the wrapping would add some overhead, which is not really ideal.

I'm leaning more towards the first option, which is what I have here. I have a utility method to support the second approach, but I can't figure out how to automatically apply it.

@jdonaldson jdonaldson force-pushed the lua_closure_instance branch 5 times, most recently from e73779a to 4a51e56 Compare June 28, 2017 23:38
@jdonaldson
Copy link
Member Author

I'm also looking for recommendations for naming. @:luaClosureInstance seemed clear, anything better?

@martinluetke
Copy link

@:luaNoImplicitSelf ?

@jdonaldson
Copy link
Member Author

@:luaDotAccess ?

@martinluetke
Copy link

martinluetke commented Jun 30, 2017

I like that better than my suggestion. Is it possible to apply that meta to a field, or only to an entire class?

@jdonaldson
Copy link
Member Author

Is it possible to apply that meta to field, or only to an entire class?

Pretty sure I can do both, let me see what that looks like.

@jdonaldson
Copy link
Member Author

Not too bad! I'll hold this open for a few more days, but I think I can check it in as-is.

@jdonaldson
Copy link
Member Author

hmmm @:luaDotMethod

@martinluetke
Copy link

martinluetke commented Jun 30, 2017

I was researching functions as members of structures and classes in the past hours and I think I have acquired a better understanding of the entire problem.
Right now handling of functions that are declared as var in structures and classes is broken/inconsistent.
Handling of functions that are declared as function is also inconsistent.

If you are interested I can create some examples of the problems, but essentialy my conclusion is this:

  1. Functions that are declared as var in structures and classes should never be assumed to use self. They should always be accessed with ".". When assigning lambdas no self should be added, they can be assigned as they are. Local and static functions (which also have no notion of "self") should simply be assigned as they are. Object methods should be assigned with _hx_bind.
  2. Functions that are declared as function in struct and classes should always be assumed to use self. When assigning lambdas a self should be added. Local and static functions should be wrapped so that self is added but ignored(as it is the case right now, well... sometimes). Object methods should be wrapped in the same fashion and assigned with _hx_bind (as it is now).
    Unless we are in an extern class and @:luaDotAccess is used, than that field/fields should be treated like they were declared with var.

These 2 rules will fix the bugs I am encountering right now, they take into account that classes that match can be assigned to structures (https://haxe.org/manual/types-structure-extensions.html) and the @:luaDotAccess attribute will still help tremendously because you cannot use overloads, named parameters(for documentation), default parameters or optional parameters with fields declared via var. All of wich are more or less required when typing external lua "classes".

@jdonaldson
Copy link
Member Author

jdonaldson commented Jun 30, 2017

Thanks for doing some research on this. I appreciate having another person thinking about it.

I originally had the generator working as you say. However, it's easy to run into problems because (as you note) you can assign class-based instances to structural types, methods can be marked "dynamic" and get rebound in the runtime, and in case of dynamic type access, the compiler won't really know which style of method to invoke. It became clear to me that I needed to stick with one style of method invocation for haxe generated code. Haxe as (a general rule) puts no restrictions on run-time access (e.g., all things are "public" in the runtime), so colon style access (with an exposed/rebindable "self" argument) made the most sense as the default.

There's a lot of hoops to jump through here, but the end result is that there's no question of how to invoke an instance method for haxe generated code.

We can use this extern to provide the sole case that breaks the rule for field access, and I think it'll be a lot easier to reason about moving forward.

@martinluetke
Copy link

Fair enough. Btw. I just installed the latest development version and some of the problems I had are actually gone. Yay :).
Yet, some are still there, I'll open new issues for them.

@jdonaldson
Copy link
Member Author

one last tweak, using @:luaDotMethod here. It's clearer about being specific to methods.

@jdonaldson
Copy link
Member Author

squashed this and merged.

@jdonaldson jdonaldson closed this Jul 2, 2017
@jdonaldson jdonaldson deleted the lua_closure_instance branch March 28, 2018 21:12
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

2 participants