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

Issue 100249: When strings can change from an ASCII representation to a... (Closed)

Created:
11 years, 7 months ago by Erik Corry
Modified:
9 years, 7 months ago
Reviewers:
Lasse Reichstein
CC:
v8-dev
Visibility:
Public.

Description

When strings can change from an ASCII representation to a UC16 representation we need to be careful about flat strings. Flat strings can be sliced or cons strings that have a flat string under them, so when we ask a flat cons or a slice whether it is ASCII or not we should ask the underlying string about its representation. This should fix http://code.google.com/p/chromium/issues/detail?id=10971 Committed: http://code.google.com/p/v8/source/detail?r=1830

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+180 lines, -108 lines) Patch
M src/heap.cc View 3 chunks +6 lines, -6 lines 0 comments Download
M src/ia32/regexp-macro-assembler-ia32.cc View 5 chunks +5 lines, -5 lines 0 comments Download
M src/interpreter-irregexp.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/jsregexp.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M src/log.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/objects.h View 2 chunks +3 lines, -2 lines 0 comments Download
M src/objects.cc View 15 chunks +16 lines, -17 lines 0 comments Download
M src/objects-inl.h View 6 chunks +23 lines, -18 lines 1 comment Download
M src/runtime.cc View 10 chunks +15 lines, -15 lines 0 comments Download
M test/cctest/test-api.cc View 5 chunks +108 lines, -41 lines 1 comment Download

Messages

Total messages: 2 (0 generated)
Erik Corry
11 years, 7 months ago (2009-05-01 09:21:45 UTC) #1
Lasse Reichstein
11 years, 7 months ago (2009-05-01 09:47:01 UTC) #2
LGTM

http://codereview.chromium.org/100249/diff/1/9
File src/objects-inl.h (right):

http://codereview.chromium.org/100249/diff/1/9#newcode218
Line 218: } else if ((type & kStringRepresentationMask) == kConsStringTag &&
You can omit the "else" here, since the former condition always returns.

http://codereview.chromium.org/100249/diff/1/2
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/100249/diff/1/2#newcode6242
Line 6242: "/[^a-z]/.test(slice_on_cons);");
Perhaps add a comment stating what is being tested here (that flattens works).

Powered by Google App Engine
This is Rietveld 408576698