[vm] Clean up changes around internal-only class ids.

* IsInternalOnlyId -> IsInternalOnlyClassId

* Removes constexpr predicate ClassComesBeforeOtherInternalOnlyClasses()
  used in only two locations and inlines the returned condition.

* Change IsErrorClassId to check <= kUnwindErrorCid instead of
  <= kLastInternalOnlyCid.

TEST=Refactoring, so existing tests.

Change-Id: Ib7dfc89aba16d52733de05687f0ca7055c16e7a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/210126
Auto-Submit: Tess Strickland <sstrickl@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>

runtime/vm/class_id.h

@@ -248,7 +248,7 @@ const int kTypedDataCidRemainderExternal = 2;
 
 // Class Id predicates.
 
-bool IsInternalOnlyId(intptr_t index);
+bool IsInternalOnlyClassId(intptr_t index);
 bool IsErrorClassId(intptr_t index);
 bool IsNumberClassId(intptr_t index);
 bool IsIntegerClassId(intptr_t index);
@@ -276,18 +276,15 @@ constexpr intptr_t kLastInternalOnlyCid = kUnwindErrorCid;
 COMPILE_ASSERT(kFirstInternalOnlyCid == kObjectCid + 1);
 COMPILE_ASSERT(kInstanceCid == kLastInternalOnlyCid + 1);
 
-// Checks that the cids in CLASS_LIST_INTERNAL_ONLY come after kObjectCid.
-// Use with COMPILE_ASSERT where code assumes that Object immediately precedes
-// the other internal-only cids, so it can be adjusted if this changes.
-constexpr bool ObjectComesBeforeOtherInternalOnlyClasses() {
-  return kFirstInternalOnlyCid == kObjectCid + 1;
-}
-
 // Returns true for any class id that either does not correspond to a real
 // class, like kIllegalCid or kForwardingCorpse, or that is internal to the VM
 // and should not be exposed directly to user code.
-inline bool IsInternalOnlyId(intptr_t index) {
-  COMPILE_ASSERT(ObjectComesBeforeOtherInternalOnlyClasses());
+inline bool IsInternalOnlyClassId(intptr_t index) {
+  // Fix the condition below if these become non-contiguous.
+  COMPILE_ASSERT(kIllegalCid + 1 == kFreeListElement &&
+                 kIllegalCid + 2 == kForwardingCorpse &&
+                 kIllegalCid + 3 == kObjectCid &&
+                 kIllegalCid + 4 == kFirstInternalOnlyCid);
   return index <= kLastInternalOnlyCid;
 }
 
@@ -297,8 +294,9 @@ inline bool IsErrorClassId(intptr_t index) {
                  kLanguageErrorCid == kErrorCid + 2 &&
                  kUnhandledExceptionCid == kErrorCid + 3 &&
                  kUnwindErrorCid == kErrorCid + 4 &&
+                 // Change if needed for detecting a new error added at the end.
                  kLastInternalOnlyCid == kUnwindErrorCid);
-  return (index >= kErrorCid && index <= kLastInternalOnlyCid);
+  return (index >= kErrorCid && index <= kUnwindErrorCid);
 }
 
 inline bool IsNumberClassId(intptr_t index) {

runtime/vm/class_table.cc

@@ -39,7 +39,7 @@ SharedClassTable::SharedClassTable()
         calloc(capacity_, sizeof(RelaxedAtomic<intptr_t>)));
     // The following cids don't have a corresponding class object in Dart code.
     // We therefore need to initialize them eagerly.
-    COMPILE_ASSERT(ObjectComesBeforeOtherInternalOnlyClasses());
+    COMPILE_ASSERT(kFirstInternalOnlyCid == kObjectCid + 1);
     for (intptr_t i = kObjectCid; i <= kLastInternalOnlyCid; i++) {
       table[i] = vm_shared_class_table->SizeAt(i);
     }
@@ -106,7 +106,7 @@ ClassTable::ClassTable(SharedClassTable* shared_class_table)
         static_cast<ClassPtr*>(calloc(capacity_, sizeof(ClassPtr)));
     // The following cids don't have a corresponding class object in Dart code.
     // We therefore need to initialize them eagerly.
-    COMPILE_ASSERT(ObjectComesBeforeOtherInternalOnlyClasses());
+    COMPILE_ASSERT(kFirstInternalOnlyCid == kObjectCid + 1);
     for (intptr_t i = kObjectCid; i <= kLastInternalOnlyCid; i++) {
       table[i] = vm_class_table->At(i);
     }

runtime/vm/compiler/backend/type_propagator.cc

