Revision: 10906
Author:   [email protected]
Date:     Fri Mar  2 06:03:59 2012
Log:      Re-land CL 9466047.

Main change from the original CL: Call::ComputeTarget does not use IsProperty
anymore, because this would potentially need a holder, which we don't have
here. Using Map::LookupInDescriptors with a NULL holder is a bit fishy in
general, because one has to be *extremely* careful when using its LookupResult.

The original CL made Chrome's NetInternalsTest.netInternalsTourTabs browser test
fail, but it's a mystery how this could happen: We should never reach
Call::ComputeTarget via Call::RecordTypeFeedback with a CALLBACKS property,
because we never consider calls to them monomorphic, which is in turn because of
the stub cache leaving them in the pre-monomorphic state. Therefore, I don't
have a clue how to write a regression test for this...

As an additional tiny bonus, the --trace-opt output for deoptimizations has been
improved.

Review URL: https://chromiumcodereview.appspot.com/9584003
http://code.google.com/p/v8/source/detail?r=10906

Modified:
 /branches/bleeding_edge/src/ast.cc
 /branches/bleeding_edge/src/objects-inl.h
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/src/objects.h
 /branches/bleeding_edge/src/property-details.h
 /branches/bleeding_edge/src/property.h

=======================================
--- /branches/bleeding_edge/src/ast.cc  Wed Feb 29 04:12:52 2012
+++ /branches/bleeding_edge/src/ast.cc  Fri Mar  2 06:03:59 2012
@@ -520,13 +520,26 @@
   LookupResult lookup(type->GetIsolate());
   while (true) {
     type->LookupInDescriptors(NULL, *name, &lookup);
-    // For properties we know the target iff we have a constant function.
-    if (lookup.IsFound() && lookup.IsProperty()) {
-      if (lookup.type() == CONSTANT_FUNCTION) {
- target_ = Handle<JSFunction>(lookup.GetConstantFunctionFromMap(*type));
-        return true;
-      }
-      return false;
+    if (lookup.IsFound()) {
+      switch (lookup.type()) {
+        case CONSTANT_FUNCTION:
+          // We surely know the target for a constant function.
+ target_ = Handle<JSFunction>(lookup.GetConstantFunctionFromMap(*type));
+          return true;
+        case NORMAL:
+        case FIELD:
+        case CALLBACKS:
+        case HANDLER:
+        case INTERCEPTOR:
+          // We don't know the target.
+          return false;
+        case MAP_TRANSITION:
+        case ELEMENTS_TRANSITION:
+        case CONSTANT_TRANSITION:
+        case NULL_DESCRIPTOR:
+          // Perhaps something interesting is up in the prototype chain...
+          break;
+      }
     }
// If we reach the end of the prototype chain, we don't know the target.
     if (!type->prototype()->IsJSObject()) return false;
=======================================
--- /branches/bleeding_edge/src/objects-inl.h   Wed Feb 29 05:29:17 2012
+++ /branches/bleeding_edge/src/objects-inl.h   Fri Mar  2 06:03:59 2012
@@ -1988,7 +1988,8 @@


 bool DescriptorArray::IsProperty(int descriptor_number) {
-  return IsRealProperty(GetType(descriptor_number));
+  Entry entry(this, descriptor_number);
+  return IsPropertyDescriptor(&entry);
 }


=======================================
--- /branches/bleeding_edge/src/objects.cc      Wed Feb 29 06:25:24 2012
+++ /branches/bleeding_edge/src/objects.cc      Fri Mar  2 06:03:59 2012
@@ -7880,9 +7880,7 @@
     code()->set_optimizable(false);
   }
   if (FLAG_trace_opt) {
-    PrintF("[disabled optimization for: ");
-    DebugName()->ShortPrint();
-    PrintF("]\n");
+    PrintF("[disabled optimization for %s]\n", *DebugName()->ToCString());
   }
 }

=======================================
--- /branches/bleeding_edge/src/objects.h       Fri Mar  2 03:33:33 2012
+++ /branches/bleeding_edge/src/objects.h       Fri Mar  2 06:03:59 2012
@@ -2589,6 +2589,20 @@
   static const int kMaxNumberOfDescriptors = 1024 + 512;

  private:
