- Represent strings internally in UTF-16 format, this makes it

- Represent strings internally in UTF-16 format, this makes it
  compatible with webkit and will allow for easy externalization of
  strings. One byte strings are retained for pure ASCII strings.
  (The language specification was changed recently to reflect this as
   follows "A string is a sequence of UTF-16 code units").
- Remove four byte string class and all references to it.
- Rename some of the string functions in Dart API to make them
  consistent and better describe the underlying functionality
  Dart_NewString => Dart_NewStringFromCString
  Dart_NewString8 => Dart_NewStringFromUTF8
  Dart_NewString16 => Dart_NewStringFromUTF16
  Dart_NewString32 => Dart_NewStringFromUTF32
  Dart_NewExternalString8 => Dart_NewExternalUTF8String
  Dart_NewExternalString16 => Dart_NewExternalUTF16String
  Dart_NewExternalString32 => Dart_NewExternalUTF32String
  Dart_StringGet8 => Dart_StringToUTF8
  Dart_StringGet16 => Dart_StringToUTF16
  Dart_StringToCString => Dart_StringToCString
  Dart_IsString8 => Removed
  Dart_IsString16 -> Removed
  Dart_StringToBytes -> Removed
  Dart_StringGet32 -> Removed
Committed: https://code.google.com/p/dart/source/detail?r=14357

[siva, 2012-10-27 01:01:21]

Moved the changelist over to dart-all

Peter could you review the dart2js parts.

[ngeoffray, 2012-10-27 09:02:00]

https://chromiumcodereview.appspot.com/11318018/diff/1/lib/compiler/implement...
File lib/compiler/implementation/string_validator.dart (right):

https://chromiumcodereview.appspot.com/11318018/diff/1/lib/compiler/implement...
lib/compiler/implementation/string_validator.dart:102: bool verify_code = false;
verify_code -> verifyCode

[ahe, 2012-10-28 12:07:02]

I don't think representing strings in UTF-16 is good for Dart users. So I would
rather we didn't do this.

Having stated that concern, I can assess the correctness of changing dart2js to
use UTF-16, and this change does not accomplish that.

For example, consider this program:

main() {
  var str = '\u{10412}';
  Expect.listEquals([0xD801, 0xDC12], str.charCodes());
}

As far as I'm concerned, there are two options: reject the program (which would
be acceptable for now), or generate correct code.

The problem is that the compiler generates this:

  $.Expect_listEquals([55297, 56338], $.charCodes('\u10412'), null);

In JavaScript, '\u10412' is equivalent to this DartString '\u{1041}2'.

https://chromiumcodereview.appspot.com/11318018/diff/1/lib/compiler/implement...
File lib/compiler/implementation/js_backend/constant_emitter.dart (right):

