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

Should static extensions really be applied through implicit casts? #5924

Closed
back2dos opened this issue Jan 5, 2017 · 8 comments
Closed

Should static extensions really be applied through implicit casts? #5924

back2dos opened this issue Jan 5, 2017 · 8 comments
Assignees
Milestone

Comments

@back2dos
Copy link
Member

back2dos commented Jan 5, 2017

Having seen #5888 reminded me of how this current behavior creeps me out: if there's a static extension for type A and there's an implicit conversion from another type B to A, then all static extensions for A are applied to B also.

Here's an example:

using Test;

class Test {
    static function main() {
        0.bar();//<-- can call static extensions of `Foo` because there's a `@:from Int`
        0.baz();//<-- but cannot call methods of `Foo`
    }
    
    static function bar(f:Foo)
    	f.bar();
}

abstract Foo(String) {
    
    inline function new(v) this = v;
    
    @:from static function ofInt(i:Int) 
      return new Foo('$i');
    
    public function bar() {};
    public function baz() {};
}

This strikes me as pretty weird overall and quite inconsistent also. I really think static extensions should behave like methods as far as possible, but certainly not be applied more liberally. If anything, it is @:forward over abstract Abstract(Underlying) that should allow applying static extensions to Underlying to Abstract. The current behavior can be pretty unintuitive and introducing new implicit casts has far reaching and often undesireable effects:

using Test.FooTools;
using Test.BarTools;

class Test {
    static function main() {
        var foo = new Foo('foo');
        trace(foo.toString());//yields "Foo: foo" but uncomment the implicit Bar from Foo conversion below and you get "Bar: foo" because the static extension for Bar will get precedence over the static extension for Foo even though the type being dispatched on is Foo
    }    
}

abstract Foo(String) {
  public inline function new(s) this = s;
}

abstract Bar(String) {
    public inline function new(s) this = s;
    //@:from static function ofFoo(f:Foo) return new Bar('$f');
}

class FooTools {
    static public function toString(f:Foo) return 'Foo: $f';
}

class BarTools {
    static public function toString(f:Bar) return 'Bar: $f';
}

I keep on running into this repeatedly and it forces me to think very carefully about the order of my static extensions to avoid passing through unnecessary (and potentially expensive) implicit conversions. Not only that, I have to recheck the order every time I add a new implicit conversion. It is a breaking change of behavior became specified in #2152, but I really think the issue then was that the compiler silently generating invalid code and rejecting the code all together would have been the right call. Since there is no mention of static extensions being applied through implicit casts in the manual, my preference would be not to have to wait for Haxe 4.

@Simn Simn self-assigned this Jan 7, 2017
@Simn Simn added this to the 4.0 milestone Jan 7, 2017
@Simn
Copy link
Member

Simn commented Jan 7, 2017

Related: #5790

I think I agree, but I'll have to check the consequences of changing this.

@Simn
Copy link
Member

Simn commented Jun 3, 2017

Does anyone have any opinions on this? It's one of the things I consider breaking for Haxe 4.

@back2dos
Copy link
Member Author

back2dos commented Jun 3, 2017

Adding to my "deliberations" above, I would also like to mention that this would also remove some of the problems with operator overloading through static extensions (as far as I understand).

@gene-pavlovsky
Copy link

Juraj's logic makes total sense to me.

@gene-pavlovsky
Copy link

I have no idea why @Simn was unassigned when I just left a comment

@Simn Simn modified the milestones: Release 4.0, Design Apr 17, 2018
@Simn Simn self-assigned this Apr 18, 2018
@Simn Simn modified the milestones: Design, Release 4.0 Jun 6, 2018
@Simn
Copy link
Member

Simn commented Jun 6, 2018

#2152 has a conflicting test, but that's the only failure in our tests.

Let's break it

@Simn
Copy link
Member

Simn commented Jun 10, 2018

I'm holding back on this until I have a solution for #7142.

@kevinresol
Copy link
Contributor

kevinresol commented Mar 6, 2019

Is this change intentionally apply only to @:from fields but not from directives?

FYR the following code still generates BarTools.stringify({ i : 1}):

using Main.FooTools;
using Main.BarTools;

class Main {
	static function main() {
		var foo:Foo = {i:1}
		trace(foo.stringify());
	}
}

typedef Base = {i:Int};
@:forward abstract Foo(Base) from Base {}
@:forward abstract Bar(Base) from Base {}

class FooTools {
	public static function stringify(v:Foo) return 'Foo ' + v.i;
}

class BarTools {
	public static function stringify(v:Bar) return 'Bar ' + v.i;
}

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