Issue 11419259: Fix bug in Utf8::CodePointCount which was causing some strings with latin1

Issue 11419259: Fix bug in Utf8::CodePointCount which was causing some strings with latin1 (Closed)

Created:
8 years ago by siva

Modified:
8 years ago

Reviewers:
cshapiro

CC:
reviews_dartlang.org, vm-dev_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/runtime/

Visibility:
Public.

More Reviews

Description

Fix bug in Utf8::CodePointCount which was causing some strings with latin1 characters to be stored as TwoByteStrings. Committed: https://code.google.com/p/dart/source/detail?r=15604

Patch Set 3 : #

Created: 8 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+42 lines, -26 lines)			Patch
M	vm/dart_api_impl_test.cc	View		1 chunk	+11 lines, -0 lines	0 comments	Download
M	vm/dart_api_message.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	vm/object.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	vm/object_test.cc	View		2 chunks	+5 lines, -5 lines	0 comments	Download
M	vm/symbols.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	vm/unicode.h	View	1 2	3 chunks	+10 lines, -7 lines	0 comments	Download
M	vm/unicode.cc	View	1 2	1 chunk	+13 lines, -11 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

cshapiro

lgtm with comments https://codereview.chromium.org/11419259/diff/7001/vm/unicode.h File vm/unicode.h (right): https://codereview.chromium.org/11419259/diff/7001/vm/unicode.h#newcode46 vm/unicode.h:46: // Returns a count of the ...

8 years ago (2012-11-30 21:32:26 UTC) #2

siva

8 years ago (2012-11-30 21:47:19 UTC) #3

https://codereview.chromium.org/11419259/diff/7001/vm/unicode.h
File vm/unicode.h (right):

https://codereview.chromium.org/11419259/diff/7001/vm/unicode.h#newcode46
vm/unicode.h:46: // Returns a count of the number of UTF-16 code units needed to
represent the
Changed the comment to:
Returns the most restricted coding form in which the sequence of utf8 characters
in 'utf8_array' can be represented in and the number of code units needed in
that form.

On 2012/11/30 21:32:26, cshapiro wrote:
> This is not strictly true, right?
> 
> This returns the minimal number of code units as well as the most restricted
> coding form.

https://codereview.chromium.org/11419259/diff/7001/vm/unicode.h#newcode87
vm/unicode.h:87: static bool IsTrailByte(uint8_t utf8_byte) {
On 2012/11/30 21:32:26, cshapiro wrote:
> the utf-8 spec removed all mention of "byte" and replaced it with code unit.
> 
> it should be okay to keep the old name since this is in the utf-8 class now.

Done.

Expand Messages | Collapse Messages