Use correct type on evaluated literal symbols.

Use correct type on evaluated literal symbols.

BUG=
R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/91616103aa6149255aa78af0b28d22c381803c1e

[Johnni Winther, 2016-05-26 14:21:55]

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

[Siggi Cherem (dart-lang), 2016-05-26 19:52:15]

lgtm, with questions...

https://codereview.chromium.org/2018523002/diff/1/pkg/compiler/lib/src/js_bac...
File pkg/compiler/lib/src/js_backend/constant_system_javascript.dart (right):

https://codereview.chromium.org/2018523002/diff/1/pkg/compiler/lib/src/js_bac...
pkg/compiler/lib/src/js_backend/constant_system_javascript.dart:354:
InterfaceType type = symbolClass.rawType;
shouldn't symbolClass.rawType and coreTypes.symbolType be the same?

I guess symbolType comes from the unpatched Symbol, and symbolClass would be
Symbol.implementation? Should we anyways have an invariant in our system that
the type returned by a class element and it's implementation is the same
(cls.rawType == cls.implementation.rawType)?

[Johnni Winther, 2016-05-27 08:51:24]

https://codereview.chromium.org/2018523002/diff/1/pkg/compiler/lib/src/js_bac...
File pkg/compiler/lib/src/js_backend/constant_system_javascript.dart (right):

https://codereview.chromium.org/2018523002/diff/1/pkg/compiler/lib/src/js_bac...
pkg/compiler/lib/src/js_backend/constant_system_javascript.dart:354:
InterfaceType type = symbolClass.rawType;
On 2016/05/26 19:52:15, Siggi Cherem (dart-lang) wrote:
> shouldn't symbolClass.rawType and coreTypes.symbolType be the same?
> 
> I guess symbolType comes from the unpatched Symbol, and symbolClass would be
> Symbol.implementation? Should we anyways have an invariant in our system that
> the type returned by a class element and it's implementation is the same
> (cls.rawType == cls.implementation.rawType)?

It's worse. `coreTypes.symbolType` is the one defined in dart:core. The
`backend.helpers.symbolImplementationClass` is defined in dart:_internal. We
have

In dart:core :

class Symbol {
  external const factory Symbol(String name);
}

in core_patch.dart :

import 'dart:_internal' as internal;

@patch
class Symbol {
  @patch
  const factory Symbol(String name) = internal.Symbol;
}

and in dart:_internal :

import 'dart:core' as core;

class Symbol implements core.Symbol {
  final String name;

  const Symbol(String name);
}

[Johnni Winther, 2016-05-27 09:05:08]

Description was changed from

==========
Use correct type on evaluated literal symbols.

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

to

==========
Use correct type on evaluated literal symbols.

BUG=
R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/91616103aa6149255aa78af0b28d22c381803c1e
==========

[Johnni Winther, 2016-05-27 09:05:09]

Committed patchset #1 (id:1) manually as
91616103aa6149255aa78af0b28d22c381803c1e (presubmit successful).

[Siggi Cherem (dart-lang), 2016-05-27 17:27:09]

https://codereview.chromium.org/2018523002/diff/1/pkg/compiler/lib/src/js_bac...
File pkg/compiler/lib/src/js_backend/constant_system_javascript.dart (right):

https://codereview.chromium.org/2018523002/diff/1/pkg/compiler/lib/src/js_bac...
pkg/compiler/lib/src/js_backend/constant_system_javascript.dart:354:
InterfaceType type = symbolClass.rawType;
On 2016/05/27 08:51:24, Johnni Winther wrote:
> On 2016/05/26 19:52:15, Siggi Cherem (dart-lang) wrote:
> > shouldn't symbolClass.rawType and coreTypes.symbolType be the same?
> > 
> > I guess symbolType comes from the unpatched Symbol, and symbolClass would be
> > Symbol.implementation? Should we anyways have an invariant in our system
that
> > the type returned by a class element and it's implementation is the same
> > (cls.rawType == cls.implementation.rawType)?
> 
> It's worse. `coreTypes.symbolType` is the one defined in dart:core. The
> `backend.helpers.symbolImplementationClass` is defined in dart:_internal. We
> have
> 
> In dart:core :
> 
> class Symbol {
>   external const factory Symbol(String name);
> }
> 
> in core_patch.dart :
> 
> import 'dart:_internal' as internal;
> 
> @patch
> class Symbol {
>   @patch
>   const factory Symbol(String name) = internal.Symbol;
> }
> 
> and in dart:_internal :
> 
> import 'dart:core' as core;
> 
> class Symbol implements core.Symbol {
>   final String name;
> 
>   const Symbol(String name);
> }

yikes - should we change the patch file to directly contain the implementation
instead?

[Johnni Winther, 2016-05-30 07:58:24]

https://codereview.chromium.org/2018523002/diff/1/pkg/compiler/lib/src/js_bac...
File pkg/compiler/lib/src/js_backend/constant_system_javascript.dart (right):

https://codereview.chromium.org/2018523002/diff/1/pkg/compiler/lib/src/js_bac...
pkg/compiler/lib/src/js_backend/constant_system_javascript.dart:354:
InterfaceType type = symbolClass.rawType;
On 2016/05/27 17:27:08, Siggi Cherem (dart-lang) wrote:
> On 2016/05/27 08:51:24, Johnni Winther wrote:
> > On 2016/05/26 19:52:15, Siggi Cherem (dart-lang) wrote:
> > > shouldn't symbolClass.rawType and coreTypes.symbolType be the same?
> > > 
> > > I guess symbolType comes from the unpatched Symbol, and symbolClass would
be
> > > Symbol.implementation? Should we anyways have an invariant in our system
> that
> > > the type returned by a class element and it's implementation is the same
> > > (cls.rawType == cls.implementation.rawType)?
> > 
> > It's worse. `coreTypes.symbolType` is the one defined in dart:core. The
> > `backend.helpers.symbolImplementationClass` is defined in dart:_internal. We
> > have
> > 
> > In dart:core :
> > 
> > class Symbol {
> >   external const factory Symbol(String name);
> > }
> > 
> > in core_patch.dart :
> > 
> > import 'dart:_internal' as internal;
> > 
> > @patch
> > class Symbol {
> >   @patch
> >   const factory Symbol(String name) = internal.Symbol;
> > }
> > 
> > and in dart:_internal :
> > 
> > import 'dart:core' as core;
> > 
> > class Symbol implements core.Symbol {
> >   final String name;
> > 
> >   const Symbol(String name);
> > }
> 
> yikes - should we change the patch file to directly contain the implementation
> instead?

I think we want to share the implementation between dart2js and VM (and
Dartino?). Don't know why; I guess Peter knows.