http://codereview.chromium.org/3226014/diff/1/4 File src/frames.cc (right):
http://codereview.chromium.org/3226014/diff/1/4#newcode158 src/frames.cc:158: if (type == StackFrame::JAVA_SCRIPT) { On 2010/08/30 07:09:41, Kasper Lund wrote:
Remove this code or guard it by #ifdef DEBUG and assert that we can
always find
the code for JavaScript frames. As it is written now, it doesn't make
much
sense.
Done. http://codereview.chromium.org/3226014/diff/1/4#newcode824 src/frames.cc:824: Address pc) { On 2010/08/30 07:09:41, Kasper Lund wrote:
Weird indentation. Use only 4 spaces.
Merged to single line http://codereview.chromium.org/3226014/diff/1/4#newcode831 src/frames.cc:831: index++; On 2010/08/30 07:09:41, Kasper Lund wrote:
I'm not sure I understand why you increment index here. Index isn't
used from
this point forward.
Done. http://codereview.chromium.org/3226014/diff/1/5 File src/frames.h (right): http://codereview.chromium.org/3226014/diff/1/5#newcode51 src/frames.h:51: static const int kPcToCodeCacheSize = 256; On 2010/08/30 07:09:41, Kasper Lund wrote:
Does the size have to be public?
No, moved to private section http://codereview.chromium.org/3226014/diff/1/5#newcode64 src/frames.h:64: // static void FlushPcToCodeCache(); On 2010/08/30 07:09:41, Kasper Lund wrote:
Remove comment or change it to something that makes sense.
Done. http://codereview.chromium.org/3226014/diff/1/5#newcode70 src/frames.h:70: On 2010/08/30 07:09:41, Kasper Lund wrote:
Remove one of the empty lines here.
Done. http://codereview.chromium.org/3226014/diff/1/5#newcode73 src/frames.h:73: On 2010/08/30 07:09:41, Kasper Lund wrote:
Remove two empty lines here.
Done. http://codereview.chromium.org/3226014/diff/1/5#newcode191 src/frames.h:191: On 2010/08/30 07:09:41, Kasper Lund wrote:
Remove one of the two newlines.
Done. http://codereview.chromium.org/3226014/diff/1/5#newcode195 src/frames.h:195: return (PcToCodeCache::GetCacheEntry(pc))->code; On 2010/08/30 07:09:41, Kasper Lund wrote:
Remove one level of ()s.
Done. http://codereview.chromium.org/3226014/diff/1/13 File src/spaces.cc (right): http://codereview.chromium.org/3226014/diff/1/13#newcode2732 src/spaces.cc:2732: // TODO(kasperl): Change this implementation to only find executable On 2010/08/30 07:09:41, Kasper Lund wrote:
File an issue and change the TODO to include the issue number.
Done. http://codereview.chromium.org/3226014/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
