dart2js: remove references to compiler in ResolvedUriTranslator

dart2js: remove references to compiler in ResolvedUriTranslator

BUG=
R=johnniwinther@google.com, sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/8f3e4cff016d96de060f1a7d9d85f559a20fc309

[Harry Terkelsen, 2016-04-12 21:22:46]

het@google.com changed reviewers:
+ sigmund@google.com

[Harry Terkelsen, 2016-04-12 21:23:36]

het@google.com changed reviewers:
+ johnniwinther@google.com

[Johnni Winther, 2016-04-13 08:21:43]

lgtm

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/pa...
File pkg/compiler/lib/src/parser/diet_parser_task.dart (right):

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/pa...
pkg/compiler/lib/src/parser/diet_parser_task.dart:42: // The error should have
already been reported by the parser.
Add a TODO (for me) to add a 'hasErrorBeenReported' or similar to
[DiagnosticReporter] to reenable the assertion.

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolved_uri_translator.dart (right):

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolved_uri_translator.dart:29: /// [:null:] if [uri] is
loaded as the main library. The
[:null:] -> `null`

The [: ... :] notation is archaic.

[Siggi Cherem (dart-lang), 2016-04-13 15:53:36]

lgtm!

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/co...
File pkg/compiler/lib/src/compiler.dart (right):

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:2020: // translator isn't set until the
compiler actually starts running.
Some ideas to make it more self explanatory:

- rename it to indicate that it is not initialized upfront (e.g.
`LazyResolvedUriTranslator` or `UninitializedResolvedUriTranslator`)

- instead of depending on compiler.resolvedUriTranslator, make it contain the
actual translator and add a setup function that will initialize it:

   /// Once initialized, the underlying implementation of the translator.
   ResolvedUriTranslator _translator;

   void setup(x, y) {
     _translator = new ResolvedUriTranslator(x, y);
   }


- create it eagerly in apiimpl at construction time, and call setup from the
apiimpl setup function.

At that point the code will not depend on compiler, so we should be able to move
it to `resolved_uri_translator.dart` too.


An alternative: make the internal fields in
resolved_uri_translator.dart#_ResolvedUriTranslator non final and expose the
setup function there.

[Harry Terkelsen, 2016-04-13 20:23:40]

Thanks, PTAL

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/co...
File pkg/compiler/lib/src/compiler.dart (right):

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:2020: // translator isn't set until the
compiler actually starts running.
On 2016/04/13 15:53:35, Siggi Cherem (dart-lang) wrote:
> Some ideas to make it more self explanatory:
> 
> - rename it to indicate that it is not initialized upfront (e.g.
> `LazyResolvedUriTranslator` or `UninitializedResolvedUriTranslator`)
> 
> - instead of depending on compiler.resolvedUriTranslator, make it contain the
> actual translator and add a setup function that will initialize it:
> 
>    /// Once initialized, the underlying implementation of the translator.
>    ResolvedUriTranslator _translator;
> 
>    void setup(x, y) {
>      _translator = new ResolvedUriTranslator(x, y);
>    }
> 
> 
> - create it eagerly in apiimpl at construction time, and call setup from the
> apiimpl setup function.
> 
> At that point the code will not depend on compiler, so we should be able to
move
> it to `resolved_uri_translator.dart` too.
> 
> 
> An alternative: make the internal fields in
> resolved_uri_translator.dart#_ResolvedUriTranslator non final and expose the
> setup function there.

Done.

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/pa...
File pkg/compiler/lib/src/parser/diet_parser_task.dart (right):

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/pa...
pkg/compiler/lib/src/parser/diet_parser_task.dart:42: // The error should have
already been reported by the parser.
On 2016/04/13 08:21:43, Johnni Winther wrote:
> Add a TODO (for me) to add a 'hasErrorBeenReported' or similar to
> [DiagnosticReporter] to reenable the assertion.

Done.

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolved_uri_translator.dart (right):

https://codereview.chromium.org/1884793002/diff/20001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolved_uri_translator.dart:29: /// [:null:] if [uri] is
loaded as the main library. The
On 2016/04/13 08:21:43, Johnni Winther wrote:
> [:null:] -> `null`
> 
> The [: ... :] notation is archaic.

Done.

[Harry Terkelsen, 2016-04-13 20:23:43]

Thanks, PTAL

[Harry Terkelsen, 2016-04-13 20:50:05]

Description was changed from

==========
dart2js: remove references to compiler in ResolvedUriTranslator

BUG=
==========

to

==========
dart2js: remove references to compiler in ResolvedUriTranslator

BUG=
R=johnniwinther@google.com, sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/8f3e4cff016d96de060f1a7d9d85f559a20fc309
==========

[Harry Terkelsen, 2016-04-13 20:50:07]

Committed patchset #6 (id:100001) manually as
8f3e4cff016d96de060f1a7d9d85f559a20fc309 (presubmit successful).

[Siggi Cherem (dart-lang), 2016-04-13 20:54:21]

lgtm

https://codereview.chromium.org/1884793002/diff/100001/pkg/compiler/lib/src/c...
File pkg/compiler/lib/src/compiler.dart (right):

https://codereview.chromium.org/1884793002/diff/100001/pkg/compiler/lib/src/c...
pkg/compiler/lib/src/compiler.dart:338: this.resolvedUriTranslator,
nit: eventually we might want to remove `this.` here and with
`this.serialization` below.