Skip to content

DDC: Abstract getter or setter overrides declared in class clobber inherited concrete implementations #29914

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

Closed
greglittlefield-wf opened this issue Jun 16, 2017 · 9 comments
Assignees
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dev-compiler

Comments

@greglittlefield-wf
Copy link
Contributor

greglittlefield-wf commented Jun 16, 2017

Declaring abstract getters that override implementations from mixins or superclasses results in the getter being undefined when compiled in the dev compiler.

This issue is also present in the reverse, for setters: declaring abstract setters results in the setter being undefined, which actually throws.

Declaring both abstract getters and setters avoids the issue.

Example:

main() {
  // Abstract getter declared after concrete impl from mixin
  var instance1 = new Class1()..foo = 'bar';
  print(instance1.foo); // null

  // Abstract getter declared after concrete impl from superclass
  // Subclasses of this also fail
  var instance2 = new Class2()..foo = 'bar';
  print(instance2.foo); // null

  // Abstract getter declared in mixin after concrete impl from superclass
  var instance3 = new Class3()..foo = 'bar';
  print(instance3.foo); // bar

  // Abstract getter declared in interface after concrete impl from superclass
  var instance4 = new Class4()..foo = 'bar';
  print(instance4.foo); // bar

  // Mixin overriding abstract getter declared in superclass, which is declared after concrete impl from mixin
  var instance5 = new Class5()..foo = 'bar';
  print(instance5.foo); // bar
}

abstract class GetterAbstract {
  get foo;
}

class GetterConcrete {
  var _foo;

  get foo => _foo;
  set foo(value) => _foo = value;
}

class Class1 extends Object with GetterConcrete {
  get foo;
}

class Class2 extends GetterConcrete {
  get foo;
}

class Class3 extends GetterConcrete with GetterAbstract {}

class Class4 extends GetterConcrete implements GetterAbstract {}

abstract class _Helper_Class5 extends GetterConcrete {
  get foo;
}
class Class5 extends _Helper_Class5 with GetterConcrete {}

This is caused by the setter being redefined for these classes, but not the getter:

  list__main.Class2 = class Class2 extends list__main.GetterConcrete {
    set foo(value) {
      super.foo = value;
    }
  };
  (list__main.Class2.new = function() {
    list__main.Class2.__proto__.new.call(this);
  }).prototype = list__main.Class2.prototype;

You can see this issue in this ES6 reduced test case:

class A {
  get foo() {
    return 'foo'
  }
  set foo(value) {}
}

class B extends A {}

class C extends A {
  set foo(value) {}
}

var b = new B();
console.log(b.foo); // 'foo'

var c = new C();
console.log(c.foo); // undefined

We frequently use this pattern of overriding getters in order to update doc comments for subclasses, so it's unfortunate that this is a problem.

This issue is present in Dart 1.24.0 and 1.25.0-dev.1.0

@greglittlefield-wf greglittlefield-wf changed the title DDC: Abstract getter overrides declared in class clobber inherited concrete implementations DDC: Abstract getter or setter overrides declared in class clobber inherited concrete implementations Jun 16, 2017
@vsmenon vsmenon added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) S1 high labels Jun 19, 2017
@greglittlefield-wf
Copy link
Contributor Author

Possibly related: #29935

@robbecker-wf
Copy link

@kevmoo # 2 DDC issue for Workiva

@jmesserly
Copy link

this still reproduces (both dartdevc and dartdevk)

@jmesserly jmesserly self-assigned this Apr 21, 2018
@robbecker-wf
Copy link

any update on this @jmesserly ? Is this fixed under Dart 2 DDC?

@jmesserly
Copy link

It is not fixed yet.

@munificent munificent added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed S1 high labels Jun 21, 2018
@robbecker-wf
Copy link

ping. wondering if this will be addressed after dart 2 ships.

@jmesserly
Copy link

ping. wondering if this will be addressed after dart 2 ships.

@robbecker-wf sorry it took so long to get back to you, I've been out of the office.

Yeah I would like to see if we can fix this now that Dart 2 has shipped.

@jmesserly
Copy link

Sent out a fix for review: https://dart-review.googlesource.com/c/sdk/+/72073

@robbecker-wf
Copy link

Hey, thanks @jmesserly !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dev-compiler
Projects
None yet
Development

No branches or pull requests

6 participants