Addressed comments, landing.
I will deal with MUST_USE_RESULT in another CL. Is it worth adding an
explicit
copy constructor to the Handle class so that warnings work in gcc 4.4.3?
http://codereview.chromium.org/9008012/diff/7001/src/runtime.cc
File src/runtime.cc (right):
http://codereview.chromium.org/9008012/diff/7001/src/runtime.cc#newcode1400
src/runtime.cc:1400: RETURN_IF_EMPTY_HANDLE(isolate,
On 2012/01/05 12:59:39, Kevin Millikin wrote:
No biggy, I prefer my first suggestion (but you may disagree):
RETURN_IF_EMPTY_HANDLE(
isolate,
JSReceiver::SetProperty(object, name, initial_value, mode,
kNonStrictMode));
One fewer line, doesn't run to the right as fast (so scales better),
and breaks
the line at a better logical place---the commas in the outer
RETURN_IF_EMPTY_HANDLE argument list don't bind as "tightly" as the
commas in
the nested SetProperty argument list.
I fixed the style for RETURN_IF_EMPTY_HANDLE (although SetProperty(...)
does not always fit into one line).
I am leaving CHECK_NOT_EMPTY_HANDLE style unchanged, because fixing it
would just add more new lines.
http://codereview.chromium.org/9008012/diff/7001/src/runtime.cc#newcode1443
src/runtime.cc:1443: RETURN_IF_EMPTY_HANDLE(isolate,
On 2012/01/05 12:59:39, Kevin Millikin wrote:
Same story:
RETURN_IF_EMPTY_HANDLE(
isolate,
JSReceiver::SetProperty(object, name, value, mode,
kNonStrictMode));
Here you'll have the kind-of unsightly dangling '(', might as well try
to have
it at a place that binds least tightly in the expression.
Done.
http://codereview.chromium.org/9008012/diff/7001/src/runtime.cc#newcode1554
src/runtime.cc:1554: RETURN_IF_EMPTY_HANDLE(isolate,
On 2012/01/05 12:59:39, Kevin Millikin wrote:
Same story:
RETURN_IF_EMPTY_HANDLE(
isolate,
JSReceiver::SetProperty(global, name, value, attributes,
kNonStrictMode));
Done.
http://codereview.chromium.org/9008012/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
