From d71b1e5b04060230d8ef1eb4033fc65ee1163bc3 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Fri, 7 Feb 2020 23:47:50 +0000 Subject: [PATCH] [vm] Remove 'dynamic' from 'dart:core'; add missing accessors to the embedding API for dynamic, void and Never. Fix crash when create type errors involving Never. Bug: https://github.com/dart-lang/sdk/issues/12478 Bug: https://github.com/dart-lang/sdk/issues/40176 Change-Id: I8cff7042850717d02da36ad935cf0a49ace80c3d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134886 Commit-Queue: Ryan Macnak Reviewed-by: Ben Konyi --- CHANGELOG.md | 3 ++ runtime/include/dart_api.h | 14 +++++- runtime/lib/mirrors.cc | 22 ++++---- runtime/vm/compiler/aot/precompiler.cc | 18 ------- runtime/vm/compiler/jit/compiler.cc | 12 ----- runtime/vm/dart_api_impl.cc | 30 +++++++++-- runtime/vm/dart_api_impl_test.cc | 69 ++++++++++++++++++++++---- runtime/vm/object.cc | 13 ++--- runtime/vm/program_visitor.cc | 7 --- 9 files changed, 117 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75921f83774..57ec411b1b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,9 @@ used (see Issue [39627][]). ### Dart VM +* Added `Dart_TypeDynamic`, `Dart_TypeVoid` and `Dart_TypeNever`. Type dynamic + can no longer by reached by `Dart_GetType(dart:core, dynamic)`. + ### Tools #### Dart Dev Compiler (DDC) diff --git a/runtime/include/dart_api.h b/runtime/include/dart_api.h index c736f393f19..c68de7b976c 100644 --- a/runtime/include/dart_api.h +++ b/runtime/include/dart_api.h @@ -1487,6 +1487,11 @@ DART_EXPORT uint8_t* Dart_ScopeAllocate(intptr_t size); */ DART_EXPORT Dart_Handle Dart_Null(); +/** + * Is this object null? + */ +DART_EXPORT bool Dart_IsNull(Dart_Handle object); + /** * Returns the empty string object. * @@ -1495,9 +1500,14 @@ DART_EXPORT Dart_Handle Dart_Null(); DART_EXPORT Dart_Handle Dart_EmptyString(); /** - * Is this object null? + * Returns types that are not classes, and which therefore cannot be looked up + * as library members by Dart_GetType. + * + * \return A handle to the dynamic, void or Never type. */ -DART_EXPORT bool Dart_IsNull(Dart_Handle object); +DART_EXPORT Dart_Handle Dart_TypeDynamic(); +DART_EXPORT Dart_Handle Dart_TypeVoid(); +DART_EXPORT Dart_Handle Dart_TypeNever(); /** * Checks if the two objects are equal. diff --git a/runtime/lib/mirrors.cc b/runtime/lib/mirrors.cc index 3f94c60d4c8..d2b1672d639 100644 --- a/runtime/lib/mirrors.cc +++ b/runtime/lib/mirrors.cc @@ -1179,18 +1179,18 @@ DEFINE_NATIVE_ENTRY(LibraryMirror_members, 0, 2) { entry = entries.GetNext(); if (entry.IsClass()) { const Class& klass = Class::Cast(entry); - ASSERT(!klass.IsVoidClass() && !klass.IsNeverClass()); - if (!klass.IsDynamicClass()) { - error = klass.EnsureIsFinalized(thread); - if (!error.IsNull()) { - Exceptions::PropagateError(error); - } - type = klass.DeclarationType(); - member_mirror = CreateClassMirror(klass, type, - Bool::True(), // is_declaration - owner_mirror); - member_mirrors.Add(member_mirror); + ASSERT(!klass.IsDynamicClass()); + ASSERT(!klass.IsVoidClass()); + ASSERT(!klass.IsNeverClass()); + error = klass.EnsureIsFinalized(thread); + if (!error.IsNull()) { + Exceptions::PropagateError(error); } + type = klass.DeclarationType(); + member_mirror = CreateClassMirror(klass, type, + Bool::True(), // is_declaration + owner_mirror); + member_mirrors.Add(member_mirror); } else if (entry.IsField()) { const Field& field = Field::Cast(entry); if (field.is_reflectable()) { diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc index a2718c24c28..7dffac58ba9 100644 --- a/runtime/vm/compiler/aot/precompiler.cc +++ b/runtime/vm/compiler/aot/precompiler.cc @@ -1378,9 +1378,6 @@ void Precompiler::CollectDynamicFunctionNames() { ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate); while (it.HasNext()) { cls = it.GetNextClass(); - if (cls.IsDynamicClass()) { - continue; // class 'dynamic' is in the read-only VM isolate. - } functions = cls.functions(); for (intptr_t j = 0; j < functions.Length(); j++) { function ^= functions.At(j); @@ -1458,10 +1455,6 @@ void Precompiler::TraceForRetainedFunctions() { ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate); while (it.HasNext()) { cls = it.GetNextClass(); - if (cls.IsDynamicClass()) { - continue; // class 'dynamic' is in the read-only VM isolate. - } - functions = cls.functions(); for (intptr_t j = 0; j < functions.Length(); j++) { function ^= functions.At(j); @@ -1517,10 +1510,6 @@ void Precompiler::DropFunctions() { ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate); while (it.HasNext()) { cls = it.GetNextClass(); - if (cls.IsDynamicClass()) { - continue; // class 'dynamic' is in the read-only VM isolate. - } - functions = cls.functions(); retained_functions = GrowableObjectArray::New(); for (intptr_t j = 0; j < functions.Length(); j++) { @@ -1581,10 +1570,6 @@ void Precompiler::DropFields() { ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate); while (it.HasNext()) { cls = it.GetNextClass(); - if (cls.IsDynamicClass()) { - continue; // class 'dynamic' is in the read-only VM isolate. - } - fields = cls.fields(); retained_fields = GrowableObjectArray::New(); for (intptr_t j = 0; j < fields.Length(); j++) { @@ -1780,9 +1765,6 @@ void Precompiler::TraceTypesFromRetainedClasses() { ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate); while (it.HasNext()) { cls = it.GetNextClass(); - if (cls.IsDynamicClass()) { - continue; // class 'dynamic' is in the read-only VM isolate. - } // The subclasses/implementors array is only needed for CHA. cls.ClearDirectSubclasses(); diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc index 1a8d269326b..998f0d6e264 100644 --- a/runtime/vm/compiler/jit/compiler.cc +++ b/runtime/vm/compiler/jit/compiler.cc @@ -1016,12 +1016,6 @@ RawError* Compiler::CompileAllFunctions(const Class& cls) { Object& result = Object::Handle(zone); Array& functions = Array::Handle(zone, cls.functions()); Function& func = Function::Handle(zone); - // Class dynamic lives in the vm isolate. Its array fields cannot be set to - // an empty array. - if (functions.IsNull()) { - ASSERT(cls.IsDynamicClass()); - return Error::null(); - } // Compile all the regular functions. for (int i = 0; i < functions.Length(); i++) { func ^= functions.At(i); @@ -1046,12 +1040,6 @@ RawError* Compiler::ReadAllBytecode(const Class& cls) { ASSERT(error.IsNull()); Array& functions = Array::Handle(zone, cls.functions()); Function& func = Function::Handle(zone); - // Class dynamic lives in the vm isolate. Its array fields cannot be set to - // an empty array. - if (functions.IsNull()) { - ASSERT(cls.IsDynamicClass()); - return Error::null(); - } // Compile all the regular functions. for (int i = 0; i < functions.Length(); i++) { func ^= functions.At(i); diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index aa6e4db83a3..f5c85804cb7 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -1981,14 +1981,35 @@ DART_EXPORT Dart_Handle Dart_Null() { return Api::Null(); } +DART_EXPORT bool Dart_IsNull(Dart_Handle object) { + TransitionNativeToVM transition(Thread::Current()); + return Api::UnwrapHandle(object) == Object::null(); +} + DART_EXPORT Dart_Handle Dart_EmptyString() { ASSERT(Isolate::Current() != NULL); return Api::EmptyString(); } -DART_EXPORT bool Dart_IsNull(Dart_Handle object) { - TransitionNativeToVM transition(Thread::Current()); - return Api::UnwrapHandle(object) == Object::null(); +DART_EXPORT Dart_Handle Dart_TypeDynamic() { + DARTSCOPE(Thread::Current()); + CHECK_CALLBACK_STATE(T); + API_TIMELINE_DURATION(T); + return Api::NewHandle(T, Type::DynamicType()); +} + +DART_EXPORT Dart_Handle Dart_TypeVoid() { + DARTSCOPE(Thread::Current()); + CHECK_CALLBACK_STATE(T); + API_TIMELINE_DURATION(T); + return Api::NewHandle(T, Type::VoidType()); +} + +DART_EXPORT Dart_Handle Dart_TypeNever() { + DARTSCOPE(Thread::Current()); + CHECK_CALLBACK_STATE(T); + API_TIMELINE_DURATION(T); + return Api::NewHandle(T, Type::NeverType()); } DART_EXPORT Dart_Handle Dart_ObjectEquals(Dart_Handle obj1, @@ -2268,7 +2289,8 @@ DART_EXPORT Dart_Handle Dart_FunctionOwner(Dart_Handle function) { #if defined(DEBUG) const Library& lib = Library::Handle(Z, owner.library()); if (lib.IsNull()) { - ASSERT(owner.IsDynamicClass() || owner.IsVoidClass()); + ASSERT(owner.IsDynamicClass() || owner.IsVoidClass() || + owner.IsNeverClass()); } #endif return Api::NewHandle(T, owner.library()); diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index 218778c4a10..f6860f29d42 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -798,6 +798,46 @@ TEST_CASE(DartAPI_EmptyString) { Dart_Handle empty = Dart_EmptyString(); EXPECT_VALID(empty); EXPECT(!Dart_IsNull(empty)); + EXPECT(Dart_IsString(empty)); + intptr_t length = -1; + EXPECT_VALID(Dart_StringLength(empty, &length)); + EXPECT_EQ(0, length); +} + +TEST_CASE(DartAPI_TypeDynamic) { + Dart_Handle type = Dart_TypeDynamic(); + EXPECT_VALID(type); + EXPECT(Dart_IsType(type)); + + Dart_Handle str = Dart_ToString(type); + EXPECT_VALID(str); + const char* cstr = nullptr; + EXPECT_VALID(Dart_StringToCString(str, &cstr)); + EXPECT_STREQ("dynamic", cstr); +} + +TEST_CASE(DartAPI_TypeVoid) { + Dart_Handle type = Dart_TypeVoid(); + EXPECT_VALID(type); + EXPECT(Dart_IsType(type)); + + Dart_Handle str = Dart_ToString(type); + EXPECT_VALID(str); + const char* cstr = nullptr; + EXPECT_VALID(Dart_StringToCString(str, &cstr)); + EXPECT_STREQ("void", cstr); +} + +TEST_CASE(DartAPI_TypeNever) { + Dart_Handle type = Dart_TypeNever(); + EXPECT_VALID(type); + EXPECT(Dart_IsType(type)); + + Dart_Handle str = Dart_ToString(type); + EXPECT_VALID(str); + const char* cstr = nullptr; + EXPECT_VALID(Dart_StringToCString(str, &cstr)); + EXPECT_STREQ("Never", cstr); } TEST_CASE(DartAPI_IdentityEquals) { @@ -1142,11 +1182,6 @@ TEST_CASE(DartAPI_ClassLibrary) { const char* str = NULL; Dart_StringToCString(lib_url, &str); EXPECT_STREQ("dart:core", str); - - // Case with no library. - type = Dart_GetType(lib, NewString("dynamic"), 0, NULL); - EXPECT_VALID(type); - EXPECT(Dart_IsNull(Dart_ClassLibrary(type))); } TEST_CASE(DartAPI_BooleanValues) { @@ -5051,7 +5086,9 @@ TEST_CASE(DartAPI_NewListOfType) { " ChannelReadResult(this.handles);\n" "}\n" "void expectListOfString(List _) {}\n" - "void expectListOfDynamic(List _) {}\n"; + "void expectListOfDynamic(List _) {}\n" + "void expectListOfVoid(List _) {}\n" + "void expectListOfNever(List _) {}\n"; Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL); Dart_Handle zxhandle_type = Dart_GetType(lib, NewString("ZXHandle"), 0, NULL); @@ -5093,14 +5130,28 @@ TEST_CASE(DartAPI_NewListOfType) { EXPECT_VALID( Dart_Invoke(lib, NewString("expectListOfString"), kNumArgs, args)); - Dart_Handle dynamic_type = - Dart_GetType(dart_core, NewString("dynamic"), 0, NULL); + Dart_Handle dynamic_type = Dart_TypeDynamic(); EXPECT_VALID(dynamic_type); - Dart_Handle dynamic_list = Dart_NewListOfType(string_type, 0); + Dart_Handle dynamic_list = Dart_NewListOfType(dynamic_type, 0); EXPECT_VALID(dynamic_list); args[0] = dynamic_list; EXPECT_VALID( Dart_Invoke(lib, NewString("expectListOfDynamic"), kNumArgs, args)); + + Dart_Handle void_type = Dart_TypeVoid(); + EXPECT_VALID(void_type); + Dart_Handle void_list = Dart_NewListOfType(void_type, 0); + EXPECT_VALID(void_list); + args[0] = void_list; + EXPECT_VALID(Dart_Invoke(lib, NewString("expectListOfVoid"), kNumArgs, args)); + + Dart_Handle never_type = Dart_TypeNever(); + EXPECT_VALID(never_type); + Dart_Handle never_list = Dart_NewListOfType(never_type, 0); + EXPECT_VALID(never_list); + args[0] = never_list; + EXPECT_VALID( + Dart_Invoke(lib, NewString("expectListOfNever"), kNumArgs, args)); } static Dart_Handle PrivateLibName(Dart_Handle lib, const char* str) { diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index b92cae71c4f..aa260d738b7 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -11832,12 +11832,6 @@ void Library::InitCoreLibrary(Isolate* isolate) { core_lib.Register(thread); isolate->object_store()->set_bootstrap_library(ObjectStore::kCore, core_lib); isolate->object_store()->set_root_library(Library::Handle()); - - // Hook up predefined classes without setting their library pointers. These - // classes are coming from the VM isolate, and are shared between multiple - // isolates so setting their library pointers would be wrong. - const Class& cls = Class::Handle(zone, Object::dynamic_class()); - core_lib.AddObject(cls, String::Handle(zone, cls.Name())); } // Invoke the function, or noSuchMethod if it is null. @@ -19210,7 +19204,9 @@ bool Type::CheckIsCanonical(Thread* thread) const { #endif // DEBUG void Type::EnumerateURIs(URIs* uris) const { - if (IsDynamicType() || IsVoidType()) { + // N.B. Not all types with kNeverCid answer true to IsNeverType, but none of + // them have a URI. + if (IsDynamicType() || IsVoidType() || (type_class_id() == kNeverCid)) { return; } Thread* thread = Thread::Current(); @@ -19451,7 +19447,8 @@ void TypeRef::EnumerateURIs(URIs* uris) const { Thread* thread = Thread::Current(); Zone* zone = thread->zone(); const AbstractType& ref_type = AbstractType::Handle(zone, type()); - ASSERT(!ref_type.IsDynamicType() && !ref_type.IsVoidType()); + ASSERT(!ref_type.IsDynamicType() && !ref_type.IsVoidType() && + !ref_type.IsNeverType()); const Class& cls = Class::Handle(zone, ref_type.type_class()); const String& name = String::Handle(zone, cls.UserVisibleName()); const Library& library = Library::Handle(zone, cls.library()); diff --git a/runtime/vm/program_visitor.cc b/runtime/vm/program_visitor.cc index 79b4c21985f..00a2772d1e3 100644 --- a/runtime/vm/program_visitor.cc +++ b/runtime/vm/program_visitor.cc @@ -29,9 +29,6 @@ void ProgramVisitor::VisitClasses(ClassVisitor* visitor) { ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate); while (it.HasNext()) { cls = it.GetNextClass(); - if (cls.IsDynamicClass()) { - continue; // class 'dynamic' is in the read-only VM isolate. - } visitor->Visit(cls); } patches = lib.used_scripts(); @@ -55,10 +52,6 @@ class ClassFunctionVisitor : public ClassVisitor { field_(Field::Handle(zone)) {} void Visit(const Class& cls) { - if (cls.IsDynamicClass()) { - return; // class 'dynamic' is in the read-only VM isolate. - } - functions_ = cls.functions(); for (intptr_t j = 0; j < functions_.Length(); j++) { function_ ^= functions_.At(j);