Revert "[VM/nnbd] Pass nullability when creating Class::DeclarationType."

This reverts commit ea57b1e.

Reason for revert: Please see https://buganizer.corp.google.com/issues/144304690

Original change's description:
> [VM/nnbd] Pass nullability when creating Class::DeclarationType.
> 
> Passing nullability rather than forcing kLegacy helps slightly reduce the number
> of Types in the VM and in snapshots.
> This allows to remove the kludge about declaration type of Null being kNullable.
> Also pass nullability when creating a simple type may help.
> 
> Note that CFE is still producing super types and some interface types that are
> non-nullable, even with nnbd features disabled. But this should get fixed and
> further reduce the number of types in the snapshot.
> 
> Change-Id: I397f00902026ee5bc91d36328f4156427702efdb
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124480
> Commit-Queue: Régis Crelier <regis@google.com>
> Reviewed-by: Alexander Markov <alexmarkov@google.com>

TBR=rmacnak@google.com,alexmarkov@google.com,asiva@google.com,regis@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Icae0a69cda6cc531bba4d5d12471e7267a277340
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125102
Reviewed-by: Siva Annamalai <asiva@google.com>

Author: a-siva

runtime/vm/compiler/backend/il_deserializer.cc

@@ -1848,7 +1848,7 @@ bool FlowGraphDeserializer::ParseType(SExpression* sexp, Object* out) {
   }
   // Guaranteed not to re-enter ParseType.
   if (!ParseClass(cls_sexp, &type_class_)) return false;
