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 {
On 2009/02/25 12:36:13, Søren Gjesse wrote:
> Please add BASE_EMBEDDED.

Done.

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) {
On 2009/02/25 12:36:13, Søren Gjesse wrote:
> 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.

Thanks to your suggestion I removed dozen lines of code. I simply didn't
knew about OpenHandle / ToApi utilities before.

http://codereview.chromium.org/28112/diff/1/5#newcode150
Line 150: static void CompileRun(const char* source) {
On 2009/02/25 12:36:13, Søren Gjesse wrote:
> Again use the public API for this.

Done.

http://codereview.chromium.org/28112/diff/1/5#newcode208
Line 208: original, patch, sizeof(patch)));
On 2009/02/25 12:36:13, Søren Gjesse wrote:
> 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?

OK, maybe we will get rid of this in the next iteration. E.g. I didn't
know about inline runtime functions.

http://codereview.chromium.org/28112/diff/1/5#newcode210
Line 210: SetGlobalProperty("JSFuncDoTrace", *call_trace);
On 2009/02/25 12:36:13, Søren Gjesse wrote:
> Look at CompileFunction in test-debug.cc to compile 'function
JSFuncDOTrace() {
> ... }'.


I tried this already, but because of lazy compilation, I haven't got a
real compiled code but instead a stub that calls LazyCompile. And
running the function prior to patching is not an option here.

http://codereview.chromium.org/28112/diff/1/5#newcode212
Line 212: SetGlobalProperty("sample", Smi::FromInt(PtrToSmi(&sample)));
On 2009/02/25 12:36:13, Søren Gjesse wrote:
> 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.

Good idea! Done.

http://codereview.chromium.org/28112

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

Reply via email to