Remove the LookupResult IsValid method because it is confusing.

Replaced IsValid by IsPropertyOrTransition and used IsProperty in most
of the places where IsValid was used before.  Most of the time when
inspecting a lookup result we really want to know if there is a real
property present.  Only for stores are we interested in transitions.

BUG=http://crbug.com/20104
TEST=cctest/test-api/NamedInterceptorMapTransitionRead
Review URL: http://codereview.chromium.org/647015

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

src/api.cc

@@ -2207,7 +2207,7 @@ Local<Value> v8::Object::GetRealNamedPropertyInPrototypeChain(
   i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
   i::LookupResult lookup;
   self_obj->LookupRealNamedPropertyInPrototypes(*key_obj, &lookup);
-  if (lookup.IsValid()) {
+  if (lookup.IsProperty()) {
     PropertyAttributes attributes;
     i::Handle<i::Object> result(self_obj->GetProperty(*self_obj,
                                                       &lookup,
@@ -2226,7 +2226,7 @@ Local<Value> v8::Object::GetRealNamedProperty(Handle<String> key) {
   i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
   i::LookupResult lookup;
   self_obj->LookupRealNamedProperty(*key_obj, &lookup);
-  if (lookup.IsValid()) {
+  if (lookup.IsProperty()) {
     PropertyAttributes attributes;
     i::Handle<i::Object> result(self_obj->GetProperty(*self_obj,
                                                       &lookup,

src/arm/stub-cache-arm.cc

@@ -579,7 +579,7 @@ static void CompileLoadInterceptor(LoadInterceptorCompiler* compiler,
       stub_compiler->CheckPrototypes(object, receiver, holder,
                                      scratch1, scratch2, name, miss);
 
-  if (lookup->IsValid() && lookup->IsCacheable()) {
+  if (lookup->IsProperty() && lookup->IsCacheable()) {
     compiler->CompileCacheable(masm,
                                stub_compiler,
                                receiver,
@@ -985,7 +985,7 @@ Object* CallStubCompiler::CompileCallInterceptor(JSObject* object,
 
   // If we call a constant function when the interceptor returns
   // the no-result sentinel, generate code that optimizes this case.
-  if (lookup.IsValid() &&
+  if (lookup.IsProperty() &&
       lookup.IsCacheable() &&
       lookup.type() == CONSTANT_FUNCTION &&
       lookup.GetConstantFunction()->is_compiled() &&

src/bootstrapper.cc

@@ -723,11 +723,11 @@ void Genesis::CreateRoots(v8::Handle<v8::ObjectTemplate> global_template,
 #ifdef DEBUG
     LookupResult lookup;
     result->LocalLookup(Heap::callee_symbol(), &lookup);
-    ASSERT(lookup.IsValid() && (lookup.type() == FIELD));
+    ASSERT(lookup.IsProperty() && (lookup.type() == FIELD));
     ASSERT(lookup.GetFieldIndex() == Heap::arguments_callee_index);
 
     result->LocalLookup(Heap::length_symbol(), &lookup);
-    ASSERT(lookup.IsValid() && (lookup.type() == FIELD));
+    ASSERT(lookup.IsProperty() && (lookup.type() == FIELD));
     ASSERT(lookup.GetFieldIndex() == Heap::arguments_length_index);
 
     ASSERT(result->map()->inobject_properties() > Heap::arguments_callee_index);
@@ -1352,7 +1352,7 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
           LookupResult result;
           to->LocalLookup(descs->GetKey(i), &result);
           // If the property is already there we skip it
-          if (result.IsValid()) continue;
+          if (result.IsProperty()) continue;
           HandleScope inner;
           Handle<DescriptorArray> inst_descs =
               Handle<DescriptorArray>(to->map()->instance_descriptors());
@@ -1389,7 +1389,7 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
         // If the property is already there we skip it.
         LookupResult result;
         to->LocalLookup(String::cast(raw_key), &result);
-        if (result.IsValid()) continue;
+        if (result.IsProperty()) continue;
         // Set the property.
         Handle<String> key = Handle<String>(String::cast(raw_key));
         Handle<Object> value = Handle<Object>(properties->ValueAt(i));

src/fast-codegen.cc

@@ -220,7 +220,7 @@ void FastCodeGenSyntaxChecker::VisitVariableProxy(VariableProxy* expr) {
   if (info()->has_global_object()) {
     LookupResult lookup;
     info()->global_object()->Lookup(*expr->name(), &lookup);
-    if (!lookup.IsValid()) {
+    if (!lookup.IsProperty()) {
       BAILOUT("Non-existing global variable");
     }
     // We do not handle global variables with accessors or interceptors.
@@ -284,7 +284,7 @@ void FastCodeGenSyntaxChecker::VisitAssignment(Assignment* expr) {
     Handle<String> name = Handle<String>::cast(key->handle());
     LookupResult lookup;
     receiver->Lookup(*name, &lookup);
-    if (!lookup.IsValid()) {
+    if (!lookup.IsProperty()) {
       BAILOUT("Assigned property not found at compile time");
     }
     if (lookup.holder() != *receiver) BAILOUT("Non-own property assignment");
@@ -322,7 +322,7 @@ void FastCodeGenSyntaxChecker::VisitProperty(Property* expr) {
     Handle<String> name = Handle<String>::cast(key->handle());
     LookupResult lookup;
     receiver->Lookup(*name, &lookup);
-    if (!lookup.IsValid()) {
+    if (!lookup.IsProperty()) {
       BAILOUT("Referenced property not found at compile time");
     }
     if (lookup.holder() != *receiver) BAILOUT("Non-own property reference");
@@ -586,7 +586,7 @@ void FastCodeGenerator::VisitVariableProxy(VariableProxy* expr) {
   info()->global_object()->Lookup(*expr->name(), &lookup);
   // We only support normal (non-accessor/interceptor) DontDelete properties
   // for now.
-  ASSERT(lookup.IsValid());
+  ASSERT(lookup.IsProperty());
   ASSERT_EQ(NORMAL, lookup.type());
   ASSERT(lookup.IsDontDelete());
   Handle<Object> cell(info()->global_object()->GetPropertyCell(&lookup));

src/ia32/stub-cache-ia32.cc

@@ -323,7 +323,7 @@ static void CompileLoadInterceptor(Compiler* compiler,
       stub_compiler->CheckPrototypes(object, receiver, holder,
                                      scratch1, scratch2, name, miss);
 
-  if (lookup->IsValid() && lookup->IsCacheable()) {
+  if (lookup->IsProperty() && lookup->IsCacheable()) {
     compiler->CompileCacheable(masm,
                                stub_compiler,
                                receiver,
@@ -484,7 +484,7 @@ class CallOptimization BASE_EMBEDDED {
       is_simple_api_call_(false),
       expected_receiver_type_(NULL),
       api_call_info_(NULL) {
-    if (!lookup->IsValid() || !lookup->IsCacheable()) return;
+    if (!lookup->IsProperty() || !lookup->IsCacheable()) return;
 
     // We only optimize constant function calls.
     if (lookup->type() != CONSTANT_FUNCTION) return;

src/ic.cc

@@ -330,10 +330,11 @@ static void LookupForRead(Object* object,
   while (true) {
     object->Lookup(name, lookup);
     // Besides normal conditions (property not found or it's not
-    // an interceptor), bail out of lookup is not cacheable: we won't
+    // an interceptor), bail out if lookup is not cacheable: we won't
     // be able to IC it anyway and regular lookup should work fine.
-    if (lookup->IsNotFound() || lookup->type() != INTERCEPTOR ||
-        !lookup->IsCacheable()) {
+    if (!lookup->IsFound()
+        || (lookup->type() != INTERCEPTOR)
+        || !lookup->IsCacheable()) {
       return;
     }
 
@@ -343,7 +344,7 @@ static void LookupForRead(Object* object,
     }
 
     holder->LocalLookupRealNamedProperty(name, lookup);
-    if (lookup->IsValid()) {
+    if (lookup->IsProperty()) {
       ASSERT(lookup->type() != INTERCEPTOR);
       return;
     }
@@ -422,7 +423,7 @@ Object* CallIC::LoadFunction(State state,
   LookupResult lookup;
   LookupForRead(*object, *name, &lookup);
 
-  if (!lookup.IsValid()) {
+  if (!lookup.IsProperty()) {
     // If the object does not have the requested property, check which
     // exception we need to throw.
     if (IsContextual(object)) {
@@ -493,7 +494,7 @@ void CallIC::UpdateCaches(LookupResult* lookup,
                           Handle<String> name) {
   ASSERT(lookup->IsLoaded());
   // Bail out if we didn't find a result.
-  if (!lookup->IsValid() || !lookup->IsCacheable()) return;
+  if (!lookup->IsProperty() || !lookup->IsCacheable()) return;
 
   // Compute the number of arguments.
   int argc = target()->arguments_count();
@@ -642,8 +643,8 @@ Object* LoadIC::Load(State state, Handle<Object> object, Handle<String> name) {
   LookupResult lookup;
   LookupForRead(*object, *name, &lookup);
 
-  // If lookup is invalid, check if we need to throw an exception.
-  if (!lookup.IsValid()) {
+  // If we did not find a property, check if we need to throw an exception.
+  if (!lookup.IsProperty()) {
     if (FLAG_strict || IsContextual(object)) {
       return ReferenceError("not_defined", name);
     }
@@ -653,7 +654,7 @@ Object* LoadIC::Load(State state, Handle<Object> object, Handle<String> name) {
   bool can_be_inlined =
       FLAG_use_ic &&
       state == PREMONOMORPHIC &&
-      lookup.IsValid() &&
+      lookup.IsProperty() &&
       lookup.IsLoaded() &&
       lookup.IsCacheable() &&
       lookup.holder() == *object &&
@@ -681,7 +682,7 @@ Object* LoadIC::Load(State state, Handle<Object> object, Handle<String> name) {
   }
 
   PropertyAttributes attr;
-  if (lookup.IsValid() && lookup.type() == INTERCEPTOR) {
+  if (lookup.IsProperty() && lookup.type() == INTERCEPTOR) {
     // Get the property.
     Object* result = object->GetProperty(*object, &lookup, *name, &attr);
     if (result->IsFailure()) return result;
@@ -704,7 +705,7 @@ void LoadIC::UpdateCaches(LookupResult* lookup,
                           Handle<String> name) {
   ASSERT(lookup->IsLoaded());
   // Bail out if we didn't find a result.
-  if (!lookup->IsValid() || !lookup->IsCacheable()) return;
+  if (!lookup->IsProperty() || !lookup->IsCacheable()) return;
 
   // Loading properties from values is not common, so don't try to
   // deal with non-JS objects here.
@@ -857,8 +858,8 @@ Object* KeyedLoadIC::Load(State state,
     LookupResult lookup;
     LookupForRead(*object, *name, &lookup);
 
-    // If lookup is invalid, check if we need to throw an exception.
-    if (!lookup.IsValid()) {
+    // If we did not find a property, check if we need to throw an exception.
+    if (!lookup.IsProperty()) {
       if (FLAG_strict || IsContextual(object)) {
         return ReferenceError("not_defined", name);
       }
@@ -869,7 +870,7 @@ Object* KeyedLoadIC::Load(State state,
     }
 
     PropertyAttributes attr;
-    if (lookup.IsValid() && lookup.type() == INTERCEPTOR) {
+    if (lookup.IsProperty() && lookup.type() == INTERCEPTOR) {
       // Get the property.
       Object* result = object->GetProperty(*object, &lookup, *name, &attr);
       if (result->IsFailure()) return result;
@@ -921,7 +922,7 @@ void KeyedLoadIC::UpdateCaches(LookupResult* lookup, State state,
                                Handle<Object> object, Handle<String> name) {
   ASSERT(lookup->IsLoaded());
   // Bail out if we didn't find a result.
-  if (!lookup->IsValid() || !lookup->IsCacheable()) return;
+  if (!lookup->IsProperty() || !lookup->IsCacheable()) return;
 
   if (!object->IsJSObject()) return;
   Handle<JSObject> receiver = Handle<JSObject>::cast(object);
@@ -994,7 +995,7 @@ void KeyedLoadIC::UpdateCaches(LookupResult* lookup, State state,
 
 static bool StoreICableLookup(LookupResult* lookup) {
   // Bail out if we didn't find a result.
-  if (!lookup->IsValid() || !lookup->IsCacheable()) return false;
+  if (!lookup->IsPropertyOrTransition() || !lookup->IsCacheable()) return false;
 
   // If the property is read-only, we leave the IC in its current
   // state.
@@ -1214,7 +1215,7 @@ void KeyedStoreIC::UpdateCaches(LookupResult* lookup,
   if (receiver->IsJSGlobalProxy()) return;
 
   // Bail out if we didn't find a result.
-  if (!lookup->IsValid() || !lookup->IsCacheable()) return;
+  if (!lookup->IsPropertyOrTransition() || !lookup->IsCacheable()) return;
 
   // If the property is read-only, we leave the IC in its current
   // state.

src/property.cc

@@ -33,7 +33,7 @@ namespace internal {
 
 #ifdef DEBUG
 void LookupResult::Print() {
-  if (!IsValid()) {
+  if (!IsFound()) {
     PrintF("Not Found\n");
     return;
   }

src/property.h

@@ -201,23 +201,17 @@ class LookupResult BASE_EMBEDDED {
   }
 
   JSObject* holder() {
-    ASSERT(IsValid());
+    ASSERT(IsFound());
     return holder_;
   }
 
   PropertyType type() {
-    ASSERT(IsValid());
+    ASSERT(IsFound());
     return details_.type();
   }
 
-  bool IsTransitionType() {
-    PropertyType t = type();
-    if (t == MAP_TRANSITION || t == CONSTANT_TRANSITION) return true;
-    return false;
-  }
-
   PropertyAttributes GetAttributes() {
-    ASSERT(IsValid());
+    ASSERT(IsFound());
     return details_.attributes();
   }
 
@@ -229,14 +223,17 @@ class LookupResult BASE_EMBEDDED {
   bool IsDontDelete() { return details_.IsDontDelete(); }
   bool IsDontEnum() { return details_.IsDontEnum(); }
   bool IsDeleted() { return details_.IsDeleted(); }
+  bool IsFound() { return lookup_type_ != NOT_FOUND; }
 
-  bool IsValid() { return  lookup_type_ != NOT_FOUND; }
-  bool IsNotFound() { return lookup_type_ == NOT_FOUND; }
-
-  // Tells whether 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 IsValid() && type() < FIRST_PHANTOM_PROPERTY_TYPE;
+    return IsFound() && (type() < FIRST_PHANTOM_PROPERTY_TYPE);
+  }
+
+  // Is the result a property or a transition?
+  bool IsPropertyOrTransition() {
+    return IsFound() && (type() != NULL_DESCRIPTOR);
   }
 
   bool IsCacheable() { return cacheable_; }

src/runtime.cc

@@ -2895,7 +2895,7 @@ static Object* Runtime_DefineOrRedefineAccessorProperty(Arguments args) {
   // If an existing property is either FIELD, NORMAL or CONSTANT_FUNCTION
   // delete it to avoid running into trouble in DefineAccessor, which
   // handles this incorrectly if the property is readonly (does nothing)
-  if (result.IsValid() &&
+  if (result.IsProperty() &&
       (result.type() == FIELD || result.type() == NORMAL
        || result.type() == CONSTANT_FUNCTION)) {
     obj->DeleteProperty(name, JSObject::NORMAL_DELETION);

src/stub-cache.cc

@@ -1070,11 +1070,13 @@ Object* StubCompiler::GetCodeWithFlags(Code::Flags flags, String* name) {
   return GetCodeWithFlags(flags, reinterpret_cast<char*>(NULL));
 }
 
+
 void StubCompiler::LookupPostInterceptor(JSObject* holder,
                                          String* name,
                                          LookupResult* lookup) {
   holder->LocalLookupRealNamedProperty(name, lookup);
-  if (lookup->IsNotFound()) {
+  if (!lookup->IsProperty()) {
+    lookup->NotFound();
     Object* proto = holder->GetPrototype();
     if (proto != Heap::null_value()) {
       proto->Lookup(name, lookup);

src/x64/stub-cache-x64.cc

@@ -399,7 +399,7 @@ static void CompileLoadInterceptor(Compiler* compiler,
       stub_compiler->CheckPrototypes(object, receiver, holder,
                                      scratch1, scratch2, name, miss);
 
-  if (lookup->IsValid() && lookup->IsCacheable()) {
+  if (lookup->IsProperty() && lookup->IsCacheable()) {
     compiler->CompileCacheable(masm,
                                stub_compiler,
                                receiver,

test/cctest/test-api.cc

@@ -2528,6 +2528,33 @@ THREADED_TEST(NamedInterceptorPropertyRead) {
 }
 
 
+static v8::Handle<Value> SetXOnPrototypeGetter(Local<String> property,
+                                               const AccessorInfo& info) {
+  // Set x on the prototype object and do not handle the get request.
+  v8::Handle<v8::Value> proto = info.Holder()->GetPrototype();
+  v8::Handle<v8::Object>::Cast(proto)->Set(v8_str("x"), v8::Integer::New(23));
+  return v8::Handle<Value>();
+}
+
+
+// This is a regression test for http://crbug.com/20104. Map
+// transitions should not interfere with post interceptor lookup.
+THREADED_TEST(NamedInterceptorMapTransitionRead) {
+  v8::HandleScope scope;
+  Local<v8::FunctionTemplate> function_template = v8::FunctionTemplate::New();
+  Local<v8::ObjectTemplate> instance_template
+      = function_template->InstanceTemplate();
+  instance_template->SetNamedPropertyHandler(SetXOnPrototypeGetter);
+  LocalContext context;
+  context->Global()->Set(v8_str("F"), function_template->GetFunction());
+  // Create an instance of F and introduce a map transition for x.
+  CompileRun("var o = new F(); o.x = 23;");
+  // Create an instance of F and invoke the getter. The result should be 23.
+  Local<Value> result = CompileRun("o = new F(); o.x");
+  CHECK_EQ(result->Int32Value(), 23);
+}
+
+
 static v8::Handle<Value> IndexedPropertyGetter(uint32_t index,
                                                const AccessorInfo& info) {
   ApiTestFuzzer::Fuzz();