Skip to content

Commit

Permalink
[VM/nnbd] Print nullability of types by default.
Browse files Browse the repository at this point in the history 
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 <regis@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
  • Loading branch information
@crelier
crelier authored and commit-bot@chromium.org committed Feb 20, 2020
1 parent dae3084 commit fd20c7b
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 43 deletions.
2 changes: 1 addition & 1 deletion runtime/vm/dart_api_impl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/debugger.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'.
Expand Down
84 changes: 53 additions & 31 deletions runtime/vm/object.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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();
}
Expand All @@ -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());
Expand All @@ -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;
}
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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() ? "<null>" : 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);
}
}

Expand Down Expand Up @@ -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());
Expand Down
18 changes: 11 additions & 7 deletions runtime/vm/object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. "<T, dynamic, List<T>, 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 {
Expand Down Expand Up @@ -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. "<T, dynamic, List<T>, 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;
Expand Down Expand Up @@ -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;

Expand Down
12 changes: 9 additions & 3 deletions tests/language_2/type_object/runtime_type_function_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -83,8 +83,8 @@ main() {
check(fn('String', 'String', {'a': 'String', 'b': 'dynamic'}), Xyzzy.nam);

// Instance method tear-offs.
check(fn('void', 'Object'), <String>[].add);
check(fn('void', 'Object'), <int>[].add);
check(fn('void', 'Object'), new MyList<String>().add);
check(fn('void', 'Object'), new MyList<int>().add);
check(fn('void', 'int'), new Xyzzy().intAdd);

check(fn('String', 'Object'), new G<String, int>().foo);
Expand Down Expand Up @@ -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<E> {
void add(E value) {}
}

class G<U, V> {
U foo(V x) => null;
U moo(V f(U x)) => null;
Expand Down

0 comments on commit fd20c7b

Please sign in to comment.