https://chromiumcodereview.appspot.com/11318018/diff/1/lib/compiler/implement...
lib/compiler/implementation/js_backend/constant_emitter.dart:113: if (code >=
0xD800 && code <= 0xDFFF) {
I think you should just remove this check. If the invariant now is that
string.iterator returns a sequence UTF-16 code units, then this should just pass
unmodified through to JavaScript.

https://chromiumcodereview.appspot.com/11318018/diff/1/lib/compiler/implement...
File lib/compiler/implementation/string_validator.dart (right):

https://chromiumcodereview.appspot.com/11318018/diff/1/lib/compiler/implement...
lib/compiler/implementation/string_validator.dart:173: // This handles both
unescaped characters and the value of unicode
I don't think this comment is correct any more.

https://chromiumcodereview.appspot.com/11318018/diff/1/lib/compiler/implement...
lib/compiler/implementation/string_validator.dart:175: if (verify_code &&
!isUnicodeScalarValue(code)) {
This doesn't look right to me. If we are switching to strings being UTF-16
encoded strings, then we shouldn't be checking for valid Unicode scalar values,
and we should allow users to write strings like this:

"\uD801\uDC12" to create a string composed of two unicode code points that make
up the single character U+10412.

[cshapiro, 2012-10-29 06:37:17]

http://codereview.chromium.org/11318018/diff/1/lib/compiler/implementation/st...
File lib/compiler/implementation/string_validator.dart (right):

http://codereview.chromium.org/11318018/diff/1/lib/compiler/implementation/st...
lib/compiler/implementation/string_validator.dart:175: if (verify_code &&
!isUnicodeScalarValue(code)) {
The specification for string literals was deliberately left the same when the
specification was changed.  Part of the spirit of the change was to avoid
exposing the details of the encoding to the user wherever possible.  I believe
ES6 prescribes similarly and is where our \u{N+} syntax came from.

http://codereview.chromium.org/11318018/diff/1/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/11318018/diff/1/runtime/include/dart_api.h#new...
runtime/include/dart_api.h:1360: DART_EXPORT Dart_Handle
Dart_NewStringFromUTF8(const uint8_t* utf8_array,
FWIW, the general term for these values (irrespective of the coding) is "code
units".

[ahe, 2012-10-29 08:11:07]

http://codereview.chromium.org/11318018/diff/1/lib/compiler/implementation/st...
File lib/compiler/implementation/string_validator.dart (right):

http://codereview.chromium.org/11318018/diff/1/lib/compiler/implementation/st...
lib/compiler/implementation/string_validator.dart:175: if (verify_code &&
!isUnicodeScalarValue(code)) {
On 2012/10/29 06:37:17, cshapiro wrote:
> The specification for string literals was deliberately left the same when the
> specification was changed.  Part of the spirit of the change was to avoid
> exposing the details of the encoding to the user wherever possible.  I believe
> ES6 prescribes similarly and is where our \u{N+} syntax came from.

If the underlying string representation is UTF-16, and this is exposed to users,
then users need to be able to create a string literal in UTF-16 notation.

By itself, either of these options are simple:

* Dart strings are a sequence of valid Unicode scalar values.

* Dart strings are a sequence of Unicode code points.

* Dart strings are a sequence of UTF-16 code units.

But you're mixing them all up. I think that is highly confusing and I strongly
urge you to reconsider this design.

[ahe, 2012-10-29 10:58:12]

Erik Corry has convinced me that UTF-16 is the best way to represent Dart
strings. However, I still think it should be possible to write string literals
that are a sequence arbitrary UTF-16 code-units.

Unfortunately, that doesn't eliminate my technical concerns about the changes to
dart2js.

[siva, 2012-10-31 01:21:25]

http://codereview.chromium.org/11318018/diff/1/lib/compiler/implementation/js...
File lib/compiler/implementation/js_backend/constant_emitter.dart (right):

http://codereview.chromium.org/11318018/diff/1/lib/compiler/implementation/js...
lib/compiler/implementation/js_backend/constant_emitter.dart:113: if (code >=
0xD800 && code <= 0xDFFF) {
I have decided to drop the change and instead created bug
6418 reflecting the issue with two tests that fail when dart2js tests are run
with a VM that represents strings in UTF-16 format.

On 2012/10/28 12:07:02, ahe wrote:
> I think you should just remove this check. If the invariant now is that
> string.iterator returns a sequence UTF-16 code units, then this should just
pass
> unmodified through to JavaScript.

http://codereview.chromium.org/11318018/diff/1/lib/compiler/implementation/st...
File lib/compiler/implementation/string_validator.dart (right):

http://codereview.chromium.org/11318018/diff/1/lib/compiler/implementation/st...
lib/compiler/implementation/string_validator.dart:175: if (verify_code &&
!isUnicodeScalarValue(code)) {
I have decided to drop these changes made to dart2js in this CL. It is clear
that I do not understand the architecture enough, so I have created bug 6418 to
track the two tests that fail in dart2js when run with a VM that has the change
to represent strings as UTf-16.
I have changed the status files to reflect bug 6418 for the two tests so that
the buildbots remain green.

On 2012/10/29 08:11:07, ahe wrote:
> On 2012/10/29 06:37:17, cshapiro wrote:
> > The specification for string literals was deliberately left the same when
the
> > specification was changed.  Part of the spirit of the change was to avoid
> > exposing the details of the encoding to the user wherever possible.  I
believe
> > ES6 prescribes similarly and is where our \u{N+} syntax came from.
> 
> If the underlying string representation is UTF-16, and this is exposed to
users,
> then users need to be able to create a string literal in UTF-16 notation.
> 
> By itself, either of these options are simple:
> 
> * Dart strings are a sequence of valid Unicode scalar values.
> 
> * Dart strings are a sequence of Unicode code points.
> 
> * Dart strings are a sequence of UTF-16 code units.
> 
> But you're mixing them all up. I think that is highly confusing and I strongly
> urge you to reconsider this design.

http://codereview.chromium.org/11318018/diff/1/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

http://codereview.chromium.org/11318018/diff/1/runtime/include/dart_api.h#new...
runtime/include/dart_api.h:1360: DART_EXPORT Dart_Handle
Dart_NewStringFromUTF8(const uint8_t* utf8_array,
I wanted to make it explicit but we can revisit the names
if it turns out to be confusing.

On 2012/10/29 06:37:17, cshapiro wrote:
> FWIW, the general term for these values (irrespective of the coding) is "code
> units".

[siva, 2012-10-31 01:50:11]

- Dropped the dart2js compiler changes from this CL
- updated status files to reflect the two test failures on dart2js with this
change (see issue 6418)
- only ascii strings are represented by OneByteString type in the VM

[Bill Hesse, 2012-11-01 11:35:02]

There seems to be a bug in Dart_StringToUTF8.  I have changed print in
builtin_natives.cc to use Dart_StringToCString instead.

https://codereview.chromium.org/11318018/diff/20001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/11318018/diff/20001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:1674: intptr_t str_len = str_obj.Length();
This seems totally wrong, unless we restrict the function to only apply to
one-byte strings (ascii).  There is no way to get the UTF8 length from a string
except Uft8::Length, as far as I see, and this is not exposed through the API,
so the API user has no way to pass the correct size array in.

[siva, 2012-11-01 20:04:11]

https://codereview.chromium.org/11318018/diff/20001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/11318018/diff/20001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:1674: intptr_t str_len = str_obj.Length();
Yes that should be
intptr_t str_len = Utf8::Length(str_obj);

I have uploaded a new CL for this.

On 2012/11/01 11:35:02, Bill Hesse wrote:
> This seems totally wrong, unless we restrict the function to only apply to
> one-byte strings (ascii).  There is no way to get the UTF8 length from a
string
> except Uft8::Length, as far as I see, and this is not exposed through the API,
> so the API user has no way to pass the correct size array in.