https://codereview.chromium.org/137403009/diff/820001/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):
https://codereview.chromium.org/137403009/diff/820001/src/arm/code-stubs-arm.cc#newcode3020
src/arm/code-stubs-arm.cc:3020: // Cache the called function in a global
property cell. Cache states
Nit: global property cell -> feedback vector slot.
https://codereview.chromium.org/137403009/diff/820001/src/arm/code-stubs-arm.cc#newcode3080
src/arm/code-stubs-arm.cc:3080: // Create an AllocationSite if we don't
already have it, store it in the cell
Nit: cell -> slot.
https://codereview.chromium.org/137403009/diff/820001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/137403009/diff/820001/src/ast.h#newcode918
src/ast.h:918: class ForInStatement V8_FINAL : public ForEachStatement,
You mentioned offline that you got rid of this multiple inheritance. Why
is it still around?
https://codereview.chromium.org/137403009/diff/820001/src/compiler.cc
File src/compiler.cc (right):
https://codereview.chromium.org/137403009/diff/820001/src/compiler.cc#newcode246
src/compiler.cc:246: void CompilationInfo::ProcessFeedbackSlots() {
Why is this in CompilationInfo? And why do we need three different call
sites for this in the pipeline?
https://codereview.chromium.org/137403009/diff/820001/src/compiler.cc#newcode247
src/compiler.cc:247: // Scope analysis must have been done.
Can we ASSERT that?
https://codereview.chromium.org/137403009/diff/820001/src/compiler.h
File src/compiler.h (right):
https://codereview.chromium.org/137403009/diff/820001/src/compiler.h#newcode641
src/compiler.h:641: bool process_feedback = true);
This bool parameter is really confusing. If we really need it here to
make things work, we should consider fixing our pipeline first, i.e.
split this method into two methods.
https://codereview.chromium.org/137403009/diff/820001/src/feedback_slots.h
File src/feedback_slots.h (right):
https://codereview.chromium.org/137403009/diff/820001/src/feedback_slots.h#newcode38
src/feedback_slots.h:38: class FeedbackSlotInterface {
Hm, do we really need a new header file for this interface?
https://codereview.chromium.org/137403009/diff/820001/src/type-info.cc
File src/type-info.cc (right):
https://codereview.chromium.org/137403009/diff/820001/src/type-info.cc#newcode84
src/type-info.cc:84: if (obj->IsSmi() || obj->IsAllocationSite() ||
Please remove this left over hacks, and use something less obscure,
i.e.:
if (!obj->IsJSFunction() ||
!CanRetainOtherContext(JSFunction::cast(obj), *native_context_)))
Extra points if you generalize CanRetainOtherContext() to take an
Object* and do the IsJSFunction() check itself.
https://codereview.chromium.org/137403009/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
