Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Constructors (named or default) may be invoked as instance methods #162

Open
vsmenon opened this issue May 4, 2015 · 4 comments
Open

Constructors (named or default) may be invoked as instance methods #162

vsmenon opened this issue May 4, 2015 · 4 comments

Comments

@vsmenon
Copy link
Contributor

vsmenon commented May 4, 2015

In the following code, both A and A.from may be invoked as instance methods on an instance of A.

class A<T> {
  T x;

  A(this.x) { print("Constructing an A"); }

  A.from(A<T> a) {
    x = a.x;
  }
}

dynamic z;

main() {
  var y = new A<int>(1);
  var x = new A<int>(2);
  z = y;
  z.from(x);
  print(y.x); // Prints '2'
  z.A(); // Prints 'Constructing an A'
}
@jmesserly
Copy link
Contributor

That's a bug in dsend (formerly dinvoke). Dynamic helpers shouldn't invoke our initializer methods.

Possible quick fix: have defineNamedConstructor mark the method as being a named constructor, or even altogether remove it from the prototype (assuming that won't break "super").

As far as generating the named constructor as an instance method, it's by design (ish), for now. Context is #51. I stared writing more, but it's a bit tangential, so opened another bug #163

@vsmenon
Copy link
Contributor Author

vsmenon commented May 4, 2015

Note, this has no dynamic invokes and hits this issue. It prints 25 in the VM but 50 in DDC:

class A {
  int x;

  A(this.x);

  from(int y) {
    x = y;
  }
}

class B extends A {
  B.from(int z) : super(z*2);
}

main() {
  A a = new B.from(12);
  a.from(a.x + 1);

  print(a.x);
} 

We could statically reject this code perhaps - i.e., reject a named constructor that "conflicts" with an instance method.

Removing from the prototype is an interesting idea.

FWIW, I'm not sure we're hitting this bug in the wild yet. I only encountered this when trying to understand #158.

@jmesserly
Copy link
Contributor

Yeah, so, we were attempting to rely on the fact that static/instance/named constructor names can't conflict, at least in the same class. Similarly static member can't override instance member.

class B extends A {
  B.from(int z) : super(z*2);
  // Error: Duplicate definition of 'from'.
  from(int y) => super.from(y);
}
// fails too
// Error: Static member cannot override instance member 'from' of 'A'.
class B extends A {
  static from(int y) => 42;
}

all specified in
http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-408.pdf

That named constructors can have the same name as an inherited member is bizarrely inconsistent ... I could see always allowing static/constructors vs instance methods to be separate namespaces. But everywhere except named constructors ... so now we have a method we can't override in the derived class ... all I can think is "WAT"

anyway, I think #163 covers that issue, we'd have to redesign named constructors to be a lot less readable if we want that to work. I don't think it's worth it for this case (IMO) but for "strong mode" support, it may be worth considering.

@jmesserly
Copy link
Contributor

@vsmenon had a related example:

class Base {
  Derived() => print("This shouldn't be called");
}

class Derived extends Base {}

main() => print(new Derived());

unsure if it triggers (depends on whether we generate a Derived initializer in Derived), but it's the kind of problem that happens with the current design if we aren't super careful.

btw -- yet another reason to try and fix #163

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

2 participants