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

fields that implement extension members don't work #548

Open
jmesserly opened this issue May 4, 2016 · 7 comments
Open

fields that implement extension members don't work #548

jmesserly opened this issue May 4, 2016 · 7 comments

Comments

@jmesserly
Copy link
Contributor

jmesserly commented May 4, 2016

Split from #520

The following test fails, because defineExtensionMember does not find a property descriptor for MyList.length, so it does not create [dartx.length] and thus ListMixin.forEach will throw.

import 'dart:collection';
import 'package:expect/expect.dart';

class MyList extends ListBase {
  int length = 4;
  int operator[](int x) => 42;
  void operator[]=(int x, val) {}
}

main() {
  var x = new MyList();
  int z = 0;
  // runtime error in here, but the bug was in how MyList type was set up
  x.forEach((y) { z += y; });
  Expect.equals(z, 4 * 42);
}
@jmesserly
Copy link
Contributor Author

@vsmenon @jacob314 -- thoughts on priority of this one?

@jmesserly
Copy link
Contributor Author

(also I think Jacob's fix might address this?)

@jmesserly jmesserly changed the title fields that define extension members don't work fields that implement extension members don't work May 5, 2016
@jacob314
Copy link
Contributor

jacob314 commented May 5, 2016

I don't think my CL impacts this as I only change behavior for native classes with extensions.

@vsmenon vsmenon added P1 high and removed P2 medium labels May 6, 2016
@vsmenon
Copy link
Contributor

vsmenon commented May 6, 2016

Here's another example I'm seeing (with same MyList above):

main() {
  dynamic x = new MyList();
  print(x.isEmpty);
}

This prints undefined. Same result if we replace dynamic with var.

@jmesserly
Copy link
Contributor Author

jmesserly commented May 6, 2016

ah chatted about that example, it's probably the same issue. Any ListBase/Mixin that uses a length field will be broken (because that is what ListMixin does: implement all members in terms of length and [] []=). Either we need to know to emit a getter/setter pair for that at compile time, or the defineExtensionMembers helper needs to know that it should do so.

There's also a related issue that "length" shows up twice in the defineExtensionMembers list (probably once from the getter and once from the setter)

@vsmenon vsmenon added P2 medium and removed P1 high labels May 6, 2016
@vsmenon
Copy link
Contributor

vsmenon commented May 6, 2016

Found a workaround, lowering back to P2.

This may be a different bug - changing length to a getter did not fix.

import 'dart:collection';

class MyList extends ListBase with ListMixin {
  int get length => 4;
  void set length(int x) {}
  int operator[](int x) => 42;
  void operator[]=(int x, val) {}
}

main() {
  dynamic x = new MyList();
  print(x.isEmpty);
}

@vsmenon
Copy link
Contributor

vsmenon commented May 6, 2016

Ok, syncing up and now the getter version works. Sorry for the noise.

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

No branches or pull requests

3 participants