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

Null<T> lost in type inference #5517

Open
nadako opened this issue Aug 11, 2016 · 6 comments
Open

Null<T> lost in type inference #5517

nadako opened this issue Aug 11, 2016 · 6 comments
Assignees
Labels
Milestone

Comments

@nadako
Copy link
Member

nadako commented Aug 11, 2016

class Node<T> {
    public function new(value:T, next:Node<T>) {
    }
}

class Main {
    static function main() {
        var i:Null<Int> = 1;
        new Node(i, new Node(1, null));
    }
}

generated AST (note the inner Node is a Node<Int>, not Node<Null<Int>>):

[New:Node<Null<Int>>]
    Node<Null<Int>>
    [Cast:Null<Int>] [Const:Int] 1
    [New:Node<Int>]
        Node<Int>
        [Const:Int] 1
        [Const:Node<Int>] null
}

this is problematic for C# target where we have to generate a copy-cast right now

@nadako
Copy link
Member Author

nadako commented Aug 11, 2016

The example was taken from Issue3681.hx which failed to compile for C# with -D fast-cast if I fix the redundant __hx_cast generation.

@Simn Simn added this to the 3.4 milestone Aug 25, 2016
@Simn
Copy link
Member

Simn commented Aug 25, 2016

I don"t even know where to start looking here. I"m assuming it"s not a regression?

@Simn Simn modified the milestones: 3.4, 4.0 Jan 9, 2017
@Simn
Copy link
Member

Simn commented Mar 6, 2017

Is that actually unexpected? You call new Node(1, null), so T gets bound to Int.

@nadako
Copy link
Member Author

nadako commented Mar 6, 2017

Yes, but the problem is that next compiler argument receives a different T (Int instead of Null<Int> in this case).

@Simn
Copy link
Member

Simn commented Mar 6, 2017

Yes, because they unify. It"s clearer if you split up the call:

var i:Null<Int> = 1;
var next = new Node(1, null);
new Node(i, next);

The only problem here is that the second argument is silently cast from Node<Int> to Node<Null<Int>> because Null<T> is a typedef. That"s not a type-inference issue though.

You could maybe make the argument that in the merged call, top-down inference should cause the inner type to be inferred to Null<T> straight away. I"m not quite sure about that, but it wouldn"t solve the problem in the split-up version anyway.

@nadako
Copy link
Member Author

nadako commented Mar 6, 2017

You could maybe make the argument that in the merged call, top-down inference should cause the inner type to be inferred to Null straight away.

Yeah that what I was expecting here originally. The split-up version is more explicit and should cause an error...

@Simn Simn modified the milestones: Release 4.0, Design Apr 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants