Arguments object has @@iterator

src/accessors.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #include "src/accessors.h"
 #include "src/api.h"
 #include "src/compiler.h"
 #include "src/contexts.h"
@@ skipping 138 matching lines @@
     // properties.
     if (!object->map()->is_extensible()) return true;
     JSObject::SetOwnPropertyIgnoreAttributes(object, Utils::OpenHandle(*name),
                                              value, NONE).Check();
   }
   return true;
 }
 
 
 //
+// Accessors::ArgumentsIterator
+//
+
+
+void Accessors::ArgumentsIteratorGetter(
+    v8::Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
+  DisallowHeapAllocation no_allocation;
+  HandleScope scope(isolate);
+  Object* result = isolate->native_context()->array_values_iterator();
+  info.GetReturnValue().Set(Utils::ToLocal(Handle<Object>(result, isolate)));
+}
+
+
+void Accessors::ArgumentsIteratorSetter(
+    v8::Local<v8::Name> name, v8::Local<v8::Value> val,
+    const v8::PropertyCallbackInfo<void>& info) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
+  HandleScope scope(isolate);
+  Handle<JSObject> object = Utils::OpenHandle(*info.This());
+  Handle<Object> value = Utils::OpenHandle(*val);
+  LookupIterator it(object, Utils::OpenHandle(*name));
+  // Advance the iterator to find out more about the property.
+  bool has_property = it.HasProperty();
+  DCHECK(has_property);
+  MaybeHandle<Object> maybe = Object::SetDataProperty(&it, value);

[Toon Verwaest, 2014/08/18 13:47:25]

This doesn't seem right to me, if only because the setter could have been called
through a derived dictionary-mode object for which the iterator will stop on the
dictionary mode object; and HasProperty will be false.

What's the expected behavior exactly? Can't you just use SetProperty(&it,
value), or SetOwnPropertyIgnoreAttributes?

[arv (Not doing code reviews), 2014/08/18 14:16:56]

The intent is to replace the existing interceptor with a plain old data
property.

[wingo, 2014/08/20 15:46:57]

As Erik says the intent is that arguments[@@iterator] is just a data property. 
It has to be implemented using API accessor callbacks because we know what the
value should be for it, and we don't want to bloat the accessor object with a
different word.  But if someone does arguments[@@iterator] = 10, it should
replace the API accessors with a normal data property.

[Toon Verwaest, 2014/08/21 11:07:20]

To be clear what I meant, what about:

o = {__proto__:arguments};
// Make o dict-mode.
%OptimizeObjectForAddingMultipleProperties(o, 0);
o[@@iterator] = 10;

The setter will be called with a slow-mode o, for which (without calling
Next()), IsFound() is true while HasProperty is false.

This sucks about ExecutableAccessorInfo: it looks like a data property but
behaves like an accessor. That's why SetPropertyOnInstanceIfInherited is used as
a hack; which by itself isn't fully correct with regard to non-extensible
objects in strict-mode.

Or even worse, what about:
String.prototype.__proto__ = arguments;
"bla"[@@iterator] = 10;

+  maybe.Check();
+}
+
+
+Handle<AccessorInfo> Accessors::ArgumentsIteratorInfo(
+    Isolate* isolate, PropertyAttributes attributes) {
+  return MakeAccessor(isolate, isolate->factory()->length_string(),

[Toon Verwaest, 2014/08/21 11:07:20]

Ok, in that case this is part of what confused me. This shouldn't be
length-string, but rather @@iterator, right?

+                      &ArgumentsIteratorGetter, &ArgumentsIteratorSetter,
+                      attributes);
+}
+
+
+//
 // Accessors::ArrayLength
 //
 
 
 // The helper function will 'flatten' Number objects.
 Handle<Object> Accessors::FlattenNumber(Isolate* isolate,
                                         Handle<Object> value) {
   if (value->IsNumber() || !value->IsJSValue()) return value;
   Handle<JSValue> wrapper = Handle<JSValue>::cast(value);
   DCHECK(wrapper->GetIsolate()->native_context()->number_function()->
@@ skipping 1185 matching lines @@
   info->set_data(Smi::FromInt(index));
   Handle<Object> getter = v8::FromCData(isolate, &ModuleGetExport);
   Handle<Object> setter = v8::FromCData(isolate, &ModuleSetExport);
   info->set_getter(*getter);
   if (!(attributes & ReadOnly)) info->set_setter(*setter);
   return info;
 }
 
 
 } }  // namespace v8::internal