Fix bad optimization of instance-of with uninstantiated types (issue 5216).

runtime/vm/object.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/object.h"
 
 #include "include/dart_api.h"
 #include "platform/assert.h"
 #include "vm/assembler.h"
 #include "vm/bigint_operations.h"
@@ skipping 26 matching lines @@
 
 DEFINE_FLAG(bool, generate_gdb_symbols, false,
     "Generate symbols of generated dart functions for debugging with GDB");
 DEFINE_FLAG(bool, reject_named_argument_as_positional, false,
     "Enforce new rules for optional parameters and disallow passing of named "
     "arguments to optional positional formal parameters");
 DEFINE_FLAG(bool, show_internal_names, false,
     "Show names of internal classes (e.g. \"OneByteString\") in error messages "
     "instead of showing the corresponding interface names (e.g. \"String\")");
 DECLARE_FLAG(bool, trace_compiler);
+DECLARE_FLAG(bool, eliminate_type_checks);
 DECLARE_FLAG(bool, enable_type_checks);
 
 static const char* kGetterPrefix = "get:";
 static const intptr_t kGetterPrefixLength = strlen(kGetterPrefix);
 static const char* kSetterPrefix = "set:";
 static const intptr_t kSetterPrefixLength = strlen(kSetterPrefix);
 
 cpp_vtable Object::handle_vtable_ = 0;
 cpp_vtable Object::builtin_vtables_[kNumPredefinedCids] = { 0 };
 cpp_vtable Smi::handle_vtable_ = 0;
@@ skipping 8469 matching lines @@
   SetFieldAtOffset(field_offset, value);
 }
 
 
 bool Instance::IsInstanceOf(const AbstractType& other,
                             const AbstractTypeArguments& other_instantiator,
                             Error* malformed_error) const {
   ASSERT(other.IsFinalized());
   ASSERT(!other.IsDynamicType());
   ASSERT(!other.IsMalformed());
-  if (IsNull()) {
+  const Class& cls = Class::Handle(clazz());
+  if (cls.IsNullClass()) {
+    if (!IsNull()) {
+      // We can only encounter Object::sentinel() or
+      // Object::transition_sentinel() if type checks were not eliminated at
+      // compile time. Both sentinels are instances of the Null class, but they

[zerny-google, 2012/09/18 11:20:24]

Can we assert that?

[regis, 2012/09/18 14:55:44]

There is already an assert for !FLAG_eliminate_type_checks. I added one checking
that this instance is indeed one of the  sentinels.

+      // are not the Object::null() instance.
+      ASSERT(!FLAG_eliminate_type_checks);
+      return true;  // We are doing an instance of test as part of a type check.
+    }
     // The null instance can be returned from a void function.
     if (other.IsVoidType()) {
       return true;
     }
     // Otherwise, null is only an instance of Object and of Dynamic.
     // It is not necessary to fully instantiate the other type for this test.
     Class& other_class = Class::Handle();
     if (other.IsTypeParameter()) {
       if (other_instantiator.IsNull()) {
         return true;  // Other type is uninstantiated, i.e. Dynamic.
       }
       const TypeParameter& other_type_param = TypeParameter::Cast(other);
       const AbstractType& instantiated_other = AbstractType::Handle(
           other_instantiator.TypeAt(other_type_param.index()));
       ASSERT(instantiated_other.IsInstantiated());
       other_class = instantiated_other.type_class();
     } else {
       other_class = other.type_class();
     }
     return other_class.IsObjectClass() || other_class.IsDynamicClass();
   }
   if (other.IsVoidType()) {
     return false;
   }
-  const Class& cls = Class::Handle(clazz());
-  // We must not encounter Object::sentinel() or Object::transition_sentinel(),
-  // both instances of class NullClass, but not instance Object::null().
-  ASSERT(!cls.IsNullClass());
   AbstractTypeArguments& type_arguments = AbstractTypeArguments::Handle();
   const intptr_t num_type_arguments = cls.NumTypeArguments();
   if (num_type_arguments > 0) {
     type_arguments = GetTypeArguments();
     if (!type_arguments.IsNull() && !type_arguments.IsCanonical()) {
       type_arguments = type_arguments.Canonicalize();
       SetTypeArguments(type_arguments);
     }
     // Verify that the number of type arguments in the instance matches the
     // number of type arguments expected by the instance class.
@@ skipping 3040 matching lines @@
   }
   return result.raw();
 }
 
 
 const char* WeakProperty::ToCString() const {
   return "_WeakProperty";
 }
 
 }  // namespace dart