@@ -821,7 +821,7 @@ const AbstractType* CompileType::ToAbstractType() {
 
     // VM-internal objects don't have a compile-type. Return dynamic-type
     // in this case.
-    if (IsInternalOnlyId(cid_) || cid_ == kTypeArgumentsCid) {
+    if (IsInternalOnlyClassId(cid_) || cid_ == kTypeArgumentsCid) {
       type_ = &Object::dynamic_type();
       return type_;
     }

runtime/vm/compiler/cha.cc

@@ -32,7 +32,7 @@ bool CHA::IsGuardedClass(intptr_t cid) const {
 
 bool CHA::HasSubclasses(const Class& cls) {
   ASSERT(!cls.IsNull());
-  ASSERT(!IsInternalOnlyId(cls.id()));
+  ASSERT(!IsInternalOnlyClassId(cls.id()));
   // Can't track dependencies for classes on the VM heap since those are
   // read-only.
   // TODO(fschneider): Enable tracking of CHA dependent code for VM heap

runtime/vm/dart_api_impl.h

@@ -212,7 +212,7 @@ class Api : AllStatic {
 
   // Returns true if the handle holds a Dart Instance.
   static bool IsInstance(Dart_Handle handle) {
-    return !IsInternalOnlyId(ClassId(handle));
+    return !IsInternalOnlyClassId(ClassId(handle));
   }
 
   // Returns true if the handle is non-dangling.

runtime/vm/heap/verifier.cc

@@ -117,7 +117,7 @@ void VerifyCanonicalVisitor::VisitObject(ObjectPtr obj) {
   // Therefore we disable the handle verification here.
   const bool old_verify_flag = FLAG_verify_handles;
   FLAG_verify_handles = false;
-  if (!IsInternalOnlyId(obj->GetClassId()) &&
+  if (!IsInternalOnlyClassId(obj->GetClassId()) &&
       (obj->GetClassId() != kTypeArgumentsCid)) {
     if (obj->untag()->IsCanonical()) {
       instanceHandle_ ^= obj;

runtime/vm/object.cc

@@ -2953,7 +2953,7 @@ ClassPtr Class::New(IsolateGroup* isolate_group, bool register_class) {
   result.set_num_type_arguments_unsafe(0);
   result.set_num_native_fields(0);
   result.set_state_bits(0);
-  if (IsInternalOnlyId(FakeObject::kClassId) ||
+  if (IsInternalOnlyClassId(FakeObject::kClassId) ||
       (FakeObject::kClassId == kTypeArgumentsCid)) {
     // VM internal classes are done. There is no finalization needed or
     // possible in this case.
@@ -4466,7 +4466,7 @@ ObjectPtr Class::EvaluateCompiledExpression(
     const Array& arguments,
     const TypeArguments& type_arguments) const {
   ASSERT(Thread::Current()->IsMutatorThread());
-  if (IsInternalOnlyId(id()) || (id() == kTypeArgumentsCid)) {
+  if (IsInternalOnlyClassId(id()) || (id() == kTypeArgumentsCid)) {
     const Instance& exception = Instance::Handle(String::New(
         "Expressions can be evaluated only with regular Dart instances"));
     const Instance& stacktrace = Instance::Handle();

runtime/vm/object_graph.cc

@@ -24,7 +24,7 @@ namespace dart {
 static bool IsUserClass(intptr_t cid) {
   if (cid == kContextCid) return true;
   if (cid == kTypeArgumentsCid) return false;
-  return !IsInternalOnlyId(cid);
+  return !IsInternalOnlyClassId(cid);
 }
 
 // The state of a pre-order, depth-first traversal of an object graph.

runtime/vm/service.cc

@@ -2786,7 +2786,7 @@ static void BuildExpressionEvaluationScope(Thread* thread, JSONStream* js) {
         isStatic = false;
       }
       if (!cls.IsTopLevel() &&
-          (IsInternalOnlyId(cls.id()) || cls.id() == kTypeArgumentsCid)) {
+          (IsInternalOnlyClassId(cls.id()) || cls.id() == kTypeArgumentsCid)) {
         js->PrintError(
             kInvalidParams,
             "Expressions can be evaluated only with regular Dart instances");

runtime/vm/tagged_pointer.h

@@ -109,7 +109,7 @@ class ObjectPtr {
   bool IsStringInstance() const { return IsStringClassId(GetClassId()); }
   bool IsRawNull() const { return GetClassId() == kNullCid; }
   bool IsDartInstance() const {
-    return (!IsHeapObject() || !IsInternalOnlyId(GetClassId()));
+    return (!IsHeapObject() || !IsInternalOnlyClassId(GetClassId()));
   }
   bool IsFreeListElement() const {
     return ((GetClassId() == kFreeListElement));