Addressed review comments. Now working on the other architectures, but
please
take another look in the meantime.
Thanks,
-Ivan
http://codereview.chromium.org/669240/diff/1/6
File src/api.cc (right):
http://codereview.chromium.org/669240/diff/1/6#newcode1216
src/api.cc:1216: i::Handle<i::Object> obj = Utils::OpenHandle(this);
On 2010/03/08 08:21:52, Kasper Lund wrote:
Maybe refactor this code so it can be shared with the same code in
Script::SetData?
Done.
http://codereview.chromium.org/669240/diff/1/17
File src/bootstrapper.cc (right):
http://codereview.chromium.org/669240/diff/1/17#newcode819
src/bootstrapper.cc:819: Handle<SharedFunctionInfo> shared_function;
On 2010/03/08 08:21:52, Kasper Lund wrote:
In general, I would probably try to call local variables of type
SharedFunctionInfo shared_info / shared instead of shared_function
because it's
weird to me that something called shared_function isn't a function.
Decided to call it function_info everywhere, shared_info is not
conveying the fact that it is the information associated with a
function.
http://codereview.chromium.org/669240/diff/1/25
File src/codegen.cc (right):
http://codereview.chromium.org/669240/diff/1/25#newcode340
src/codegen.cc:340: Compiler::BuildFunction(node->fun(), script(),
this);
On 2010/03/08 08:21:52, Kasper Lund wrote:
It's somewhat weird that BuildFunction doesn't return a function.
BuildFunctionInfo?
Done.
http://codereview.chromium.org/669240/diff/1/20
File src/compiler.cc (right):
http://codereview.chromium.org/669240/diff/1/20#newcode120
src/compiler.cc:120: static Handle<SharedFunctionInfo> MakeFunction(
On 2010/03/08 08:21:52, Kasper Lund wrote:
It's weird that MakeFunction doesn't return a function. Maybe change
to
MakeFunctionInfo?
Done.
http://codereview.chromium.org/669240/diff/1/20#newcode422
src/compiler.cc:422: Handle<Script> script,
On 2010/03/08 08:21:52, Kasper Lund wrote:
Funky indentation.
Done.
http://codereview.chromium.org/669240/diff/1/27
File src/compiler.h (right):
http://codereview.chromium.org/669240/diff/1/27#newcode254
src/compiler.h:254: // Compile a function boilerplate object (the
function is possibly
On 2010/03/08 08:21:52, Kasper Lund wrote:
Update comment. Maybe change name from BuildFunction to
BuildFunctionInfo?
Done.
http://codereview.chromium.org/669240/diff/1/28
File src/debug.cc (right):
http://codereview.chromium.org/669240/diff/1/28#newcode1967
src/debug.cc:1967: Handle<SharedFunctionInfo> fun) {
On 2010/03/08 08:21:52, Kasper Lund wrote:
fun -> shared
ditto
http://codereview.chromium.org/669240/diff/1/12
File src/debug.h (right):
http://codereview.chromium.org/669240/diff/1/12#newcode608
src/debug.h:608: Handle<SharedFunctionInfo> fun);
On 2010/03/08 08:21:52, Kasper Lund wrote:
fun -> shared?
This fun ctionality has been changed in the meantime.
http://codereview.chromium.org/669240/diff/1/30
File src/fast-codegen.cc (right):
http://codereview.chromium.org/669240/diff/1/30#newcode566
src/fast-codegen.cc:566: SharedFunctionLiteral* expr) {
On 2010/03/08 08:21:52, Kasper Lund wrote:
Indentation seems slightly off here.
Done.
http://codereview.chromium.org/669240/diff/1/7
File src/heap.cc (right):
http://codereview.chromium.org/669240/diff/1/7#newcode1264
src/heap.cc:1264: // Allocate the empty array
On 2010/03/08 08:21:52, Kasper Lund wrote:
Terminate comment with .
Done.
http://codereview.chromium.org/669240/diff/1/7#newcode1269
src/heap.cc:1269: obj = AllocateEmptyFixedArray();
On 2010/03/08 08:21:52, Kasper Lund wrote:
It would be nice with at least a comment on what this is for.
ditto
http://codereview.chromium.org/669240/diff/1/35
File src/heap.h (right):
http://codereview.chromium.org/669240/diff/1/35#newcode63
src/heap.h:63: V(FixedArray, empty_fixed_array2, EmptyFixedArray2)
\
On 2010/03/08 08:21:52, Kasper Lund wrote:
Line too long ... and what is this?
Surviving debugging code. Gone!
http://codereview.chromium.org/669240/diff/1/14
File src/ia32/codegen-ia32.cc (right):
http://codereview.chromium.org/669240/diff/1/14#newcode4060
src/ia32/codegen-ia32.cc:4060: if (0 && scope()->is_function_scope() &&
On 2010/03/08 08:21:52, Kasper Lund wrote:
0 -> false
Done. We should implement the FastNewClosureStub, but I am focusing on
getting this massive change in first...
http://codereview.chromium.org/669240/diff/1/16
File src/ia32/codegen-ia32.h (right):
http://codereview.chromium.org/669240/diff/1/16#newcode541
src/ia32/codegen-ia32.h:541: // Instantiate the function boilerplate.
On 2010/03/08 08:21:52, Kasper Lund wrote:
Update comment.
Done.
http://codereview.chromium.org/669240/diff/1/13
File src/ia32/fast-codegen-ia32.cc (right):
http://codereview.chromium.org/669240/diff/1/13#newcode199
src/ia32/fast-codegen-ia32.cc:199: SharedFunctionLiteral* expr) {
On 2010/03/08 08:21:52, Kasper Lund wrote:
Indentation.
Done.
http://codereview.chromium.org/669240/diff/1/13#newcode771
src/ia32/fast-codegen-ia32.cc:771: SharedFunctionLiteral* expr) {
On 2010/03/08 08:21:52, Kasper Lund wrote:
Indentation.
Done.
http://codereview.chromium.org/669240/diff/1/5
File src/prettyprinter.cc (right):
http://codereview.chromium.org/669240/diff/1/5#newcode922
src/prettyprinter.cc:922: PrintLiteralIndented("SHARED FUNCTION",
node->shared_function(), true);
On 2010/03/08 08:21:52, Kasper Lund wrote:
SHARED FUNCTION -> SHARED INFO?
Done.
http://codereview.chromium.org/669240/diff/1/3
File test/cctest/test-func-name-inference.cc (right):
http://codereview.chromium.org/669240/diff/1/3#newcode65
test/cctest/test-func-name-inference.cc:65: Handle<Object> obj =
v8::Utils::OpenHandle(*script);
On 2010/03/08 08:21:52, Kasper Lund wrote:
This also looks a bit like the code in Script::SetData.
Yes, except that I cannot access the static helper function inside
api.cc from here.
http://codereview.chromium.org/669240
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply
to this email with the words "REMOVE ME" as the subject.
