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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10906 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/ast.cc b/src/ast.cc
index b19fc35..d0aa0f0 100644 (file)
--- a/src/ast.cc
+++ b/src/ast.cc
@@ -520,13 +520,26 @@ bool Call::ComputeTarget(Handle<Map> type, Handle<String> name) {
   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;
+    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;
       }
-      return false;
     }
     // If we reach the end of the prototype chain, we don't know the target.
     if (!type->prototype()->IsJSObject()) return false;

diff --git a/src/objects-inl.h b/src/objects-inl.h
index bc6217b..176be9b 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1988,7 +1988,8 @@ AccessorDescriptor* DescriptorArray::GetCallbacks(int descriptor_number) {
 
 
 bool DescriptorArray::IsProperty(int descriptor_number) {
-  return IsRealProperty(GetType(descriptor_number));
+  Entry entry(this, descriptor_number);
+  return IsPropertyDescriptor(&entry);
 }
 
 

diff --git a/src/objects.cc b/src/objects.cc
index bc0a82e..a27cc67 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -7880,9 +7880,7 @@ void SharedFunctionInfo::DisableOptimization() {
     code()->set_optimizable(false);
   }
   if (FLAG_trace_opt) {
-    PrintF("[disabled optimization for: ");
-    DebugName()->ShortPrint();
-    PrintF("]\n");
+    PrintF("[disabled optimization for %s]\n", *DebugName()->ToCString());
   }
 }
 

diff --git a/src/objects.h b/src/objects.h
index 7d8a55e..482b794 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -2589,6 +2589,20 @@ class DescriptorArray: public FixedArray {
   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;
@@ -7922,6 +7936,10 @@ class AccessorPair: public Struct {
     }
   }
 
+  bool ContainsAccessor() {
+    return IsJSAccessor(getter()) || IsJSAccessor(setter());
+  }
+
 #ifdef OBJECT_PRINT
   void AccessorPairPrint(FILE* out = stdout);
 #endif
@@ -7934,6 +7952,15 @@ class AccessorPair: public Struct {
   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);
 };
 

diff --git a/src/property-details.h b/src/property-details.h
index 81f521a..c79aa96 100644 (file)
--- a/src/property-details.h
+++ b/src/property-details.h
@@ -73,26 +73,6 @@ enum PropertyType {
 };
 
 
-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 {

diff --git a/src/property.h b/src/property.h
index d5efb7f..9235c32 100644 (file)
--- a/src/property.h
+++ b/src/property.h
@@ -164,6 +164,35 @@ class CallbacksDescriptor:  public Descriptor {
 };
 
 
+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 {
  public:
   explicit LookupResult(Isolate* isolate)
@@ -261,10 +290,9 @@ class LookupResult BASE_EMBEDDED {
   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_; }