+  // An entry in a DescriptorArray, represented as an (array, index) pair.
+  class Entry {
+   public:
+    inline explicit Entry(DescriptorArray* descs, int index) :
+        descs_(descs), index_(index) { }
+
+    inline PropertyType type() { return descs_->GetType(index_); }
+    inline Object* GetCallbackObject() { return descs_->GetValue(index_); }
+
+   private:
+    DescriptorArray* descs_;
+    int index_;
+  };
+
   // Conversion from descriptor number to array indices.
   static int ToKeyIndex(int descriptor_number) {
     return descriptor_number+kFirstIndex;
@@ -7921,6 +7935,10 @@
       set_setter(value);
     }
   }
+
+  bool ContainsAccessor() {
+    return IsJSAccessor(getter()) || IsJSAccessor(setter());
+  }

 #ifdef OBJECT_PRINT
   void AccessorPairPrint(FILE* out = stdout);
@@ -7934,6 +7952,15 @@
   static const int kSize = kSetterOffset + kPointerSize;

  private:
+ // Strangely enough, in addition to functions and harmony proxies, the spec
+  // requires us to consider undefined as a kind of accessor, too:
+  //    var obj = {};
+  //    Object.defineProperty(obj, "foo", {get: undefined});
+  //    assertTrue("foo" in obj);
+  bool IsJSAccessor(Object* obj) {
+    return obj->IsSpecFunction() || obj->IsUndefined();
+  }
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(AccessorPair);
 };

=======================================
--- /branches/bleeding_edge/src/property-details.h      Wed Feb 29 05:29:17 2012
+++ /branches/bleeding_edge/src/property-details.h      Fri Mar  2 06:03:59 2012
@@ -73,26 +73,6 @@
 };


-inline bool IsRealProperty(PropertyType type) {
-  switch (type) {
-    case NORMAL:
-    case FIELD:
-    case CONSTANT_FUNCTION:
-    case CALLBACKS:
-    case HANDLER:
-    case INTERCEPTOR:
-      return true;
-    case MAP_TRANSITION:
-    case ELEMENTS_TRANSITION:
-    case CONSTANT_TRANSITION:
-    case NULL_DESCRIPTOR:
-      return false;
-  }
-  UNREACHABLE();  // keep the compiler happy
-  return false;
-}
-
-
 // PropertyDetails captures type and attributes for a property.
 // They are used both in property dictionaries and instance descriptors.
 class PropertyDetails BASE_EMBEDDED {
=======================================
--- /branches/bleeding_edge/src/property.h      Wed Feb 29 05:29:17 2012
+++ /branches/bleeding_edge/src/property.h      Fri Mar  2 06:03:59 2012
@@ -162,6 +162,35 @@
                       int index = 0)
       : Descriptor(key, foreign, attributes, CALLBACKS, index) {}
 };
+
+
+template <class T>
+bool IsPropertyDescriptor(T* desc) {
+  switch (desc->type()) {
+    case NORMAL:
+    case FIELD:
+    case CONSTANT_FUNCTION:
+    case HANDLER:
+    case INTERCEPTOR:
+      return true;
+    case CALLBACKS: {
+      Object* callback_object = desc->GetCallbackObject();
+ // Non-JavaScript (i.e. native) accessors are always a property, otherwise + // either the getter or the setter must be an accessor. Put another way:
+      // If we only see map transitions and holes in a pair, this is not a
+      // property.
+      return (!callback_object->IsAccessorPair() ||
+              AccessorPair::cast(callback_object)->ContainsAccessor());
+    }
+    case MAP_TRANSITION:
+    case ELEMENTS_TRANSITION:
+    case CONSTANT_TRANSITION:
+    case NULL_DESCRIPTOR:
+      return false;
+  }
+  UNREACHABLE();  // keep the compiler happy
+  return false;
+}


 class LookupResult BASE_EMBEDDED {
@@ -261,10 +290,9 @@
   bool IsFound() { return lookup_type_ != NOT_FOUND; }
   bool IsHandler() { return lookup_type_ == HANDLER_TYPE; }

-  // Is the result is a property excluding transitions and the null
-  // descriptor?
+ // Is the result is a property excluding transitions and the null descriptor?
   bool IsProperty() {
-    return IsFound() && IsRealProperty(GetPropertyDetails().type());
+    return IsFound() && IsPropertyDescriptor(this);
   }

   bool IsCacheable() { return cacheable_; }

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to