-  *out = Type::New(type_class_, *type_args_ptr, token_pos);
+  *out = Type::New(type_class_, *type_args_ptr, token_pos, Heap::kOld);
   auto& type = Type::Cast(*out);
   if (auto const sig_sexp = list->ExtraLookupValue("signature")) {
     auto& function = Function::Handle(zone());

runtime/vm/compiler/frontend/bytecode_reader.cc

@@ -1669,7 +1669,14 @@ RawObject* BytecodeReaderHelper::ReadType(intptr_t tag,
       if (!cls.is_declaration_loaded()) {
         LoadReferencedClass(cls);
       }
-      return cls.DeclarationType(nullability);
+      Type& type = Type::Handle(Z, cls.DeclarationType());
+      // TODO(regis): Remove this workaround once nullability of Null provided
+      // by CFE is always kNullable.
+      if (type.IsNullType()) {
+        ASSERT(type.IsNullable());
+        return type.raw();
+      }
+      return type.ToNullability(nullability, Heap::kOld);
     }
     case kTypeParameter: {
       Object& parent = Object::Handle(Z, ReadObject());
@@ -1703,9 +1710,9 @@ RawObject* BytecodeReaderHelper::ReadType(intptr_t tag,
       }
       const TypeArguments& type_arguments =
           TypeArguments::CheckedHandle(Z, ReadObject());
-      const Type& type =
-          Type::Handle(Z, Type::New(cls, type_arguments,
-                                    TokenPosition::kNoSource, nullability));
+      const Type& type = Type::Handle(
+          Z, Type::New(cls, type_arguments, TokenPosition::kNoSource));
+      type.set_nullability(nullability);
       type.SetIsFinalized();
       return type.Canonicalize();
     }
@@ -1735,9 +1742,9 @@ RawObject* BytecodeReaderHelper::ReadType(intptr_t tag,
       pending_recursive_types_->SetLength(id);
       pending_recursive_types_ = saved_pending_recursive_types;
 
-      Type& type =
-          Type::Handle(Z, Type::New(cls, type_arguments,
-                                    TokenPosition::kNoSource, nullability));
+      Type& type = Type::Handle(
+          Z, Type::New(cls, type_arguments, TokenPosition::kNoSource));
+      type.set_nullability(nullability);
       type_ref.set_type(type);
       type.SetIsFinalized();
       if (id != 0) {

runtime/vm/compiler/frontend/kernel_to_il.cc

@@ -2275,7 +2275,8 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfNoSuchMethodForwarder(
           Function::ZoneHandle(Z, function.parent_function());
       const Class& owner = Class::ZoneHandle(Z, parent.Owner());
       AbstractType& type = AbstractType::ZoneHandle(Z);
-      type = Type::New(owner, TypeArguments::Handle(Z), owner.token_pos());
+      type = Type::New(owner, TypeArguments::Handle(Z), owner.token_pos(),
+                       Heap::kOld);
       type = ClassFinalizer::FinalizeType(owner, type);
       body += Constant(type);
     } else {

runtime/vm/compiler/frontend/kernel_translation_helper.cc

@@ -2767,8 +2767,9 @@ void TypeTranslator::BuildTypeInternal() {
       result_ = Object::void_type().raw();
       break;
     case kBottomType:
-      result_ = Class::Handle(Z, I->object_store()->null_class())
-                    .DeclarationType(kNullable);
+      result_ =
+          Class::Handle(Z, I->object_store()->null_class()).DeclarationType();
+      // We set the nullability of Null to kNullable, even in legacy mode.
       ASSERT(result_.IsNullable());
       break;
     case kNeverType:
@@ -2813,13 +2814,18 @@ void TypeTranslator::BuildInterfaceType(bool simple) {
   if (simple) {
     if (finalize_ || klass.is_type_finalized()) {
       // Fast path for non-generic types: retrieve or populate the class's only
-      // canonical type (as long as only one nullability variant is used), which
-      // is its declaration type.
-      result_ = klass.DeclarationType(nullability);
+      // canonical type, which is its declaration type.
+      result_ = klass.DeclarationType();
+      // TODO(regis): Remove this workaround once nullability of Null provided
+      // by CFE is always kNullable.
+      if (!result_.IsNullType()) {
+        result_ = Type::Cast(result_).ToNullability(nullability, Heap::kOld);
+      }
     } else {
       // Note that the type argument vector is not yet extended.
-      result_ = Type::New(klass, Object::null_type_arguments(),
-                          klass.token_pos(), nullability);
+      result_ =
+          Type::New(klass, Object::null_type_arguments(), klass.token_pos());
+      Type::Cast(result_).set_nullability(nullability);
     }
     return;
   }
@@ -2924,7 +2930,8 @@ void TypeTranslator::BuildFunctionType(bool simple) {
   finalize_ = finalize;
 
   Type& signature_type =
-      Type::ZoneHandle(Z, signature_function.SignatureType(nullability));
+      Type::ZoneHandle(Z, signature_function.SignatureType());
+  signature_type.set_nullability(nullability);
 
   if (finalize_) {
     signature_type ^=

runtime/vm/object.cc

@@ -944,13 +944,13 @@ void Object::Init(Isolate* isolate) {
   cls.set_is_type_finalized();
 
   cls = dynamic_class_;
-  *dynamic_type_ = Type::NewNonParameterizedType(cls, kNullable);
+  *dynamic_type_ = Type::NewNonParameterizedType(cls);
 
   cls = void_class_;
-  *void_type_ = Type::NewNonParameterizedType(cls, kNullable);
+  *void_type_ = Type::NewNonParameterizedType(cls);
 
   cls = never_class_;
-  *never_type_ = Type::NewNonParameterizedType(cls, kNonNullable);
+  *never_type_ = Type::NewNonParameterizedType(cls);
 
   // Since TypeArguments objects are passed as function arguments, make them
   // behave as Dart instances, although they are just VM objects.
@@ -1567,7 +1567,7 @@ RawError* Object::Init(Isolate* isolate,
     cls = object_store->array_class();  // Was allocated above.
     RegisterPrivateClass(cls, Symbols::_List(), core_lib);
     pending_classes.Add(cls);
-    // We cannot use NewNonParameterizedType(), because Array is
+    // We cannot use NewNonParameterizedType(cls), because Array is
     // parameterized.  Warning: class _List has not been patched yet. Its
     // declared number of type parameters is still 0. It will become 1 after
     // patching. The array type allocated below represents the raw type _List
@@ -1934,8 +1934,7 @@ RawError* Object::Init(Isolate* isolate,
     // name is a built-in identifier (this is wrong).  The corresponding types
     // are stored in the object store.
     cls = object_store->null_class();
-    type = Type::NewNonParameterizedType(cls, kNullable);
-    cls.set_declaration_type(type);
+    type = Type::NewNonParameterizedType(cls);
     object_store->set_null_type(type);
     ASSERT(type.IsNullable());
 
@@ -4336,23 +4335,18 @@ void Class::set_declaration_type(const Type& value) const {
   // TODO(regis): Since declaration type is used as the runtime type of
   // instances of a non-generic class, the nullability should be set to
   // kNonNullable instead of kLegacy.
-  // For now, we accept any except for Null (kNullable).
-  ASSERT(!value.IsNullType() || value.IsNullable());
+  // For now, we set the nullability to kLegacy, except for Null.
+  ASSERT(value.IsLegacy() || (value.IsNullType() && value.IsNullable()));
   StorePointer(&raw_ptr()->declaration_type_, value.raw());
 }
 
-RawType* Class::DeclarationType(Nullability nullability) const {
+RawType* Class::DeclarationType() const {
   ASSERT(is_declaration_loaded());
-  if (IsNullClass()) {
-    // Ignore requested nullability (e.g. by mirrors).
-    nullability = kNullable;
-  }
-  Type& type = Type::Handle(declaration_type());
-  if (!type.IsNull()) {
-    return type.ToNullability(nullability, Heap::kOld);
+  if (declaration_type() != Type::null()) {
+    return declaration_type();
   }
-  type = Type::New(*this, TypeArguments::Handle(type_parameters()), token_pos(),
-                   nullability);
+  Type& type = Type::Handle(
+      Type::New(*this, TypeArguments::Handle(type_parameters()), token_pos()));
   type ^= ClassFinalizer::FinalizeType(*this, type);
   set_declaration_type(type);
   return type.raw();
@@ -6324,8 +6318,8 @@ RawType* Function::SignatureType(Nullability nullability) const {
     const TypeArguments& signature_type_arguments =
         TypeArguments::Handle(scope_class.type_parameters());
     // Return the still unfinalized signature type.
-    type = Type::New(scope_class, signature_type_arguments, token_pos(),
-                     nullability);
+    type = Type::New(scope_class, signature_type_arguments, token_pos());
+    type.set_nullability(nullability);
     type.set_signature(*this);
     SetSignatureType(type);
   }
@@ -16662,10 +16656,7 @@ RawAbstractType* Instance::GetType(Heap::Space space) const {
     if (cls.NumTypeArguments() > 0) {
       type_arguments = GetTypeArguments();
     }
-    // TODO(regis): The runtime type of a non-null instance should be
-    // non-nullable instead of legacy. Revisit.
-    type = Type::New(cls, type_arguments, TokenPosition::kNoSource, kLegacy,
-                     space);
+    type = Type::New(cls, type_arguments, TokenPosition::kNoSource, space);
     type.SetIsFinalized();
     type ^= type.Canonicalize();
   }
@@ -17649,22 +17640,20 @@ RawType* Type::DartTypeType() {
   return Isolate::Current()->object_store()->type_type();
 }
 
-RawType* Type::NewNonParameterizedType(const Class& type_class,
-                                       Nullability nullability) {
+RawType* Type::NewNonParameterizedType(const Class& type_class) {
   ASSERT(type_class.NumTypeArguments() == 0);
   // It is too early to use the class finalizer, as type_class may not be named
   // yet, so do not call DeclarationType().
   Type& type = Type::Handle(type_class.declaration_type());
   if (type.IsNull()) {
     type = Type::New(Class::Handle(type_class.raw()),
-                     Object::null_type_arguments(), TokenPosition::kNoSource,
-                     nullability);
+                     Object::null_type_arguments(), TokenPosition::kNoSource);
     type.SetIsFinalized();
     type ^= type.Canonicalize();
     type_class.set_declaration_type(type);
   }
   ASSERT(type.IsFinalized());
-  return type.ToNullability(nullability, Heap::kOld);
+  return type.raw();
 }
 
 void Type::SetIsFinalized() const {
@@ -17800,8 +17789,8 @@ RawAbstractType* Type::InstantiateFrom(
   }
   // This uninstantiated type is not modified, as it can be instantiated
   // with different instantiators. Allocate a new instantiated version of it.
-  const Type& instantiated_type = Type::Handle(
-      zone, Type::New(cls, type_arguments, token_pos(), nullability(), space));
+  const Type& instantiated_type =
+      Type::Handle(zone, Type::New(cls, type_arguments, token_pos(), space));
   // For a function type, possibly instantiate and set its signature.
   if (!sig_fun.IsNull()) {
     // If we are finalizing a typedef, do not yet instantiate its signature,
@@ -18020,8 +18009,10 @@ RawAbstractType* Type::Canonicalize(TrailPtr trail) const {
   const Class& cls = Class::Handle(zone, type_class());
 
   // Fast canonical lookup/registry for simple types.
-  if (!cls.IsGeneric() && !cls.IsClosureClass() && !cls.IsTypedefClass()) {
+  if (IsNullType() || (IsLegacy() && !cls.IsGeneric() &&
+                       !cls.IsClosureClass() && !cls.IsTypedefClass())) {
     ASSERT(!IsFunctionType());
+    ASSERT(!IsNullType() || IsNullable());
     Type& type = Type::Handle(zone, cls.declaration_type());
     if (type.IsNull()) {
       ASSERT(!cls.raw()->InVMIsolateHeap() || (isolate == Dart::vm_isolate()));
@@ -18054,12 +18045,10 @@ RawAbstractType* Type::Canonicalize(TrailPtr trail) const {
         }
       }
     }
-    if (nullability() == type.nullability()) {
-      ASSERT(this->Equals(type));
-      ASSERT(type.IsCanonical());
-      ASSERT(type.IsOld());
-      return type.raw();
-    }
+    ASSERT(this->Equals(type));
+    ASSERT(type.IsCanonical());
+    ASSERT(type.IsOld());
+    return type.raw();
   }
 
   AbstractType& type = Type::Handle(zone);
@@ -18147,13 +18136,12 @@ bool Type::CheckIsCanonical(Thread* thread) const {
   const Class& cls = Class::Handle(zone, type_class());
 
   // Fast canonical lookup/registry for simple types.
-  if (!cls.IsGeneric() && !cls.IsClosureClass() && !cls.IsTypedefClass()) {
+  if (IsNullType() || (IsLegacy() && !cls.IsGeneric() &&
+                       !cls.IsClosureClass() && !cls.IsTypedefClass())) {
     ASSERT(!IsFunctionType());
+    ASSERT(!IsNullType() || IsNullable());
     type = cls.declaration_type();
-    ASSERT(type.IsCanonical());
-    if (nullability() == type.nullability()) {
-      return (raw() == type.raw());
-    }
+    return (raw() == type.raw());
   }
 
   ObjectStore* object_store = isolate->object_store();
@@ -18246,7 +18234,6 @@ RawType* Type::New(Heap::Space space) {
 RawType* Type::New(const Class& clazz,
                    const TypeArguments& arguments,
                    TokenPosition token_pos,
-                   Nullability nullability,
                    Heap::Space space) {
   Zone* Z = Thread::Current()->zone();
   const Type& result = Type::Handle(Z, Type::New(space));
@@ -18255,7 +18242,11 @@ RawType* Type::New(const Class& clazz,
   result.SetHash(0);
   result.set_token_pos(token_pos);
   result.StoreNonPointer(&result.raw_ptr()->type_state_, RawType::kAllocated);
-  result.set_nullability(nullability);
+  if (clazz.id() == kNullCid) {
+    result.set_nullability(kNullable);
+  } else {
+    result.set_nullability(kLegacy);
+  }
 
   result.SetTypeTestingStub(
       Code::Handle(Z, TypeTestingStubGenerator::DefaultCodeForType(result)));

runtime/vm/object.h

@@ -837,21 +837,6 @@ typedef ZoneGrowableHandlePtrArray<const AbstractType>* TrailPtr;
 // The third string in the triplet is "print" if the triplet should be printed.
 typedef ZoneGrowableHandlePtrArray<const String> URIs;
 
-// Keep in sync with package:kernel/lib/ast.dart
-enum Nullability {
-  kUndetermined = 0,
-  kNullable = 1,
-  kNonNullable = 2,
-  kLegacy = 3,
-};
-
-// Nullability aware subtype checking modes.
-enum NNBDMode {
-  kUnaware,
-  kWeak,
-  kStrong,
-};
-
 class Class : public Object {
  public:
   enum InvocationDispatcherEntry {
@@ -950,11 +935,7 @@ class Class : public Object {
   // class B<T, S>
   // class C<R> extends B<R, int>
   // C.DeclarationType() --> C [R, int, R]
-  // The declaration type is legacy by default, but another nullability
-  // variant may be requested. The first requested type gets cached in the class
-  // and subsequent nullability variants get cached in the object store.
-  // TODO(regis): Is this caching still useful or should we eliminate it?
-  RawType* DeclarationType(Nullability nullability = kLegacy) const;
+  RawType* DeclarationType() const;
 
   static intptr_t declaration_type_offset() {
     return OFFSET_OF(RawClass, declaration_type_);
@@ -2149,6 +2130,21 @@ class ICData : public Object {
   friend class SnapshotWriter;
 };
 
+// Keep in sync with package:kernel/lib/ast.dart
+enum Nullability {
+  kUndetermined = 0,
+  kNullable = 1,
+  kNonNullable = 2,
+  kLegacy = 3,
+};
+
+// Nullability aware subtype checking modes.
+enum NNBDMode {
+  kUnaware,
+  kWeak,
+  kStrong,
+};
+
 // Often used constants for number of free function type parameters.
 enum {
   kNoneFree = 0,
@@ -7157,13 +7153,11 @@ class Type : public AbstractType {
   static RawType* DartTypeType();
 
   // The finalized type of the given non-parameterized class.
-  static RawType* NewNonParameterizedType(const Class& type_class,
-                                          Nullability nullability = kLegacy);
+  static RawType* NewNonParameterizedType(const Class& type_class);
 
   static RawType* New(const Class& clazz,
                       const TypeArguments& arguments,
                       TokenPosition token_pos,
-                      Nullability nullability = kLegacy,
                       Heap::Space space = Heap::kOld);
 
  private: