implement private members, fixes #74

implement private members, fixes #74

naturally, lots of things to iterate on, but this expresses the key idea (per-library ES6 symbols for privacy).

R=vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/2167e1ac3f4de544ae7d30af917f2015f3d90622

[Jennifer Messerly, 2015-02-28 00:51:40]

jmesserly@google.com changed reviewers:
+ vsm@google.com

[Jennifer Messerly, 2015-03-02 18:48:16]

https://codereview.chromium.org/963343002/diff/1/lib/src/codegen/js_codegen.dart
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/963343002/diff/1/lib/src/codegen/js_codegen.d...
lib/src/codegen/js_codegen.dart:48: final _privateNames = new Set<String>();
hmmm, another thought ... we could emit these as we go ... so they end up not
all at the top of the file. That might make the code feel a bit more natural?

[vsm, 2015-03-02 18:52:06]

On 2015/03/02 18:48:16, John Messerly wrote:
>
https://codereview.chromium.org/963343002/diff/1/lib/src/codegen/js_codegen.dart
> File lib/src/codegen/js_codegen.dart (right):
> 
>
https://codereview.chromium.org/963343002/diff/1/lib/src/codegen/js_codegen.d...
> lib/src/codegen/js_codegen.dart:48: final _privateNames = new Set<String>();
> hmmm, another thought ... we could emit these as we go ... so they end up not
> all at the top of the file. That might make the code feel a bit more natural?

One of my concerns is that introspecting on and/or invoking privates from the
dev tools will be a little harder.  Perhaps we should stick with compile-time
enforcement?  Though that does mean we'd need to deal with duplicate private
symbols.  What do you think?

[Jennifer Messerly, 2015-03-02 18:59:50]

Hmmm. That's interesting. ES6 is intending the features to be used exactly like
this though. So I would presume they intend to address the dev tools issue**.

There are plenty of JS libraries already, that use WeakMaps and such as a way of
adding state to objects in private fashion.

We could certainly do renaming. I just worry it doesn't have the right semantics
in an ES6 world. e.g. you can override a private field like "_foo" from an ES6
subclass.

** If dev tools is really the sticking point, there are several options. We
could use a runtime helper instead of `Symbol` directly so we can swap it out
for a String property name. Also, it's quite possible in EcmaScript to write a
`callPrivate(obj, "_foo", args)` method where callPrivate uses ES6 reflection to
get at the field/property/method...

[Jennifer Messerly, 2015-03-02 20:21:26]

On 2015/03/02 18:59:50, John Messerly wrote:
> Hmmm. That's interesting. ES6 is intending the features to be used exactly
like
> this though. So I would presume they intend to address the dev tools issue**.
> 
> There are plenty of JS libraries already, that use WeakMaps and such as a way
of
> adding state to objects in private fashion.
> 
> We could certainly do renaming. I just worry it doesn't have the right
semantics
> in an ES6 world. e.g. you can override a private field like "_foo" from an ES6
> subclass.
> 
> ** If dev tools is really the sticking point, there are several options. We
> could use a runtime helper instead of `Symbol` directly so we can swap it out
> for a String property name. Also, it's quite possible in EcmaScript to write a
> `callPrivate(obj, "_foo", args)` method where callPrivate uses ES6 reflection
to
> get at the field/property/method...

for example:

var obj = { [Symbol('_foo')]() { return 42; } };
var sym = Object.getOwnPropertySymbols(obj).filter((s) => s.toString() ==
'Symbol(_foo)')[0]
obj[sym]() // 42

Another thought. We could export these symbols on the library:

  // ES5
  let _foo = exports._foo = Symbol('_foo');
  // ES6
  export let _foo = Symbol('_foo');

That would let folks easily access these from the dev console.
(Though I'm still not sure that is desirable. In Dartium, you have to use the
library drop down if you want to call privates ... presumably ES6 modules may
have a similar feature, where the currently active module controls what scope
you see.)

[vsm, 2015-03-02 21:17:01]

Note, you have a bunch of git conflicts in your latest upload.

Interspersing the symbol declarations sounds great.

https://codereview.chromium.org/963343002/diff/20001/test/codegen/expect/_iso...
File test/codegen/expect/_isolate_helper/_isolate_helper.js (right):

https://codereview.chromium.org/963343002/diff/20001/test/codegen/expect/_iso...
test/codegen/expect/_isolate_helper/_isolate_helper.js:50: <<<<<<< HEAD
Resolve this?  :-)

https://codereview.chromium.org/963343002/diff/20001/test/codegen/expect/_iso...
test/codegen/expect/_isolate_helper/_isolate_helper.js:136: <<<<<<< HEAD
Ditto

https://codereview.chromium.org/963343002/diff/20001/test/codegen/expect/_iso...
test/codegen/expect/_isolate_helper/_isolate_helper.js:316: <<<<<<< HEAD
ditto ... and a few more below

[Jennifer Messerly, 2015-03-02 21:26:10]

On 2015/03/02 21:17:01, vsm wrote:
> Note, you have a bunch of git conflicts in your latest upload.
> 
> Interspersing the symbol declarations sounds great.
> 
>
https://codereview.chromium.org/963343002/diff/20001/test/codegen/expect/_iso...
> File test/codegen/expect/_isolate_helper/_isolate_helper.js (right):
> 
>
https://codereview.chromium.org/963343002/diff/20001/test/codegen/expect/_iso...
> test/codegen/expect/_isolate_helper/_isolate_helper.js:50: <<<<<<< HEAD
> Resolve this?  :-)
> 
>
https://codereview.chromium.org/963343002/diff/20001/test/codegen/expect/_iso...
> test/codegen/expect/_isolate_helper/_isolate_helper.js:136: <<<<<<< HEAD
> Ditto
> 
>
https://codereview.chromium.org/963343002/diff/20001/test/codegen/expect/_iso...
> test/codegen/expect/_isolate_helper/_isolate_helper.js:316: <<<<<<< HEAD
> ditto ... and a few more below

oops! forgot to copy over test baselines ... working on it.

[Jennifer Messerly, 2015-03-02 22:38:02]

PTAL! I also fixed it to generate symbols closer to their use

[vsm, 2015-03-03 13:52:53]

lgtm

[Jennifer Messerly, 2015-03-03 14:44:30]

Committed patchset #4 (id:50001) manually as
2167e1ac3f4de544ae7d30af917f2015f3d90622 (presubmit successful).