lgtm
Some nitpicks. But if the compilers are happy, I'm too. :)
https://codereview.chromium.org/1162363005/diff/20001/src/accessors.cc
File src/accessors.cc (right):
https://codereview.chromium.org/1162363005/diff/20001/src/accessors.cc#newcode1416
src/accessors.cc:1416: }
I don't think I understand the logic here:
Previously, if info.Data() didn't contain a valid slot number, if would
run into the DCHECK in FixedArray::get. I presume that in release
builds, it would just wildly access memory. Apparently, this code
assumed that surely the slot number would be valid.
This now checks for one specific error condition, namely no slot number
being given at all, but all other error conditions are still unchecked.
Why? If we assume it's the caller responsibility to provide valid data,
then why handle this particular situation?
https://codereview.chromium.org/1162363005/diff/20001/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/1162363005/diff/20001/src/api.cc#newcode325
src/api.cc:325: .ToLocal(&resource_name)) {
How could this NewFromUtf8 call fail?
https://codereview.chromium.org/1162363005/diff/20001/src/extensions/gc-extension.cc
File src/extensions/gc-extension.cc (right):
https://codereview.chromium.org/1162363005/diff/20001/src/extensions/gc-extension.cc#newcode23
src/extensions/gc-extension.cc:23:
->BooleanValue(args.GetIsolate()->GetCurrentContext())
style nitpick: Why newline + 8 indent?
https://codereview.chromium.org/1162363005/
--
--
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/d/optout.
