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

Forwarded fields lose information about abstract type. #6639

Closed
RealyUniqueName opened this issue Oct 4, 2017 · 12 comments
Closed

Forwarded fields lose information about abstract type. #6639

RealyUniqueName opened this issue Oct 4, 2017 · 12 comments
Assignees
Milestone

Comments

@RealyUniqueName
Copy link
Member

@:forward abstract NativeStructArray<T:{}>(T) from T {}
typedef AnonOptions = NativeStructArray<{hostt:String}>;

class Main {

    static var tmp:Dynamic;

    static function main() {
        var options:AnonOptions = {hostt:'example.com'};
        tmp = options;
        tmp = options.hostt; //see dump for this line below
}

-D dump=ast

<....>
[Field:String]
    [Local options(1206):AnonOptions]
    [FAnon:String] hostt

Add this to any generator:

match expr.eexpr with
    | TField (fexpr, FAnon ( { cf_name = "hostt" } )) ->        
        (match fexpr.etype with
            | TMono _ -> print_endline "TMono"
            | TEnum _ -> print_endline "TEnum"
            | TInst _ -> print_endline "TInst"
            | TType _ -> print_endline "TType"
            | TFun _ -> print_endline "TFun"
            | TAnon _ -> print_endline "TAnon"
            | TDynamic _ -> print_endline "TDynamic"
            | TAbstract _ -> print_endline "TAbstract"
            | TLazy f ->
                print_endline "TLazy";
                (match !f with
                    | LProcessing _ -> print_endline "LProcessing"
                    | LWait _ -> print_endline "LWait"
                    | LAvailable etype ->
                        print_endline "LAvailable";
                        (match etype with
                            | TMono _ -> print_endline "TMono"
                            | TEnum _ -> print_endline "TEnum"
                            | TInst _ -> print_endline "TInst"
                            | TType _ -> print_endline "TType"
                            | TFun _ -> print_endline "TFun"
                            | TAnon _ -> print_endline "TAnon"
                            | TDynamic _ -> print_endline "TDynamic"
                            | TAbstract _ -> print_endline "TAbstract"
                            | TLazy _ -> print_endline "TLazy"
                        )
                )
        )
    | _ -> ()

Expected output:

$ haxe build.hxml
TLazy
TAvailable
TAbstract

Current output:

$ haxe build.hxml
TLazy
TAvailable
TAnon

That means generator cannot recognize NativeStructArray behind AnonOptions in options.hostt expression.

@nadako
Copy link
Member

nadako commented Oct 4, 2017

As I mentioned before, this is not related to TLazy, but field @:forwarding. When you access @:forwarded fields, the object is re-typed as the underlying type (see

haxe/src/typing/typer.ml

Lines 1536 to 1537 in 2ab958e

if does_forward a false then
type_field ~resume:true ctx {e with etype = apply_params a.a_params pl a.a_this} i p mode
).

I see two ways of fixing this:

  • have a FForwarded of t* field_access variant for field_access or something like that
  • generate (cast obj : Underlying).field expression that can be matched in the generator

@RealyUniqueName
Copy link
Member Author

I think FForwarded is the proper way to go.

@Simn
Copy link
Member

Simn commented Oct 4, 2017

I don't really understand why you guys expect this to resolve to the abstract. The whole idea of @:forward is the pass through the abstract and use the underlying type.

@nadako
Copy link
Member

nadako commented Oct 4, 2017

I don't really understand why you guys expect this to resolve to the abstract. The whole idea of @:forward is the pass through the abstract and use the underlying type.

I think it's certainly fine if FInstance/FAnon/etc. contains the underlying type information, but suddenly changing the type of an object expression (in case of TLocal also making it inconsistent with v_type) without introducing a cast sounds wrong to me.

@Simn
Copy link
Member

Simn commented Oct 4, 2017

I think this can happen with inlining too because it's a very similar situation. We can insert a cast, but then we might run into problems with (a : T) = c kind of stuff.

@nadako
Copy link
Member

nadako commented Oct 4, 2017

What about not touching types and adding casts and introduce FForward of field_access variant that would be used for forwarded TFields?

@Simn
Copy link
Member

Simn commented Oct 4, 2017

I don't see the point given that inlining already generates these casts. This would just introduce another special case you have to deal with.

@RealyUniqueName RealyUniqueName changed the title TLazy skips types. Forwarded fields lose information about abstract type. Oct 5, 2017
@RealyUniqueName
Copy link
Member Author

Here is my motivation:
PHP api utilizes a lot of associative arrays with limited set of well-documented sets of keys (e.g. http://php.net/manual/en/function.proc-get-status.php)
I want to add type safety to such arrays. My idea is to create an abstract which will fowrard all fields of an anon structure:

@:forward
abstract NativeStructArray<T:{}>(T) {<...>}

And then in genphp i generate all field access to NativeStructArray as array access.
This approach will allow type checking, code completion and documentation hints. Otherwise it feels like coding php with Haxe syntax )
Unfortunately due to current issue i can't detect NativeStructArray in generator.

@back2dos
Copy link
Member

back2dos commented Oct 5, 2017

Well, one way would be to use @:enum abstract based GADTs: https://try.haxe.org/#cdC7D

Another one might be something like:

@:native("array")
extern class ProcStatus {
  var command(default, null):String;
  var pid(default, null):Int;
  // ...
}

Or you can use some explicit metadata (yay, more metadata \o/) or a marker interface or something.

@RealyUniqueName
Copy link
Member Author

RealyUniqueName commented Oct 5, 2017

Ye, i'm trying to avoid unnecessary metas )
Your solution looks interesting, but not too convenient. I'd like std lib to be more "fluid".
Marker interface may be a way to go if this issue will be considered wontfix.
But function procGetStatus():NativeStructArray<{pid:Int, command:String}> feels so much better.

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

Can we have this for Haxe4?

@Simn
Copy link
Member

Simn commented Feb 6, 2024

This was fixed in 9e7a820, the typed AST now has a cast:

[Field:String]
	[Cast:{ hostt : String }] [Local options(0):AnonOptions:AnonOptions]
	[FAnon:String] hostt:String

@Simn Simn closed this as completed Feb 6, 2024
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

No branches or pull requests

4 participants