Also, LGTM. On 2009/01/15 12:21:54, William Hesse wrote: > http://codereview.chromium.org/18089/diff/1/5 > File src/jump-target-ia32.cc (right): > > http://codereview.chromium.org/18089/diff/1/5#newcode106 > Line 106: ASSERT(!cgen_->has_cc()); > This looks like a duplicate line. > > http://codereview.chromium.org/18089/diff/1/7 > File src/jump-target.h (right): > > http://codereview.chromium.org/18089/diff/1/7#newcode127 > Line 127: // become the current frame after the bind. > Are we allowing non-mergable expected frames, which might be changed to be > mergable or merged when bound, so that the frame after the binding site won't be > exactly the expected frame? > > http://codereview.chromium.org/18089/diff/1/7#newcode184 > Line 184: // jump target shadows other one, which is hidden as the > Not grammatical, confusing. > > http://codereview.chromium.org/18089/diff/1/7#newcode199 > Line 199: JumpTarget* other_target() const { return other_target_; } > Could this be called "active" target? This is the JumpTarget passed to the > constructor, which always seems to be the active one, or the one that is "seen". > The ShadowTarget represents the hidden target while it is masked by another, > and the target that was used to hide the original, after the hiding is over, > right? > > http://codereview.chromium.org/18089/diff/1/4 > File src/virtual-frame-ia32.cc (left): > > http://codereview.chromium.org/18089/diff/1/4#oldcode321 > Line 321: cgen_->SetFrame(this, &non_frame_registers); > Can you put an assert in, that the frame is its cgen's current frame?
http://codereview.chromium.org/18089 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
