Add experimental expando support.

Add experimental expando support.

R=iposva@google.com
BUG=
TEST=

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

[Ivan Posva, 2012-06-11 11:18:33]

https://chromiumcodereview.appspot.com/10534089/diff/1/corelib/src/expando.dart
File corelib/src/expando.dart (right):

https://chromiumcodereview.appspot.com/10534089/diff/1/corelib/src/expando.da...
corelib/src/expando.dart:5: interface Expando<T> default
ExpandoImplementation<T> {
Private implementation?

https://chromiumcodereview.appspot.com/10534089/diff/1/corelib/src/expando.da...
corelib/src/expando.dart:17: T operator [](Object object);
What is the result if the value does not exist? Is it legal to associate null
with a key, if so we will also need a has.

https://chromiumcodereview.appspot.com/10534089/diff/1/corelib/src/expando.da...
corelib/src/expando.dart:23: 
Deleting a value is something we discussed. In the JS case this might mean to
set the property to null or a deleted sentinel.

https://chromiumcodereview.appspot.com/10534089/diff/1/tests/corelib/corelib....
File tests/corelib/corelib.status (right):

https://chromiumcodereview.appspot.com/10534089/diff/1/tests/corelib/corelib....
tests/corelib/corelib.status:8: expando_test: Fail
The test seems to be missing from the CL.

[Ivan Posva, 2012-06-11 12:08:08]

https://chromiumcodereview.appspot.com/10534089/diff/6001/tests/corelib/expan...
File tests/corelib/expando_test.dart (right):

https://chromiumcodereview.appspot.com/10534089/diff/6001/tests/corelib/expan...
tests/corelib/expando_test.dart:13: }
Currently you are only testing adding a specific expando to one object, but not
adding the same expando property to many objects. How about adding a static or
top-level expando to all tested objects?

https://chromiumcodereview.appspot.com/10534089/diff/6001/tests/corelib/expan...
tests/corelib/expando_test.dart:38: Expando<int> alternative = new
Expando('myexpando');
You might also want to do this while having the original expando registered.

[kasperl, 2012-06-11 12:44:39]

PTAL.

https://chromiumcodereview.appspot.com/10534089/diff/6001/tests/corelib/expan...
File tests/corelib/expando_test.dart (right):

https://chromiumcodereview.appspot.com/10534089/diff/6001/tests/corelib/expan...
tests/corelib/expando_test.dart:13: }
On 2012/06/11 12:08:08, Ivan Posva wrote:
> Currently you are only testing adding a specific expando to one object, but
not
> adding the same expando property to many objects. How about adding a static or
> top-level expando to all tested objects?

Done.

https://chromiumcodereview.appspot.com/10534089/diff/6001/tests/corelib/expan...
tests/corelib/expando_test.dart:38: Expando<int> alternative = new
Expando('myexpando');
On 2012/06/11 12:08:08, Ivan Posva wrote:
> You might also want to do this while having the original expando registered.

Done.

[Ivan Posva, 2012-06-11 13:54:37]

LGTM -ip

https://chromiumcodereview.appspot.com/10534089/diff/5003/corelib/src/expando...
File corelib/src/expando.dart (right):

https://chromiumcodereview.appspot.com/10534089/diff/5003/corelib/src/expando...
corelib/src/expando.dart:19: * [Expando]s with the same name does not
short comment

[kasperl, 2012-06-12 05:16:05]

https://chromiumcodereview.appspot.com/10534089/diff/5003/corelib/src/expando...
File corelib/src/expando.dart (right):

https://chromiumcodereview.appspot.com/10534089/diff/5003/corelib/src/expando...
corelib/src/expando.dart:19: * [Expando]s with the same name does not
On 2012/06/11 13:54:37, Ivan Posva wrote:
> short comment

Done.

[sra1, 2012-06-12 07:10:26]

I need something like this for some dart:html APIs.

The API and documentation are missing a few details.  If you can address my
comments in a follow-up CL I should be able to use Expando.
Currently I have my own expando-like mechanism which I will otherwise use until
Expando matures.

https://chromiumcodereview.appspot.com/10534089/diff/5004/corelib/src/expando...
File corelib/src/expando.dart (right):

https://chromiumcodereview.appspot.com/10534089/diff/5004/corelib/src/expando...
corelib/src/expando.dart:6: * An [Expando] allows adding new properties to
objects.
It does not look like adding properties to an object.
I can't start writing obj.foo or obj[foo] where foo is the Expando.
I have to write foo[obj] which looks more like an identity map.
So why is this called 'Expando' rather than 'IdentityMap' or something similar?

Ultimately this is a map from instances to values with some lifetime guarantees.
The values attached to an object via an Expando will be GC-ed if the object is
GC-ed and is the only reference to the value is via the Expando.  It is an
implementation detail that the information directly hangs off the instance.

https://chromiumcodereview.appspot.com/10534089/diff/5004/corelib/src/expando...
corelib/src/expando.dart:17: * Creates a new [Expando]. The optional name is
only used for
'only used' - I'm confused.
If I create two const [Expando]s with the same name, the behaviour is not
(obviously) specified in this one-sentence comment.

