Feedback addressed. Please take another look

https://codereview.chromium.org/1137703002/diff/100001/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/1137703002/diff/100001/src/code-stubs.cc#newcode19
src/code-stubs.cc:19: using namespace v8::internal::compiler;
On 2015/05/11 04:40:12, Benedikt Meurer wrote:
This violates the style guide. No toplevel using namespace. How about
moving
this into the GenerateCode method below? Or even better: Move the last
three
lines of GenerateCode into a method on the CompilationInfo, i.e.
GenerateCodeStub().

Done.

https://codereview.chromium.org/1137703002/diff/100001/src/code-stubs.cc#newcode476
src/code-stubs.cc:476: }
On 2015/05/11 04:40:12, Benedikt Meurer wrote:
Use

}  // namespace

instead of just

}

to make both clang-format and cpplint agree :-)

Done.

https://codereview.chromium.org/1137703002/diff/100001/src/compiler/access-builder.cc
File src/compiler/access-builder.cc (right):

https://codereview.chromium.org/1137703002/diff/100001/src/compiler/access-builder.cc#newcode161
src/compiler/access-builder.cc:161: FieldAccess
AccessBuilder::ForSharedFunctionInfoTypeFeedbackVector() {
On 2015/05/11 04:40:12, Benedikt Meurer wrote:
Nit: Missing // static before definition.

Done.

https://codereview.chromium.org/1137703002/diff/100001/src/compiler/access-builder.cc#newcode165
src/compiler/access-builder.cc:165:
On 2015/05/11 04:40:12, Benedikt Meurer wrote:
Nit: redundant new line :-)

Done.

https://codereview.chromium.org/1137703002/diff/100001/src/runtime.js
File src/runtime.js (right):

https://codereview.chromium.org/1137703002/diff/100001/src/runtime.js#newcode906
src/runtime.js:906: %_FixedArraySet(%_GetTypeFeedbackVector(f),
((i|0)+1)|0, 1);
On 2015/05/11 04:40:12, Benedikt Meurer wrote:
Please add a comment explaining what this line does.

Done.

https://codereview.chromium.org/1137703002/diff/100001/src/runtime/runtime-array.cc
File src/runtime/runtime-array.cc (right):

https://codereview.chromium.org/1137703002/diff/100001/src/runtime/runtime-array.cc#newcode76
src/runtime/runtime-array.cc:76:
RUNTIME_FUNCTION(Runtime_GetTypeFeedbackVector) {
On 2015/05/11 04:40:12, Benedikt Meurer wrote:
runtime-array.cc doesn't seems to be a good candidate for this
function...

Done.

https://codereview.chromium.org/1137703002/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to