Support symbol literals.

Support symbol literals.

BUG=
R=ahe@google.com

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

[ahe, 2013-08-06 12:20:55]

The changes are good, but I'm concerned about lack of negative tests and how you
deal with private names.

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/scanner/listener.dart (right):

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/listener.dart:1344: if
(identifierCount > 0) {
How can identifier count be zero? Is that an error?

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/scanner/parser.dart (right):

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/parser.dart:1717: token =
parseIdentifier(token);
Can you use parseQualified?

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/parser.dart:1727: 
Extra line.

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/tree/nodes.dart (right):

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:951: class
SymbolLiteral extends Expression {
This should be LiteralSymbol and it probably should extend Literal<Symbol>.

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:954: final NodeList
identifiers;
Feels to me like the natural way to represent this is with just a NodeList where
the last element might be an operator.

[Johnni Winther, 2013-09-03 12:27:49]

PTAL

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/scanner/listener.dart (right):

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/listener.dart:1344: if
(identifierCount > 0) {
On 2013/08/06 12:20:55, ahe wrote:
> How can identifier count be zero? Is that an error?

It meant operatorToken was != null. Both are now in the identifiers list.

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/scanner/parser.dart (right):

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/parser.dart:1717: token =
parseIdentifier(token);
On 2013/08/06 12:20:55, ahe wrote:
> Can you use parseQualified?

No. Then I will get one Send instead of a NodeList.

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/parser.dart:1727: 
On 2013/08/06 12:20:55, ahe wrote:
> Extra line.

Done.

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/tree/nodes.dart (right):

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:951: class
SymbolLiteral extends Expression {
On 2013/08/06 12:20:55, ahe wrote:
> This should be LiteralSymbol and it probably should extend Literal<Symbol>.

Changed to LiteralSymbol. Don't think it should extend Literal<Symbol>. The only
use case is in the SsaBuilder, where it needs to be generated as a constructed
constant anyway.

https://codereview.chromium.org/21511003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:954: final NodeList
identifiers;
On 2013/08/06 12:20:55, ahe wrote:
> Feels to me like the natural way to represent this is with just a NodeList
where
> the last element might be an operator.

Done.

[Johnni Winther, 2013-09-03 12:40:35]

PTAL

[ahe, 2013-09-03 17:53:34]

LGTM!

https://codereview.chromium.org/21511003/diff/8001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/tree/nodes.dart (right):

https://codereview.chromium.org/21511003/diff/8001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:969: String get
nameString => '${identifiers}';
What is this for? Looks like a method that should be named "slowToString".

https://codereview.chromium.org/21511003/diff/8001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:972: 
Extra line.

[Johnni Winther, 2013-09-04 06:34:23]

Committed patchset #3 manually as r27095 (presubmit successful).

[ngeoffray, 2013-09-04 07:14:17]

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/compile_time_constants.dart
(right):

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compile_time_constants.dart:481:
return [constantSystem.createString(
add <Constant> (since you went into the trouble of typing the return type with
it.

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2491:
visitLiteralSymbol(LiteralSymbol node) {
You should report that to the backend!!! :-) You could just use the scheme with
world.registerInstantiatedClass, that also registers it to the backend, and the
backend recognizes literal types.

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4082:
compiler.enqueuer.codegen.registerConstSymbol(node.slowNameString, elements);
Line too long.

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4082:
compiler.enqueuer.codegen.registerConstSymbol(node.slowNameString, elements);
Please do the enqueing at codegen.

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/types/inferrer_visitor.dart
(right):

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/inferrer_visitor.dart:642:
return types.nonNullExact(compiler.symbolClass.rawType);
Please change this to actually use types.symbolType and have the backend tell
you what is the symbol class implementation.

[Johnni Winther, 2013-09-10 09:09:07]

https://codereview.chromium.org/21511003/diff/8001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/tree/nodes.dart (right):

https://codereview.chromium.org/21511003/diff/8001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:969: String get
nameString => '${identifiers}';
On 2013/09/03 17:53:34, ahe wrote:
> What is this for? Looks like a method that should be named "slowToString".

Renamed to slowNameString. It doesn't include # so I wouldn't call it
slowToString.

https://codereview.chromium.org/21511003/diff/8001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:972: 
On 2013/09/03 17:53:34, ahe wrote:
> Extra line.

Done.

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/compile_time_constants.dart
(right):

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compile_time_constants.dart:481:
return [constantSystem.createString(
On 2013/09/04 07:14:17, ngeoffray wrote:
> add <Constant> (since you went into the trouble of typing the return type with
> it.

Done.

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/21511003/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2491:
visitLiteralSymbol(LiteralSymbol node) {
On 2013/09/04 07:14:17, ngeoffray wrote:
> You should report that to the backend!!! :-) You could just use the scheme
with
> world.registerInstantiatedClass, that also registers it to the backend, and
the
> backend recognizes literal types.

Done.