Missed the inline comments.

http://codereview.chromium.org/28112/diff/1/3
File src/log.h (right):

http://codereview.chromium.org/28112/diff/1/3#newcode276
Line 276: class StackTracer {
Please add BASE_EMBEDDED.

http://codereview.chromium.org/28112/diff/1/5
File test/cctest/test-log-ia32.cc (right):

http://codereview.chromium.org/28112/diff/1/5#newcode141
Line 141: static Handle<JSFunction> Compile(const char* source) {
For compiling JavaScript I think you should stick to using the public
API when possible instead of using internal functions, e.g.
CompileFunction in test-debug.cc. You can always later convert API
objects to their internal counterparts when required.

http://codereview.chromium.org/28112/diff/1/5#newcode150
Line 150: static void CompileRun(const char* source) {
Again use the public API for this.

http://codereview.chromium.org/28112/diff/1/5#newcode208
Line 208: original, patch, sizeof(patch)));
This code patching seems risky and changes to the compiler might break
it. How about adding a runtime function to return the FP of the caller?

http://codereview.chromium.org/28112/diff/1/5#newcode210
Line 210: SetGlobalProperty("JSFuncDoTrace", *call_trace);
Look at CompileFunction in test-debug.cc to compile 'function
JSFuncDOTrace() { ... }'.

http://codereview.chromium.org/28112/diff/1/5#newcode212
Line 212: SetGlobalProperty("sample", Smi::FromInt(PtrToSmi(&sample)));
I don't see the need to store these values in the JS global object.
Can't they just be global variables in C++? They are only used from C++
code in this file.

http://codereview.chromium.org/28112

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to