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

inline constructor of abstracts #11526

Closed
Tracked by #11583 ...
ncannasse opened this issue Jan 30, 2024 · 4 comments
Closed
Tracked by #11583 ...

inline constructor of abstracts #11526

ncannasse opened this issue Jan 30, 2024 · 4 comments

Comments

@ncannasse
Copy link
Member

We recently changed our Heaps Vector implementation to use an abstract so we can have some operator overloading, but we noticed some regression due to constructor inline not being triggered anymore.

We can reproduce with the following sample:

class P {
   public var x : Float;
   public inline function new(x=0) this.x = x;
}


@:forward
abstract PA(P) {
  public inline function new(x) this = new P(x);
}

class Test {
  static function foo(x:Int) {
    var p1 = { v : new P(x) }
    trace(p1.v.x);
    var p2 = { v : new PA(5) }
    trace(p2.v.x);
  }
  static function main() {
    foo(5);
  }
}

It seems inline new is not handled for abstracts ?

@Simn
Copy link
Member

Simn commented Jan 30, 2024

cc @basro

@basro
Copy link
Contributor

basro commented Jan 30, 2024

A quick note:
Inlining abstracts is working, it seems the combination of abstracts and anonymous structs is what is cancelling the inlining.

This inlines everything:

class P {
   public var x : Float;
   public inline function new(x) this.x = x;
}


@:forward
abstract PA(P) {
  public inline function new(x) this = new P(x);
}

class Test {
  static function foo(x:Int) {
    var p1 = { v : new P(x) }
    trace(p1.v.x);
    var p2 = new PA(5);
    trace(p2.x);
  }
  static function main() {
    foo(5);
  }
}

@basro
Copy link
Contributor

basro commented Jan 30, 2024

I have not tested with the compiler but my bet is that the cancellation is happening in this line.

It is likely that the type of the inlined object and the type of the field access are not strictly equal.

@Simn
Copy link
Member

Simn commented Jan 30, 2024

Good intuition:

p2<-6565>.v
  v_type: TAbstract(PA, [])
  etype: TInst(P, [])

I don't know where that goes wrong though because the type is initially correct:

 17 |   $type(p2.v);
    |         ^^^^
    | PA

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

3 participants