Library prefix should be an identifier

Library prefix should be an identifier
(we don't yet check that it is not a reserved keyword, that can be done only
after we are able to store the keyword table as a singleton in the vm isolate)
Committed: https://code.google.com/p/dart/source/detail?r=1720

[hausner, 2011-11-21 19:53:58]

See comment below. LGTM after comments addressed.

http://codereview.chromium.org/8500006/diff/4002/vm/scanner.cc
File vm/scanner.cc (right):

http://codereview.chromium.org/8500006/diff/4002/vm/scanner.cc#newcode121
vm/scanner.cc:121: if (!str.IsOneByteString()) {
Is there a guarantee that strings will always be of the smallest type, ie. that
we could not encounter the word "if" in a two-byte string?

http://codereview.chromium.org/8500006/diff/4002/vm/scanner.cc#newcode125
vm/scanner.cc:125: if (!IsIdentChar(str.CharAt(i))) {
The first character in the string must be checked with IsIdentStartChar(),
otherwise "55" would pass as a valid identifier.

Also, you need to check that the string has at least one character.

[siva, 2011-11-21 21:48:04]

PTAL

http://codereview.chromium.org/8500006/diff/4002/vm/scanner.cc
File vm/scanner.cc (right):

http://codereview.chromium.org/8500006/diff/4002/vm/scanner.cc#newcode121
vm/scanner.cc:121: if (!str.IsOneByteString()) {
No they are not guaranteed to be always the smallest type and hence the check to
filter those out as invalid.

On 2011/11/21 19:53:58, hausner wrote:
> Is there a guarantee that strings will always be of the smallest type, ie.
that
> we could not encounter the word "if" in a two-byte string?

http://codereview.chromium.org/8500006/diff/4002/vm/scanner.cc#newcode125
vm/scanner.cc:125: if (!IsIdentChar(str.CharAt(i))) {
Good catch.

On 2011/11/21 19:53:58, hausner wrote:
> The first character in the string must be checked with IsIdentStartChar(),
> otherwise "55" would pass as a valid identifier.
> 
> Also, you need to check that the string has at least one character.

[hausner, 2011-11-21 21:58:10]

LGTM w/c

http://codereview.chromium.org/8500006/diff/15005/vm/parser.cc
File vm/parser.cc (right):

http://codereview.chromium.org/8500006/diff/15005/vm/parser.cc#newcode3166
vm/parser.cc:3166: prefix = String::NewSymbol(prefix);
The scanner already makes a symbol out of a string literal. In other words,

#import("/foo.dart", prefix: "foo");

"foo" is already a symbol, no need to call NewSymbol again.

[hausner, 2011-11-21 21:58:11]

LGTM w/c

[siva, 2011-11-21 22:14:28]

http://codereview.chromium.org/8500006/diff/15005/vm/parser.cc
File vm/parser.cc (right):

http://codereview.chromium.org/8500006/diff/15005/vm/parser.cc#newcode3166
vm/parser.cc:3166: prefix = String::NewSymbol(prefix);
On 2011/11/21 21:58:10, hausner wrote:
> The scanner already makes a symbol out of a string literal. In other words,
> 
> #import("/foo.dart", prefix: "foo");
> 
> "foo" is already a symbol, no need to call NewSymbol again.

Done.