 Chromium Code Reviews
 Chromium Code Reviews Issue 963343002:
  implement private members, fixes #74  (Closed) 
  Base URL: git@github.com:dart-lang/dev_compiler.git@master
    
  
    Issue 963343002:
  implement private members, fixes #74  (Closed) 
  Base URL: git@github.com:dart-lang/dev_compiler.git@master| Index: lib/src/codegen/js_codegen.dart | 
| diff --git a/lib/src/codegen/js_codegen.dart b/lib/src/codegen/js_codegen.dart | 
| index 1f69fefc77ace646444ca3f6ddc6c8f4f11fbdbb..a08c7531a576e6e388f8fc6a01de7611aa7db211 100644 | 
| --- a/lib/src/codegen/js_codegen.dart | 
| +++ b/lib/src/codegen/js_codegen.dart | 
| @@ -45,6 +45,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| final _exports = <String>[]; | 
| final _lazyFields = <VariableDeclaration>[]; | 
| final _properties = <FunctionDeclaration>[]; | 
| + final _privateNames = new Set<String>(); | 
| 
Jennifer Messerly
2015/03/02 18:48:16
hmmm, another thought ... we could emit these as w
 | 
| JSCodegenVisitor(LibraryInfo libraryInfo, TypeRules rules) | 
| : libraryInfo = libraryInfo, | 
| @@ -75,17 +76,17 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| } | 
| var name = _libraryName; | 
| + var symbolInit = _privateNames.map(_initPrivateSymbol); | 
| return new JS.Block([ | 
| js.statement('var #;', name), | 
| - js.statement(''' | 
| - (function (#) { | 
| - 'use strict'; | 
| - #; | 
| - })(# || (# = {})); | 
| - ''', [name, body, name, name]) | 
| + js.statement("(function (#) { 'use strict'; #; #; })(# || (# = {}));", | 
| + [name, symbolInit, body, name, name]) | 
| ]); | 
| } | 
| + JS.Statement _initPrivateSymbol(String name) => | 
| + js.statement('let # = Symbol(#);', [name, js.string(name, "'")]); | 
| + | 
| @override | 
| JS.Statement visitCompilationUnit(CompilationUnit node) { | 
| // TODO(jmesserly): scriptTag, directives. | 
| @@ -402,7 +403,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| var className = classDecl.name.name; | 
| var name = _constructorName(className, node.constructorName); | 
| - return js.statement('this.#(#);', [name, _visit(node.argumentList)]); | 
| + return js.statement('this.#(#);', | 
| + [_jsMemberName(name), _visit(node.argumentList)]); | 
| } | 
| JS.Statement _superConstructorCall(ClassDeclaration clazz, | 
| @@ -452,7 +454,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| if (p is DefaultFormalParameter) p = p.parameter; | 
| if (p is FieldFormalParameter) { | 
| var name = p.identifier.name; | 
| - body.add(js.statement('this.# = #;', [name, name])); | 
| + body.add(js.statement('this.# = #;', | 
| + [_jsMemberName(name), _visit(p)])); | 
| unsetFields.remove(name); | 
| } | 
| } | 
| @@ -482,7 +485,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| value = new JS.LiteralNull(); | 
| } | 
| } | 
| - body.add(js.statement('this.# = #;', [name, value])); | 
| + body.add(js.statement('this.# = #;', [_jsMemberName(name), value])); | 
| }); | 
| return _statement(body); | 
| @@ -550,7 +553,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| var params = _visit(node.parameters); | 
| if (params == null) params = []; | 
| - return new JS.Method(new JS.PropertyName(_jsMethodName(node.name.name)), | 
| + return new JS.Method(_jsMemberName(node.name.name), | 
| new JS.Fun(params, _visit(node.body)), | 
| isGetter: node.isGetter, | 
| isSetter: node.isSetter, | 
| @@ -643,7 +646,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| (e.library != libraryInfo.library || _needsModuleGetter(e))) { | 
| return js.call('#.#', [jsLibraryName(e.library), name]); | 
| } else if (currentClass != null && _needsImplicitThis(e)) { | 
| - return js.call('this.#', name); | 
| + return js.call('this.#', _jsMemberName(name)); | 
| + } else if (e is ParameterElement && e.isInitializingFormal) { | 
| + name = _fieldParameterName(name); | 
| } | 
| return new JS.VariableUse(name); | 
| } | 
| @@ -850,7 +855,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| result.add(new JS.Parameter(_jsNamedParameterName)); | 
| break; | 
| } | 
| - result.add(new JS.Parameter(param.identifier.name)); | 
| + result.add(_visit(param)); | 
| } | 
| return result; | 
| } | 
| @@ -1214,12 +1219,16 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| _visit(node.expression); | 
| @override | 
| - visitSimpleFormalParameter(SimpleFormalParameter node) => | 
| - _visit(node.identifier); | 
| + visitFormalParameter(FormalParameter node) => | 
| + new JS.Parameter(node.identifier.name); | 
| @override | 
| - visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) => | 
| - _visit(node.identifier); | 
| + visitFieldFormalParameter(FieldFormalParameter node) => | 
| + new JS.Parameter(_fieldParameterName(node.identifier.name)); | 
| + | 
| + /// Rename private names so they don't shadow the private field symbol. | 
| + // TODO(jmesserly): replace this ad-hoc rename with a general strategy. | 
| + _fieldParameterName(name) => name.startsWith('_') ? '\$$name' : name; | 
| @override | 
| JS.This visitThisExpression(ThisExpression node) => new JS.This(); | 
| @@ -1246,7 +1255,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| return js.call( | 
| 'dart.dload(#, #)', [_visit(target), js.string(name.name, "'")]); | 
| } else { | 
| - return js.call('#.#', [_visit(target), name.name]); | 
| + return js.call('#.#', [_visit(target), _jsMemberName(name.name)]); | 
| } | 
| } | 
| @@ -1594,11 +1603,34 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| return result; | 
| } | 
| - /// The following names are allowed for user-defined operators: | 
| + /// This handles member renaming for private names and operators. | 
| + /// | 
| + /// Private names are generated using ES6 symbols: | 
| + /// | 
| + /// // At the top of the module: | 
| + /// let _x = Symbol('_x'); | 
| + /// let _y = Symbol('_y'); | 
| + /// ... | 
| + /// | 
| + /// class Point { | 
| + /// Point(x, y) { | 
| + /// this[_x] = x; | 
| + /// this[_y] = y; | 
| + /// } | 
| + /// get x() { return this[_x]; } | 
| + /// get y() { return this[_y]; } | 
| + /// } | 
| + /// | 
| + /// For user-defined operators the following names are allowed: | 
| /// | 
| /// <, >, <=, >=, ==, -, +, /, ˜/, *, %, |, ˆ, &, <<, >>, []=, [], ˜ | 
| /// | 
| - /// For the indexing operators, we use `get` and `set` instead: | 
| + /// They generate code like: | 
| + /// | 
| + /// x['+'](y) | 
| + /// | 
| + /// There are three exceptions: [], []= and unary -. | 
| + /// The indexing operators we use `get` and `set` instead: | 
| /// | 
| /// x.get('hi') | 
| /// x.set('hi', 123) | 
| @@ -1606,16 +1638,25 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { | 
| /// This follows the same pattern as EcmaScript 6 Map: | 
| /// <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map> | 
| /// | 
| - /// For all others we use the operator name: | 
| - /// | 
| - /// x['+'](y) | 
| + /// Unary minus looks like: `x['unary-']()`. Note that [unary] must be passed | 
| + /// for this transformation to happen, otherwise binary minus is assumed. | 
| /// | 
| /// Equality is a bit special, it is generated via the Dart `equals` runtime | 
| /// helper, that checks for null. The user defined method is called '=='. | 
| - String _jsMethodName(String name) { | 
| - if (name == '[]') return 'get'; | 
| - if (name == '[]=') return 'set'; | 
| - return name; | 
| + /// | 
| + JS.Expression _jsMemberName(String name, {bool unary: false}) { | 
| + if (name.startsWith('_')) { | 
| + _privateNames.add(name); | 
| + return new JS.VariableUse(name); | 
| + } | 
| + if (name == '[]') { | 
| + name = 'get'; | 
| + } else if (name == '[]=') { | 
| + name = 'set'; | 
| + } else if (unary && name == '-') { | 
| + name = 'unary-'; | 
| + } | 
| + return new JS.PropertyName(name); | 
| } | 
| bool _externalOrNative(node) => |