Please spell out the const and non-const cases.
Significant different behaviors deserve separate sentences.

The comment must at least explain the coding pattern for how a package should
use the API to add a field/expando that does not conflict with other packages.

https://chromiumcodereview.appspot.com/10534089/diff/5004/corelib/src/expando...
corelib/src/expando.dart:27: * [null].
Explain when the call will fail,

https://chromiumcodereview.appspot.com/10534089/diff/5004/lib/compiler/implem...
File lib/compiler/implementation/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/10534089/diff/5004/lib/compiler/implem...
lib/compiler/implementation/lib/js_helper.dart:573: if (object is bool || object
is num || object is String) {
This predicate is in the wrong place.  A public API that uses the API needs to
be doing the check, and possibly exporting the predicate.

https://chromiumcodereview.appspot.com/10534089/diff/5004/lib/compiler/implem...
lib/compiler/implementation/lib/js_helper.dart:581: if (object is bool || object
is num || object is String) {
Ditto.

https://chromiumcodereview.appspot.com/10534089/diff/5004/tests/corelib/expan...
File tests/corelib/expando_test.dart (right):

https://chromiumcodereview.appspot.com/10534089/diff/5004/tests/corelib/expan...
tests/corelib/expando_test.dart:64: var e0 = const Expando<int>('horse');
Add 'difficult' names:
'prototype'
'toString'
'__proto__'

https://chromiumcodereview.appspot.com/10534089/diff/5004/tests/corelib/expan...
tests/corelib/expando_test.dart:94: testIllegal() {
Illegal cases make the Expando API difficult to use.

My use case for dart:html is to walk the object graph and translate general
forms of List<T> and Map<K,V> to base representations.  I need a private
'visited' flag, for which an Expando is the only feasible solution to date.  How
do I know when I should use and when I should avoid my 'visited' Expando?

Please add a public legality predicate so that general object graph traversal
can be written without knowledge of implementation details.

[kasperl, 2012-06-14 15:33:44]

On 2012/06/12 07:10:26, sra1 wrote:
> I need something like this for some dart:html APIs.
> 
> The API and documentation are missing a few details.  If you can address my
> comments in a follow-up CL I should be able to use Expando.
> Currently I have my own expando-like mechanism which I will otherwise use
until
> Expando matures.
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/corelib/src/expando...
> File corelib/src/expando.dart (right):
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/corelib/src/expando...
> corelib/src/expando.dart:6: * An [Expando] allows adding new properties to
> objects.
> It does not look like adding properties to an object.
> I can't start writing obj.foo or obj[foo] where foo is the Expando.
> I have to write foo[obj] which looks more like an identity map.
> So why is this called 'Expando' rather than 'IdentityMap' or something
similar?
> 
> Ultimately this is a map from instances to values with some lifetime
guarantees.
> The values attached to an object via an Expando will be GC-ed if the object is
> GC-ed and is the only reference to the value is via the Expando.  It is an
> implementation detail that the information directly hangs off the instance.
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/corelib/src/expando...
> corelib/src/expando.dart:17: * Creates a new [Expando]. The optional name is
> only used for
> 'only used' - I'm confused.
> If I create two const [Expando]s with the same name, the behaviour is not
> (obviously) specified in this one-sentence comment.
> 
> Please spell out the const and non-const cases.
> Significant different behaviors deserve separate sentences.
> 
> The comment must at least explain the coding pattern for how a package should
> use the API to add a field/expando that does not conflict with other packages.
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/corelib/src/expando...
> corelib/src/expando.dart:27: * [null].
> Explain when the call will fail,
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/lib/compiler/implem...
> File lib/compiler/implementation/lib/js_helper.dart (right):
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/lib/compiler/implem...
> lib/compiler/implementation/lib/js_helper.dart:573: if (object is bool ||
object
> is num || object is String) {
> This predicate is in the wrong place.  A public API that uses the API needs to
> be doing the check, and possibly exporting the predicate.
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/lib/compiler/implem...
> lib/compiler/implementation/lib/js_helper.dart:581: if (object is bool ||
object
> is num || object is String) {
> Ditto.
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/tests/corelib/expan...
> File tests/corelib/expando_test.dart (right):
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/tests/corelib/expan...
> tests/corelib/expando_test.dart:64: var e0 = const Expando<int>('horse');
> Add 'difficult' names:
> 'prototype'
> 'toString'
> '__proto__'
> 
>
https://chromiumcodereview.appspot.com/10534089/diff/5004/tests/corelib/expan...
> tests/corelib/expando_test.dart:94: testIllegal() {
> Illegal cases make the Expando API difficult to use.
> 
> My use case for dart:html is to walk the object graph and translate general
> forms of List<T> and Map<K,V> to base representations.  I need a private
> 'visited' flag, for which an Expando is the only feasible solution to date. 
How
> do I know when I should use and when I should avoid my 'visited' Expando?
> 
> Please add a public legality predicate so that general object graph traversal
> can be written without knowledge of implementation details.

Thanks for the feedback, Stephen. I'll try to work with Ivan to address it and
to get this to run on the VM too.