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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 11085003: Convert String to a class. (Closed)

Created:
8 years, 2 months ago by Lasse Reichstein Nielsen
Modified:
8 years, 1 month ago
Reviewers:
ahe, ngeoffray, hausner
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Convert String to a class. Remove the StringImplementation class. Committed: https://code.google.com/p/dart/source/detail?r=14779

Patch Set 1 #

Total comments: 4

Patch Set 2 : Remove left-over List change in comment. #

Total comments: 4

Patch Set 3 : Merged to tip of bleeding_edge. Updated test expecteation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+48 lines, -62 lines) Patch
M runtime/lib/lib_sources.gypi View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M runtime/lib/string_patch.dart View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M runtime/vm/class_finalizer.cc View 1 2 2 chunks +6 lines, -3 lines 0 comments Download
M runtime/vm/dart_api_message.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/flow_graph_compiler_ia32.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/flow_graph_compiler_x64.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/object.h View 1 2 2 chunks +4 lines, -4 lines 0 comments Download
M runtime/vm/object.cc View 1 2 3 chunks +8 lines, -6 lines 0 comments Download
M runtime/vm/object_store.h View 1 2 2 chunks +4 lines, -4 lines 0 comments Download
M runtime/vm/object_store.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/object_test.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/parser.cc View 1 2 3 chunks +3 lines, -3 lines 0 comments Download
M runtime/vm/snapshot.cc View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M runtime/vm/snapshot_ids.h View 1 chunk +1 line, -1 line 0 comments Download
M sdk/lib/_internal/compiler/implementation/lib/core_patch.dart View 1 2 2 chunks +4 lines, -2 lines 0 comments Download
M sdk/lib/core/string.dart View 1 2 2 chunks +2 lines, -23 lines 0 comments Download
M sdk/lib/core/string_buffer.dart View 1 2 1 chunk +1 line, -1 line 0 comments Download
M sdk/lib/core/strings.dart View 1 2 1 chunk +2 lines, -6 lines 0 comments Download
M tests/co19/co19-dart2dart.status View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Lasse Reichstein Nielsen
8 years, 2 months ago (2012-10-08 12:05:52 UTC) #1
ngeoffray
Non-VM change LGTM
8 years, 2 months ago (2012-10-08 13:59:17 UTC) #2
ahe
https://codereview.chromium.org/11085003/diff/3001/lib/compiler/implementation/lib/core_patch.dart File lib/compiler/implementation/lib/core_patch.dart (right): https://codereview.chromium.org/11085003/diff/3001/lib/compiler/implementation/lib/core_patch.dart#newcode89 lib/compiler/implementation/lib/core_patch.dart:89: // TODO(ager): Split out into date_patch.dart and allow #source ...
8 years, 2 months ago (2012-10-08 14:06:11 UTC) #3
Lasse Reichstein Nielsen
https://codereview.chromium.org/11085003/diff/3001/lib/compiler/implementation/lib/core_patch.dart File lib/compiler/implementation/lib/core_patch.dart (right): https://codereview.chromium.org/11085003/diff/3001/lib/compiler/implementation/lib/core_patch.dart#newcode89 lib/compiler/implementation/lib/core_patch.dart:89: // TODO(ager): Split out into date_patch.dart and allow #source ...
8 years, 2 months ago (2012-10-09 11:53:22 UTC) #4
hausner
VM changes LGTM with comments. https://codereview.chromium.org/11085003/diff/1/runtime/vm/class_finalizer.cc File runtime/vm/class_finalizer.cc (right): https://codereview.chromium.org/11085003/diff/1/runtime/vm/class_finalizer.cc#newcode305 runtime/vm/class_finalizer.cc:305: RawClass* super_raw = super_class.raw(); ...
8 years, 2 months ago (2012-10-11 17:04:44 UTC) #5
Lasse Reichstein Nielsen
https://codereview.chromium.org/11085003/diff/1/runtime/vm/class_finalizer.cc File runtime/vm/class_finalizer.cc (right): https://codereview.chromium.org/11085003/diff/1/runtime/vm/class_finalizer.cc#newcode305 runtime/vm/class_finalizer.cc:305: RawClass* super_raw = super_class.raw(); ACK, defactored. https://codereview.chromium.org/11085003/diff/1/runtime/vm/object.h File runtime/vm/object.h ...
8 years, 2 months ago (2012-10-12 08:03:51 UTC) #6
Lasse Reichstein Nielsen
8 years, 2 months ago (2012-10-12 09:17:10 UTC) #7 
Sorry, this change doesn't work any longer.
The utility functions in StringBase are no longer reachable from the core
library since _StringBase became private in coreimpl.

Would it be possible for the VM to move _StringBase into the core string_patch
patch file instead? (Above my VM-hacking ability, I'm afraid).

Powered by Google App Engine
This is Rietveld 408576698