Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(393)

Issue 9615005: Update unicode tables to version 6.1.0. (Closed)

Created:
8 years, 9 months ago by Michael Starzinger
Modified:
8 years, 9 months ago
Reviewers:
Erik Corry
CC:
v8-dev
Visibility:
Public.

Description

Update unicode tables to version 6.1.0. R=erik.corry@gmail.com BUG=v8:1965 Committed: https://code.google.com/p/v8/source/detail?r=10933

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed comments by Erik Corry. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+769 lines, -533 lines) Patch
M src/unicode.cc View 32 chunks +769 lines, -533 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Michael Starzinger
8 years, 9 months ago (2012-03-06 09:10:25 UTC) #1
Erik Corry
https://chromiumcodereview.appspot.com/9615005/diff/1/src/unicode.cc File src/unicode.cc (right): https://chromiumcodereview.appspot.com/9615005/diff/1/src/unicode.cc#newcode750 src/unicode.cc:750: case 5: return LookupPredicate(kNumberTable5, I understand that the data ...
8 years, 9 months ago (2012-03-06 09:31:56 UTC) #2
Erik Corry
LGTM
8 years, 9 months ago (2012-03-06 09:38:11 UTC) #3
Michael Starzinger
8 years, 9 months ago (2012-03-06 09:39:54 UTC) #4
Addressed comments. Added new patch set.

https://chromiumcodereview.appspot.com/9615005/diff/1/src/unicode.cc
File src/unicode.cc (right):

https://chromiumcodereview.appspot.com/9615005/diff/1/src/unicode.cc#newcode750
src/unicode.cc:750: case 5: return LookupPredicate(kNumberTable5,
On 2012/03/06 09:31:57, Erik Corry wrote:
> I understand that the data has changed, but why have more tables been added.

As discussed offline: The large unicode table is split into chunks, the chunk
index is defined by "int chunk_index = c >> 13". There just was nothing in the
chunk with index 5 before, now there is.

https://chromiumcodereview.appspot.com/9615005/diff/1/src/unicode.h
File src/unicode.h (right):

https://chromiumcodereview.appspot.com/9615005/diff/1/src/unicode.h#newcode1
src/unicode.h:1: // Copyright 2012 the V8 project authors. All rights reserved.
On 2012/03/06 09:31:57, Erik Corry wrote:
> This seems wrong

Done.

Powered by Google App Engine
This is Rietveld 408576698