Ensure that generated code for object literals will call Runtime_DefineOrRedefineAccessorProperty o…

src/full-codegen.h

Index: src/full-codegen.h
diff --git a/src/full-codegen.h b/src/full-codegen.h
index 25e3dba124dfc2f954b5b4dffa90f2e67784443f..a013e7a4d5abc21c185df35554276a35b22d7098 100644
--- a/src/full-codegen.h
+++ b/src/full-codegen.h
@@ -470,6 +470,8 @@ class FullCodeGenerator: public AstVisitor {
                                  Label* done);
   void EmitVariableLoad(VariableProxy* proxy);
 
+  void EmitAccessor(Expression* expression);
+
   // Expects the arguments and the function already pushed.
   void EmitResolvePossiblyDirectEval(int arg_count);
 
@@ -804,6 +806,81 @@ class FullCodeGenerator: public AstVisitor {
 };
 
 
+// A map from property names to getter/setter pairs with an STL-like iterator
+// interface. In STL speak, an AccessorTable is roughly something like a
+// zone-allocated map<Literal*, pair<Expression*,Expression*> >. Note that only
+// a subset of the usual iterator functions/operators are defined below, but the
+// rest would be easy to add when required.
+class AccessorTable {
+ public:
+  class Accessors: public ZoneObject {

[rossberg, 2012/03/13 13:59:10]

This is just a simple pair, not abstracting anything, so a plain struct seems
more appropriate.

[Sven Panne, 2012/03/14 15:26:56]

I'm perfectly fine with this, and as Uncle Bob says: "The quasi-encapsulation of
beans seems to make some OO purists feel better, but usually provides no other
benefit." :-)

+   public:
+    Accessors() : getter_(NULL), setter_(NULL) { }
+    Expression* getter() const { return getter_; }
+    void set_getter(Expression* expr) { getter_ = expr; }
+    Expression* setter() const { return setter_; }
+    void set_setter(Expression* expr) { setter_ = expr; }
+
+   private:
+    Expression* getter_;
+    Expression* setter_;
+  };
+
+  class Iterator;
+
+  class value_type: private ZoneHashMap::Entry {
+   public:
+    Literal* first() const { return reinterpret_cast<Literal*>(key); }

[rossberg, 2012/03/13 13:59:10]

These can be static_cast's.

[Sven Panne, 2012/03/14 15:26:56]

Done.

+    Accessors* second() const { return reinterpret_cast<Accessors*>(value); }
+
+   private:
+    static value_type* cast(ZoneHashMap::Entry* entry) {
+      return static_cast<value_type*>(entry);
+    }
+    friend class Iterator;
+  };
+
+  class Iterator {
+   public:
+    Iterator& operator++() {
+      entry_ = map_->Next(entry_);
+      return *this;
+    }
+
+    value_type* operator->() { return value_type::cast(entry_); }
+
+    bool operator!=(const Iterator& other) {
+      return  map_ != other.map_ || entry_ != other.entry_;

[rossberg, 2012/03/13 13:59:10]

Is the first condition necessary? I believe iterator comparison is undefined
anyway if pointing into different containers.

[Sven Panne, 2012/03/14 15:26:56]

In a first version, I left the first part out, but feared a comment saying "This
is not correct for blablabla reasons.", so I added it. :-) Actually the first
test is even totally removed by gcc in the loop condition for the iteration over
the table, because it can statically see that it is always false.

Removing it again...

+    }
+
+   private:
+    Iterator(const ZoneHashMap* map, ZoneHashMap::Entry* entry) :
+        map_(map), entry_(entry) { }
+
+    const ZoneHashMap* map_;
+    ZoneHashMap::Entry* entry_;
+
+    friend class AccessorTable;
+  };
+
+  explicit AccessorTable(Zone* zone) : map_(Literal::Match), zone_(zone) { }
+  Iterator begin() const { return Iterator(&map_, map_.Start()); }
+  Iterator end() const { return Iterator(&map_, NULL); }
+
+  Iterator find(Literal* key) {
+    ZoneHashMap::Entry* entry = map_.Lookup(key, key->Hash(), true);
+    if (entry->value == NULL) entry->value = new(zone_) Accessors();
+    return Iterator(&map_, entry);
+  }
+
+ private:
+  ZoneHashMap map_;
+  Zone* zone_;
+
+  DISALLOW_COPY_AND_ASSIGN(AccessorTable);
+};
+
+
 } }  // namespace v8::internal
 
 #endif  // V8_FULL_CODEGEN_H_