From fd20c7b92bb8956c78c8c7bed7a6be6675a02ed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Crelier?= Date: Thu, 20 Feb 2020 21:43:25 +0000 Subject: [PATCH] [VM/nnbd] Print nullability of types by default. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only '?' is shown by default. To print '*' and '%', use debugging flag --show-internal-names. Fix expectations in tests. Change-Id: Idc60fbfb4d477602eb0c713f4bdcc1e573a3328a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135790 Commit-Queue: Régis Crelier Reviewed-by: Alexander Markov Reviewed-by: Liam Appelbe --- runtime/vm/dart_api_impl_test.cc | 2 +- runtime/vm/debugger.cc | 2 +- runtime/vm/object.cc | 84 ++++++++++++------- runtime/vm/object.h | 18 ++-- .../runtime_type_function_test.dart | 12 ++- 5 files changed, 75 insertions(+), 43 deletions(-) diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index 512027867efbe..60ce652412d43 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -6086,7 +6086,7 @@ TEST_CASE(DartAPI_GetNullableType) { EXPECT_VALID(name); const char* name_cstr = ""; EXPECT_VALID(Dart_StringToCString(name, &name_cstr)); - EXPECT_STREQ("Class", name_cstr); + EXPECT_STREQ("Class?", name_cstr); name = Dart_GetField(type, NewString("name")); EXPECT_VALID(name); diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc index 75f0f40906dbf..2550363017a55 100644 --- a/runtime/vm/debugger.cc +++ b/runtime/vm/debugger.cc @@ -1542,7 +1542,7 @@ RawTypeArguments* ActivationFrame::BuildParameters( mapping_offset -= size; for (intptr_t j = 0; j < size; ++j) { type_param = TypeParameter::RawCast(type_params.TypeAt(j)); - name = type_param.Name(); + name = type_param.name(); // Write the names in backwards in terms of chain of functions. // But keep the order of names within the same function. so they // match up with the order of the types in 'type_arguments'. diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 50c1907b9d5bb..8ea6895084920 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -73,9 +73,8 @@ DEFINE_FLAG( 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\")"); -// TODO(regis): Remove this temporary flag used to debug nullability. -DEFINE_FLAG(bool, show_nullability, false, "Show nullability in type names"); + "instead of showing the corresponding interface names (e.g. \"String\"). " + "Also show legacy nullability in type names."); DEFINE_FLAG(bool, use_lib_cache, false, "Use library name cache"); DEFINE_FLAG(bool, use_exp_cache, false, "Use library exported name cache"); @@ -5664,7 +5663,11 @@ void TypeArguments::PrintSubvectorName(intptr_t from_index, for (intptr_t i = 0; i < len; i++) { if (from_index + i < Length()) { type = TypeAt(from_index + i); - type.PrintName(name_visibility, printer); + if (type.IsNull()) { + printer->AddString("null"); // Unfinalized vector. + } else { + type.PrintName(name_visibility, printer); + } } else { printer->AddString("dynamic"); } @@ -18084,17 +18087,26 @@ RawString* AbstractType::PrintURIs(URIs* uris) { return Symbols::FromConcatAll(thread, pieces); } -static const char* NullabilitySuffix(Nullability value) { +const char* AbstractType::NullabilitySuffix( + NameVisibility name_visibility) const { + if (IsDynamicType() || IsVoidType() || IsNullType()) { + // Hide nullable suffix. + return ""; + } // Keep in sync with Nullability enum in runtime/vm/object.h. - switch (value) { + switch (nullability()) { case Nullability::kUndetermined: - return "%"; + return (FLAG_show_internal_names || name_visibility == kInternalName) + ? "%" + : ""; case Nullability::kNullable: return "?"; case Nullability::kNonNullable: return ""; case Nullability::kLegacy: - return "*"; + return (FLAG_show_internal_names || name_visibility == kInternalName) + ? "*" + : ""; default: UNREACHABLE(); } @@ -18121,9 +18133,7 @@ void AbstractType::PrintName(NameVisibility name_visibility, Zone* zone = thread->zone(); if (IsTypeParameter()) { printer->AddString(String::Handle(zone, TypeParameter::Cast(*this).name())); - if (FLAG_show_nullability) { - printer->AddString(NullabilitySuffix(nullability())); - } + printer->AddString(NullabilitySuffix(name_visibility)); return; } const TypeArguments& args = TypeArguments::Handle(zone, arguments()); @@ -18136,9 +18146,14 @@ void AbstractType::PrintName(NameVisibility name_visibility, const Function& signature_function = Function::Handle(zone, Type::Cast(*this).signature()); if (!cls.IsTypedefClass()) { - signature_function.PrintSignature(kUserVisibleName, printer); - if (FLAG_show_nullability) { - printer->AddString(NullabilitySuffix(nullability())); + const char* suffix = NullabilitySuffix(name_visibility); + if (suffix[0] != '\0') { + printer->AddString("("); + } + signature_function.PrintSignature(name_visibility, printer); + if (suffix[0] != '\0') { + printer->AddString(")"); + printer->AddString(suffix); } return; } @@ -18148,9 +18163,7 @@ void AbstractType::PrintName(NameVisibility name_visibility, if (!IsFinalized() || IsBeingFinalized()) { // TODO(regis): Check if this is dead code. printer->AddString(class_name); - if (FLAG_show_nullability) { - printer->AddString(NullabilitySuffix(nullability())); - } + printer->AddString(NullabilitySuffix(name_visibility)); return; } // Print the name of a typedef as a regular, possibly parameterized, class. @@ -18188,9 +18201,7 @@ void AbstractType::PrintName(NameVisibility name_visibility, args.PrintSubvectorName(first_type_param_index, num_type_params, name_visibility, printer); } - if (FLAG_show_nullability) { - printer->AddString(NullabilitySuffix(nullability())); - } + printer->AddString(NullabilitySuffix(name_visibility)); // The name is only used for type checking and debugging purposes. // Unless profiling data shows otherwise, it is not worth caching the name in // the type. @@ -19251,32 +19262,42 @@ const char* Type::ToCString() const { return "Type: null"; } Zone* zone = Thread::Current()->zone(); + ZoneTextBuffer args(zone); const TypeArguments& type_args = TypeArguments::Handle(zone, arguments()); - const char* args_cstr = type_args.IsNull() ? "null" : type_args.ToCString(); + const char* args_cstr = ""; + if (!type_args.IsNull()) { + type_args.PrintSubvectorName(0, type_args.Length(), kInternalName, &args); + args_cstr = args.buffer(); + } const Class& cls = Class::Handle(zone, type_class()); const char* class_name; const String& name = String::Handle(zone, cls.Name()); class_name = name.IsNull() ? "" : name.ToCString(); + const char* suffix = NullabilitySuffix(kInternalName); if (IsFunctionType()) { const Function& sig_fun = Function::Handle(zone, signature()); ZoneTextBuffer sig(zone); + if (suffix[0] != '\0') { + sig.AddString("("); + } sig_fun.PrintSignature(kInternalName, &sig); + if (suffix[0] != '\0') { + sig.AddString(")"); + sig.AddString(suffix); + } if (cls.IsClosureClass()) { ASSERT(type_args.IsNull()); return OS::SCreate(zone, "Function Type: %s", sig.buffer()); } - return OS::SCreate(zone, "Function Type: %s (class: %s, args: %s)", - sig.buffer(), class_name, args_cstr); + return OS::SCreate(zone, "Function Type: %s (%s%s%s)", sig.buffer(), + class_name, args_cstr, suffix); } - if (type_args.IsNull()) { - return OS::SCreate(zone, "Type: class '%s'", class_name); - } else if (IsFinalized() && IsRecursive()) { + if (IsFinalized() && IsRecursive()) { const intptr_t hash = Hash(); - return OS::SCreate(zone, "Type: (H%" Px ") class '%s', args:[%s]", hash, - class_name, args_cstr); + return OS::SCreate(zone, "Type: (H%" Px ") %s%s%s", hash, class_name, + args_cstr, suffix); } else { - return OS::SCreate(zone, "Type: class '%s', args:[%s]", class_name, - args_cstr); + return OS::SCreate(zone, "Type: %s%s%s", class_name, args_cstr, suffix); } } @@ -19713,7 +19734,8 @@ const char* TypeParameter::ToCString() const { Thread* thread = Thread::Current(); ZoneTextBuffer printer(thread->zone()); printer.Printf("TypeParameter: name "); - printer.AddString(String::Handle(Name())); + printer.AddString(String::Handle(name())); + printer.AddString(NullabilitySuffix(kInternalName)); printer.Printf("; index: %" Pd ";", index()); if (IsFunctionTypeParameter()) { const Function& function = Function::Handle(parameterized_function()); diff --git a/runtime/vm/object.h b/runtime/vm/object.h index b7d29f8242a9e..8857ae4707beb 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -7051,6 +7051,13 @@ class TypeArguments : public Instance { // Names of internal classes are mapped to their public interfaces. RawString* UserVisibleName() const; + // Print the internal or public name of a subvector of this type argument + // vector, e.g. ", int>". + void PrintSubvectorName(intptr_t from_index, + intptr_t len, + NameVisibility name_visibility, + ZoneTextBuffer* printer) const; + // Check if the subvector of length 'len' starting at 'from_index' of this // type argument vector consists solely of DynamicType. bool IsRaw(intptr_t from_index, intptr_t len) const { @@ -7227,13 +7234,6 @@ class TypeArguments : public Instance { intptr_t from_index, intptr_t len) const; - // Return the internal or public name of a subvector of this type argument - // vector, e.g. ", int>". - void PrintSubvectorName(intptr_t from_index, - intptr_t len, - NameVisibility name_visibility, - ZoneTextBuffer* printer) const; - RawArray* instantiations() const; void set_instantiations(const Array& value) const; RawAbstractType* const* TypeAddr(intptr_t index) const; @@ -7364,6 +7364,10 @@ class AbstractType : public Instance { // Return a formatted string of the uris. static RawString* PrintURIs(URIs* uris); + // Returns a C-String (possibly "") representing the nullability of this type. + // Legacy and undetermined suffixes are only displayed with kInternalName. + virtual const char* NullabilitySuffix(NameVisibility name_visibility) const; + // The name of this type, including the names of its type arguments, if any. virtual RawString* Name() const; diff --git a/tests/language_2/type_object/runtime_type_function_test.dart b/tests/language_2/type_object/runtime_type_function_test.dart index 78d542297dfa2..b1c30c69aef88 100644 --- a/tests/language_2/type_object/runtime_type_function_test.dart +++ b/tests/language_2/type_object/runtime_type_function_test.dart @@ -68,7 +68,7 @@ main() { check(fn('dynamic', ''), main); // Top-level tear-off. check(fn('void', ''), Xyzzy.foo); // Class static member tear-off. - check(fn('void', 'Object'), [].add); // Instance tear-off. + check(fn('void', 'Object'), new MyList().add); // Instance tear-off. check(fn('int', ''), () => 1); // closure. var s = new Xyzzy().runtimeType.toString(); @@ -83,8 +83,8 @@ main() { check(fn('String', 'String', {'a': 'String', 'b': 'dynamic'}), Xyzzy.nam); // Instance method tear-offs. - check(fn('void', 'Object'), [].add); - check(fn('void', 'Object'), [].add); + check(fn('void', 'Object'), new MyList().add); + check(fn('void', 'Object'), new MyList().add); check(fn('void', 'int'), new Xyzzy().intAdd); check(fn('String', 'Object'), new G().foo); @@ -122,6 +122,12 @@ class Xyzzy { void intAdd(int x) {} } +// Using 'MyList' instead of core lib 'List' keeps covariant parameter type of +// tear-offs 'Object' (legacy lib) instead of 'Object?' (opted-in lib). +class MyList { + void add(E value) {} +} + class G { U foo(V x) => null; U moo(V f(U x)) => null;