Tests for instanceMembers, staticMembers, toplevelMembers.

Tests and docs for instanceMembers, staticMembers, toplevelMembers.

BUG=http://dartbug.com/14632
BUG=http://dartbug.com/14633
R=ahe@google.com, gbracha@google.com

Committed: https://code.google.com/p/dart/source/detail?r=29593

[rmacnak, 2013-10-25 21:58:35]

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
File tests/lib/mirrors/instance_members_unimplemented_interface_test.dart
(right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
tests/lib/mirrors/instance_members_unimplemented_interface_test.dart:23:
[#implementMe,
?

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
File tests/lib/mirrors/instance_members_with_override_test.dart (right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
tests/lib/mirrors/instance_members_with_override_test.dart:82:
Expect.equals(sMirror, cMirror.instanceMembers[#notOverridden].owner);
?

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
File tests/lib/mirrors/toplevel_members_test.dart (right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
tests/lib/mirrors/toplevel_members_test.dart:32: #Predicate,
?

[gbracha, 2013-10-26 18:37:48]

https://codereview.chromium.org/44813004/diff/30001/sdk/lib/mirrors/mirrors.dart
File sdk/lib/mirrors/mirrors.dart (right):

https://codereview.chromium.org/44813004/diff/30001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:687: Map<Symbol, MethodMirror> get toplevelMembers;
So is the agreed name for api? It's not great, but I can live with it. There's
no spec for it though. It needs to be added.

https://codereview.chromium.org/44813004/diff/30001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:850: * by fields are included iff the field is
non-final.
Since finals no longer imply a setter, we can drop the "iff the field is
non-final".

https://codereview.chromium.org/44813004/diff/30001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:857: Map<Symbol, MethodMirror> get staticMembers;
Add analogous spec for static members.

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
File tests/lib/mirrors/instance_members_unimplemented_interface_test.dart
(right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
tests/lib/mirrors/instance_members_unimplemented_interface_test.dart:23:
[#implementMe,
On 2013/10/25 21:58:36, Ryan Macnak wrote:
> ?

No, C says it implements I but it does not. Classes do not inherit members from
via the implements clause.
C should also generate a static warning.

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
File tests/lib/mirrors/instance_members_with_override_test.dart (right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
tests/lib/mirrors/instance_members_with_override_test.dart:82:
Expect.equals(sMirror, cMirror.instanceMembers[#notOverridden].owner);
On 2013/10/25 21:58:36, Ryan Macnak wrote:
> ?

Hmm. So the theory is that the actual method that can be invoked is the one that
shows up in this case (and by extension, in various combinations involving
mixins). I guess that is fair.

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
File tests/lib/mirrors/toplevel_members_test.dart (right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
tests/lib/mirrors/toplevel_members_test.dart:32: #Predicate,
On 2013/10/25 21:58:36, Ryan Macnak wrote:
> ?

So the notion of API for a library is alas hazy.  There is no library object,
but if/when there is on it would likely have accessors for the type objects.
Predicate is no different from the other types.

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
tests/lib/mirrors/toplevel_members_test.dart:45: #Predicate,
So are all the type getters synthetic? I'm not so sure about that.

[ahe, 2013-10-29 10:14:38]

LGTM

https://codereview.chromium.org/44813004/diff/30001/sdk/lib/mirrors/mirrors.dart
File sdk/lib/mirrors/mirrors.dart (right):

https://codereview.chromium.org/44813004/diff/30001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:858: 
Extra line.

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
File tests/lib/mirrors/instance_members_with_override_test.dart (right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
tests/lib/mirrors/instance_members_with_override_test.dart:56:
Expect.equals(sMirror, sMirror.instanceMembers[const Symbol('setter=')].owner);
Long line.

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/instanc...
tests/lib/mirrors/instance_members_with_override_test.dart:79:
Expect.equals(cMirror, cMirror.instanceMembers[const Symbol('setter=')].owner);
Long line.

[rmacnak, 2013-10-30 18:50:42]

Updated doc.

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
File tests/lib/mirrors/toplevel_members_test.dart (right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
tests/lib/mirrors/toplevel_members_test.dart:32: #Predicate,
On 2013/10/26 18:37:48, gbracha wrote:
> On 2013/10/25 21:58:36, Ryan Macnak wrote:
> > ?
> 
> So the notion of API for a library is alas hazy.  There is no library object,
> but if/when there is on it would likely have accessors for the type objects.
> Predicate is no different from the other types.

I'm thinking this should behave how a prefix would behave if it were an object:
includes declared members that are not hidden and exported stuff. Perhaps
libraries need both a exportedMembers and a localMembers... Uck.

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
tests/lib/mirrors/toplevel_members_test.dart:45: #Predicate,
On 2013/10/26 18:37:48, gbracha wrote:
> So are all the type getters synthetic? I'm not so sure about that.

They weren't hand-written.

The Newspeak mirrors don't let you ask such a question. There we don't have an
accessor that aggregates slots, methods and nested classes. We merely have
canUnderstand:, which is used to determine whether a class is an application or
test configuration and to highlight suspect implicit receiver sends.

Perhaps the api accessors should return Map<Symbol, DeclarationMirror> such that
members[#someField] and members[#someField=] both answer the VariableMirror. A
library could then put TypeMirrors in its map, and we could drop isSynthetic.

[rmacnak, 2013-10-30 18:51:57]

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
File tests/lib/mirrors/toplevel_members_test.dart (right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
tests/lib/mirrors/toplevel_members_test.dart:45: #Predicate,
On 2013/10/30 18:50:42, Ryan Macnak wrote:
> On 2013/10/26 18:37:48, gbracha wrote:
> > So are all the type getters synthetic? I'm not so sure about that.
> 
> They weren't hand-written.
> 
> The Newspeak mirrors don't let you ask such a question. There we don't have an
> accessor that aggregates slots, methods and nested classes. We merely have
> canUnderstand:, which is used to determine whether a class is an application
or
> test configuration and to highlight suspect implicit receiver sends.
> 
> Perhaps the api accessors should return Map<Symbol, DeclarationMirror> such
that
> members[#someField] and members[#someField=] both answer the VariableMirror. A
> library could then put TypeMirrors in its map, and we could drop isSynthetic.

It would also allow the maps to provide the functionality of instanceLookup et
al.

[gbracha, 2013-10-30 18:58:40]

tests lgtm

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
File tests/lib/mirrors/toplevel_members_test.dart (right):

https://codereview.chromium.org/44813004/diff/30001/tests/lib/mirrors/topleve...
tests/lib/mirrors/toplevel_members_test.dart:45: #Predicate,
On 2013/10/30 18:51:58, Ryan Macnak wrote:
> On 2013/10/30 18:50:42, Ryan Macnak wrote:
> > On 2013/10/26 18:37:48, gbracha wrote:
> > > So are all the type getters synthetic? I'm not so sure about that.
> > 
> > They weren't hand-written.
> > 
> > The Newspeak mirrors don't let you ask such a question. There we don't have
an
> > accessor that aggregates slots, methods and nested classes. We merely have
> > canUnderstand:, which is used to determine whether a class is an application
> or
> > test configuration and to highlight suspect implicit receiver sends.
> > 
> > Perhaps the api accessors should return Map<Symbol, DeclarationMirror> such
> that
> > members[#someField] and members[#someField=] both answer the VariableMirror.
A
> > library could then put TypeMirrors in its map, and we could drop
isSynthetic.
> 
> It would also allow the maps to provide the functionality of instanceLookup et
> al.

Since this is topLevelMembers, not declarations, I guess we can consider the
type accessors synthetic.  In Newspeak they aren't because these are real
messagse on objects. If we ever get to first class libraries, this might be nice
to change. Not sure if change will be possible. For now, I'd keep as is.

[rmacnak, 2013-10-30 21:59:33]

marked failures and assigned issues.

[rmacnak, 2013-10-30 22:10:03]

Committed patchset #3 manually as r29593 (presubmit successful).