Address some comments for class flags

Change-Id: I354f48aefc37ce92c4d02cfce1723db0e28907bf

5 files changed, 55 insertions, 26 deletions

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index b547d079cd..f17b2eb73a 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -570,12 +570,16 @@ void ClassLinker::InitWithoutImage(std::vector<std::unique_ptr<const DexFile>> b
   CHECK_EQ(java_lang_ref_Reference->GetClassSize(),
            mirror::Reference::ClassSize(image_pointer_size_));
   class_root = FindSystemClass(self, "Ljava/lang/ref/FinalizerReference;");
+  CHECK_EQ(class_root->GetClassFlags(), mirror::kClassFlagNormal);
   class_root->SetClassFlags(class_root->GetClassFlags() | mirror::kClassFlagFinalizerReference);
   class_root = FindSystemClass(self, "Ljava/lang/ref/PhantomReference;");
+  CHECK_EQ(class_root->GetClassFlags(), mirror::kClassFlagNormal);
   class_root->SetClassFlags(class_root->GetClassFlags() | mirror::kClassFlagPhantomReference);
   class_root = FindSystemClass(self, "Ljava/lang/ref/SoftReference;");
+  CHECK_EQ(class_root->GetClassFlags(), mirror::kClassFlagNormal);
   class_root->SetClassFlags(class_root->GetClassFlags() | mirror::kClassFlagSoftReference);
   class_root = FindSystemClass(self, "Ljava/lang/ref/WeakReference;");
+  CHECK_EQ(class_root->GetClassFlags(), mirror::kClassFlagNormal);
   class_root->SetClassFlags(class_root->GetClassFlags() | mirror::kClassFlagWeakReference);
 
   // Setup the ClassLoader, verifying the object_size_.
@@ -4387,8 +4391,9 @@ bool ClassLinker::LinkSuperClass(Handle<mirror::Class> klass) {
   }
 
   // Inherit reference flags (if any) from the superclass.
-  int reference_flags = (super->GetClassFlags() & mirror::kClassFlagReference);
+  uint32_t reference_flags = (super->GetClassFlags() & mirror::kClassFlagReference);
   if (reference_flags != 0) {
+    CHECK_EQ(klass->GetClassFlags(), 0u);
     klass->SetClassFlags(klass->GetClassFlags() | reference_flags);
   }
   // Disallow custom direct subclasses of java.lang.ref.Reference.
@@ -5232,17 +5237,26 @@ bool ClassLinker::LinkFields(Thread* self, Handle<mirror::Class> klass, bool is_
     mirror::Class* super_class = klass->GetSuperClass();
     if (num_reference_fields == 0 || super_class == nullptr) {
       // object has one reference field, klass, but we ignore it since we always visit the class.
-      // If the super_class is null then we are java.lang.Object.
+      // super_class is null iff the class is java.lang.Object.
       if (super_class == nullptr ||
           (super_class->GetClassFlags() & mirror::kClassFlagNoReferenceFields) != 0) {
         klass->SetClassFlags(klass->GetClassFlags() | mirror::kClassFlagNoReferenceFields);
-      } else if (kIsDebugBuild) {
-        size_t total_reference_instance_fields = 0;
-        while (super_class != nullptr) {
-          total_reference_instance_fields += super_class->NumReferenceInstanceFields();
-          super_class = super_class->GetSuperClass();
-        }
-        CHECK_GT(total_reference_instance_fields, 1u);
+      }
+    }
+    if (kIsDebugBuild) {
+      DCHECK_EQ(super_class == nullptr, klass->DescriptorEquals("Ljava/lang/Object;"));
+      size_t total_reference_instance_fields = 0;
+      mirror::Class* cur_super = klass.Get();
+      while (cur_super != nullptr) {
+        total_reference_instance_fields += cur_super->NumReferenceInstanceFieldsDuringLinking();
+        cur_super = cur_super->GetSuperClass();
+      }
+      if (super_class == nullptr) {
+        CHECK_EQ(total_reference_instance_fields, 1u) << PrettyDescriptor(klass.Get());
+      } else {
+        // Check that there is at least num_reference_fields other than Object.class.
+        CHECK_GE(total_reference_instance_fields, 1u + num_reference_fields)
+            << PrettyClass(klass.Get());
       }
     }
     if (!klass->IsVariableSize()) {
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index 3bea978974..ad268247bf 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -44,7 +44,7 @@ void Class::SetClassClass(Class* java_lang_Class) {
       << java_lang_Class_.Read()
       << " " << java_lang_Class;
   CHECK(java_lang_Class != nullptr);
-  java_lang_Class->SetClassFlags(java_lang_Class->GetClassFlags() | mirror::kClassFlagClass);
+  java_lang_Class->SetClassFlags(mirror::kClassFlagClass);
   java_lang_Class_ = GcRoot<Class>(java_lang_Class);
 }
 
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index cbcb517001..8709d459b6 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -239,15 +239,15 @@ class MANAGED Class FINAL : public Object {
   }
 
   ALWAYS_INLINE void SetStringClass() SHARED_REQUIRES(Locks::mutator_lock_) {
-    SetClassFlags(GetClassFlags() | kClassFlagString | kClassFlagNoReferenceFields);
+    SetClassFlags(kClassFlagString | kClassFlagNoReferenceFields);
   }
 
   ALWAYS_INLINE bool IsClassLoaderClass() SHARED_REQUIRES(Locks::mutator_lock_) {
-    return (GetClassFlags() & kClassFlagClassLoader) != 0;
+    return GetClassFlags() == kClassFlagClassLoader;
   }
 
   ALWAYS_INLINE void SetClassLoaderClass() SHARED_REQUIRES(Locks::mutator_lock_) {
-    SetClassFlags(GetClassFlags() | kClassFlagClassLoader);
+    SetClassFlags(kClassFlagClassLoader);
   }
 
   // Returns true if the class is abstract.
@@ -282,22 +282,22 @@ class MANAGED Class FINAL : public Object {
 
   template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
   bool IsWeakReferenceClass() SHARED_REQUIRES(Locks::mutator_lock_) {
-    return (GetClassFlags<kVerifyFlags>() & kClassFlagWeakReference) != 0;
+    return GetClassFlags<kVerifyFlags>() == kClassFlagWeakReference;
   }
 
   template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
   bool IsSoftReferenceClass() SHARED_REQUIRES(Locks::mutator_lock_) {
-    return (GetClassFlags<kVerifyFlags>() & kClassFlagSoftReference) != 0;
+    return GetClassFlags<kVerifyFlags>() == kClassFlagSoftReference;
   }
 
   template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
   bool IsFinalizerReferenceClass() SHARED_REQUIRES(Locks::mutator_lock_) {
-    return (GetClassFlags<kVerifyFlags>() & kClassFlagFinalizerReference) != 0;
+    return GetClassFlags<kVerifyFlags>() == kClassFlagFinalizerReference;
   }
 
   template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
   bool IsPhantomReferenceClass() SHARED_REQUIRES(Locks::mutator_lock_) {
-    return (GetClassFlags<kVerifyFlags>() & kClassFlagPhantomReference) != 0;
+    return GetClassFlags<kVerifyFlags>() == kClassFlagPhantomReference;
   }
 
   // Can references of this type be assigned to by things of another type? For non-array types
@@ -867,8 +867,8 @@ class MANAGED Class FINAL : public Object {
   uint32_t NumInstanceFields() SHARED_REQUIRES(Locks::mutator_lock_);
   ArtField* GetInstanceField(uint32_t i) SHARED_REQUIRES(Locks::mutator_lock_);
 
-  // Returns the number of instance fields containing reference types not counting fields in the
-  // super class.
+  // Returns the number of instance fields containing reference types. Does not count fields in any
+  // super classes.
   uint32_t NumReferenceInstanceFields() SHARED_REQUIRES(Locks::mutator_lock_) {
     DCHECK(IsResolved() || IsErroneous());
     return GetField32(OFFSET_OF_OBJECT_MEMBER(Class, num_reference_instance_fields_));
diff --git a/runtime/mirror/class_flags.h b/runtime/mirror/class_flags.h
index 6c15639177..eb2e2ebf81 100644
--- a/runtime/mirror/class_flags.h
+++ b/runtime/mirror/class_flags.h
@@ -22,27 +22,39 @@
 namespace art {
 namespace mirror {
 
-// Object types stored in class to help GC with faster object marking.
+// Normal instance with at least one ref field other than the class.
 static constexpr uint32_t kClassFlagNormal             = 0x00000000;
+
 // Only normal objects which have no reference fields, e.g. string or primitive array or normal
-// class instance.
+// class instance with no fields other than klass.
 static constexpr uint32_t kClassFlagNoReferenceFields  = 0x00000001;
+
+// Class is java.lang.String.class.
 static constexpr uint32_t kClassFlagString             = 0x00000004;
+
+// Class is an object array class.
 static constexpr uint32_t kClassFlagObjectArray        = 0x00000008;
+
+// Class is java.lang.Class.class.
 static constexpr uint32_t kClassFlagClass              = 0x00000010;
 
-// class is ClassLoader or one of its subclasses
+// Class is ClassLoader or one of its subclasses.
 static constexpr uint32_t kClassFlagClassLoader        = 0x00000020;
 
-// class is a soft/weak/phantom ref
+// Class is a soft/weak/phantom class.
 static constexpr uint32_t kClassFlagSoftReference      = 0x00000040;
-// class is a weak reference
+
+// Class is a weak reference class.
 static constexpr uint32_t kClassFlagWeakReference      = 0x00000080;
-// class is a finalizer reference
+
+// Class is a finalizer reference class.
 static constexpr uint32_t kClassFlagFinalizerReference = 0x00000100;
-// class is a phantom reference
+
+// Class is the phantom reference class.
 static constexpr uint32_t kClassFlagPhantomReference   = 0x00000200;
 
+// Combination of flags to figure out if the class is either the weak/soft/phantom/finalizer
+// reference class.
 static constexpr uint32_t kClassFlagReference =
     kClassFlagSoftReference |
     kClassFlagWeakReference |
diff --git a/runtime/mirror/object-inl.h b/runtime/mirror/object-inl.h
index 702a0f49ea..e35ddccd0d 100644
--- a/runtime/mirror/object-inl.h
+++ b/runtime/mirror/object-inl.h
@@ -1016,6 +1016,9 @@ inline void Object::VisitReferences(const Visitor& visitor,
     DCHECK(!klass->IsVariableSize());
     VisitInstanceFieldsReferences(klass, visitor);
     DCHECK(!klass->IsClassClass());
+    DCHECK(!klass->IsStringClass());
+    DCHECK(!klass->IsClassLoaderClass());
+    DCHECK(!klass->IsArrayClass());
   } else {
     if ((class_flags & kClassFlagNoReferenceFields) == 0) {
       DCHECK(!klass->IsStringClass());