LGTM with comments.
-- Vitaly http://codereview.chromium.org/2770014/diff/1/8 File src/assembler.cc (right): http://codereview.chromium.org/2770014/diff/1/8#newcode598 src/assembler.cc:598: return ExternalReference(Isolate::Current() nit: Leave "->" on this line. http://codereview.chromium.org/2770014/diff/1/8#newcode604 src/assembler.cc:604: return ExternalReference(Isolate::Current() Ditto. http://codereview.chromium.org/2770014/diff/1/9 File src/heap.h (right): http://codereview.chromium.org/2770014/diff/1/9#newcode1559 src/heap.h:1559: results_[i] = 0; Use kAbsent? http://codereview.chromium.org/2770014/diff/1/6 File src/isolate.h (right): http://codereview.chromium.org/2770014/diff/1/6#newcode94 src/isolate.h:94: KeyedLookupCache* keyed_lookup_cache() const { Please remove "const" here and below (and also for "transcendental_cache" above). These are physical consts and don't express the intent. Logically we're exposing mutable parts of the isolate here. http://codereview.chromium.org/2770014/diff/1/3 File src/objects.cc (right): http://codereview.chromium.org/2770014/diff/1/3#newcode1685 src/objects.cc:1685: Isolate * const isolate = Isolate::Current(); On 2010/06/11 20:41:26, Dmitry Titov wrote:
It doesn't look like the rest of the code uses 'const' in similar
places. Also,
'*' is usually glued to the type name.
+1. In this particular case having a local variable holding the lookup cache would be more helpful. http://codereview.chromium.org/2770014/diff/1/4 File src/runtime.cc (right): http://codereview.chromium.org/2770014/diff/1/4#newcode3774 src/runtime.cc:3774: Isolate* const isolate = Isolate::Current(); See above. http://codereview.chromium.org/2770014/diff/1/5 File src/scopeinfo.h (right): http://codereview.chromium.org/2770014/diff/1/5#newcode176 src/scopeinfo.h:176: String* name, nit: Formatting. http://codereview.chromium.org/2770014/diff/1/5#newcode181 src/scopeinfo.h:181: String* name, nit: Formatting. http://codereview.chromium.org/2770014/diff/1/5#newcode194 src/scopeinfo.h:194: values_[i] = 0; Use kNotFound? http://codereview.chromium.org/2770014/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
