Skip to content

Commit

Permalink
[VM/NNBD] - Make the null safety flag be isolate specific.
Browse files Browse the repository at this point in the history
Change-Id: I73a1e8a22db770ca14af6d53707a335bbbcdabcb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/139029
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
  • Loading branch information
a-siva authored and commit-bot@chromium.org committed Mar 20, 2020
1 parent b589647 commit 1bab476
Show file tree
Hide file tree
Showing 13 changed files with 82 additions and 45 deletions.
5 changes: 4 additions & 1 deletion runtime/bin/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,10 @@ gen_snapshot_action("generate_snapshot_bin") {
]
args = []
if (use_nnbd) {
args += [ "--enable-experiment=non-nullable" ]
args += [
"--enable-experiment=non-nullable",
"--null-safety",
]
}
args += [
"--deterministic",
Expand Down
1 change: 1 addition & 0 deletions runtime/include/dart_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@ typedef struct {
bool load_vmservice_library;
bool unsafe_trust_strong_mode_types;
bool copy_parent_code;
bool null_safety;
} Dart_IsolateFlags;

/**
Expand Down
3 changes: 2 additions & 1 deletion runtime/vm/compiler/backend/flow_graph_compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2373,7 +2373,8 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
// instantiated with a non-nullable type which rejects null.
// In NNBD weak mode or if type parameter is non-nullable or has
// undetermined nullability null instance is correctly handled by TTS.
if (FLAG_null_safety && (dst_type.IsNullable() || dst_type.IsLegacy())) {
if (isolate()->null_safety() &&
(dst_type.IsNullable() || dst_type.IsLegacy())) {
__ CompareObject(TypeTestABI::kInstanceReg, Object::null_object());
__ BranchIf(EQUAL, done);
}
Expand Down
21 changes: 13 additions & 8 deletions runtime/vm/compiler/frontend/constant_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ bool ConstantReader::IsInstanceConstant(intptr_t constant_offset,

RawInstance* ConstantReader::ReadConstantInternal(intptr_t constant_offset) {
// Get reader directly into raw bytes of constant table.
bool null_safety = H.thread()->isolate()->null_safety();
KernelReaderHelper reader(Z, &H, script_, H.constants_table(), 0);
reader.ReadUInt(); // skip variable-sized int for adjusted constant offset
reader.SetOffset(reader.ReaderOffset() + constant_offset);
Expand Down Expand Up @@ -182,8 +183,9 @@ RawInstance* ConstantReader::ReadConstantInternal(intptr_t constant_offset) {
const auto& list_class =
Class::Handle(Z, corelib.LookupClassAllowPrivate(Symbols::_List()));
// Build type from the raw bytes (needs temporary translator).
TypeTranslator type_translator(&reader, this, active_class_, true,
active_class_->RequireLegacyErasure());
TypeTranslator type_translator(
&reader, this, active_class_, true,
active_class_->RequireLegacyErasure(null_safety));
auto& type_arguments =
TypeArguments::Handle(Z, TypeArguments::New(1, Heap::kOld));
AbstractType& type = type_translator.BuildType();
Expand Down Expand Up @@ -224,8 +226,9 @@ RawInstance* ConstantReader::ReadConstantInternal(intptr_t constant_offset) {
ASSERT(klass.is_enum_class() || klass.is_const());
instance = Instance::New(klass, Heap::kOld);
// Build type from the raw bytes (needs temporary translator).
TypeTranslator type_translator(&reader, this, active_class_, true,
active_class_->RequireLegacyErasure());
TypeTranslator type_translator(
&reader, this, active_class_, true,
active_class_->RequireLegacyErasure(null_safety));
const intptr_t number_of_type_arguments = reader.ReadUInt();
if (klass.NumTypeArguments() > 0) {
auto& type_arguments = TypeArguments::Handle(
Expand Down Expand Up @@ -267,8 +270,9 @@ RawInstance* ConstantReader::ReadConstantInternal(intptr_t constant_offset) {
ASSERT(!constant.IsNull());

// Build type from the raw bytes (needs temporary translator).
TypeTranslator type_translator(&reader, this, active_class_, true,
active_class_->RequireLegacyErasure());
TypeTranslator type_translator(
&reader, this, active_class_, true,
active_class_->RequireLegacyErasure(null_safety));
const intptr_t number_of_type_arguments = reader.ReadUInt();
ASSERT(number_of_type_arguments > 0);
auto& type_arguments = TypeArguments::Handle(
Expand Down Expand Up @@ -301,8 +305,9 @@ RawInstance* ConstantReader::ReadConstantInternal(intptr_t constant_offset) {
}
case kTypeLiteralConstant: {
// Build type from the raw bytes (needs temporary translator).
TypeTranslator type_translator(&reader, this, active_class_, true,
active_class_->RequireLegacyErasure());
TypeTranslator type_translator(
&reader, this, active_class_, true,
active_class_->RequireLegacyErasure(null_safety));
instance = type_translator.BuildType().raw();
break;
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/compiler/frontend/kernel_translation_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -1294,8 +1294,8 @@ class ActiveClass {
return member->IsFactory();
}

bool RequireLegacyErasure() const {
return klass != nullptr && !FLAG_null_safety &&
bool RequireLegacyErasure(bool null_safety) const {
return klass != nullptr && !null_safety &&
Library::Handle(klass->library()).nnbd_compiled_mode() ==
NNBDCompiledMode::kAgnostic;
}
Expand Down
3 changes: 2 additions & 1 deletion runtime/vm/compiler/frontend/prologue_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ Fragment PrologueBuilder::BuildOptionalParameterHandling(
} else {
ASSERT(num_opt_named_params > 0);

bool null_safety = Isolate::Current()->null_safety();
const intptr_t first_name_offset =
compiler::target::ArgumentsDescriptor::first_named_entry_offset() -
compiler::target::Array::data_offset();
Expand Down Expand Up @@ -384,7 +385,7 @@ Fragment PrologueBuilder::BuildOptionalParameterHandling(
// We had no match. If the param is required, throw a NoSuchMethod error.
// Otherwise just load the default constant.
Fragment not_good(missing);
if (FLAG_null_safety && function_.IsRequiredAt(opt_param_position[i])) {
if (null_safety && function_.IsRequiredAt(opt_param_position[i])) {
not_good += Goto(nsm);
} else {
not_good += Constant(
Expand Down
2 changes: 2 additions & 0 deletions runtime/vm/isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ typedef FixedCache<intptr_t, CatchEntryMovesRefPtr, 16> CatchEntryMovesCache;
V(NONPRODUCT, use_field_guards, UseFieldGuards, use_field_guards, \
FLAG_use_field_guards) \
V(NONPRODUCT, use_osr, UseOsr, use_osr, FLAG_use_osr) \
V(PRODUCT, null_safety, NullSafety, null_safety, FLAG_null_safety) \
V(PRECOMPILER, obfuscate, Obfuscate, obfuscate, false_by_default) \
V(PRODUCT, unsafe_trust_strong_mode_types, UnsafeTrustStrongModeTypes, \
unsafe_trust_strong_mode_types, \
Expand Down Expand Up @@ -1258,6 +1259,7 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
V(UseOsr) \
V(Obfuscate) \
V(ShouldLoadVmService) \
V(NullSafety) \
V(UnsafeTrustStrongModeTypes)

// Isolate specific flags.
Expand Down
30 changes: 20 additions & 10 deletions runtime/vm/isolate_reload.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2062,7 +2062,10 @@ class FieldInvalidator {
delayed_function_type_arguments_(TypeArguments::Handle(zone)) {}

void CheckStatics(const GrowableArray<const Field*>& fields) {
HANDLESCOPE(Thread::Current());
Thread* thread = Thread::Current();
Isolate* isolate = thread->isolate();
bool null_safety = isolate->null_safety();
HANDLESCOPE(thread);
instantiator_type_arguments_ = TypeArguments::null();
for (intptr_t i = 0; i < fields.length(); i++) {
const Field& field = *fields[i];
Expand All @@ -2074,24 +2077,27 @@ class FieldInvalidator {
}
value_ = field.StaticValue();
if (value_.raw() != Object::sentinel().raw()) {
CheckValueType(value_, field);
CheckValueType(null_safety, value_, field);
}
}
}

void CheckInstances(const GrowableArray<const Instance*>& instances) {
Thread* thread = Thread::Current();
Isolate* isolate = thread->isolate();
bool null_safety = isolate->null_safety();
for (intptr_t i = 0; i < instances.length(); i++) {
// This handle scope does run very frequently, but is a net-win by
// preventing us from spending too much time in malloc for new handle
// blocks.
HANDLESCOPE(Thread::Current());
CheckInstance(*instances[i]);
HANDLESCOPE(thread);
CheckInstance(null_safety, *instances[i]);
}
}

private:
DART_FORCE_INLINE
void CheckInstance(const Instance& instance) {
void CheckInstance(bool null_safety, const Instance& instance) {
cls_ = instance.clazz();
if (cls_.NumTypeArguments() > 0) {
instantiator_type_arguments_ = instance.GetTypeArguments();
Expand All @@ -2105,12 +2111,14 @@ class FieldInvalidator {
continue;
}
const Field& field = Field::Cast(entry_);
CheckInstanceField(instance, field);
CheckInstanceField(null_safety, instance, field);
}
}

DART_FORCE_INLINE
void CheckInstanceField(const Instance& instance, const Field& field) {
void CheckInstanceField(bool null_safety,
const Instance& instance,
const Field& field) {
if (field.needs_load_guard()) {
return; // Already guarding.
}
Expand All @@ -2121,12 +2129,14 @@ class FieldInvalidator {
field.set_needs_load_guard(true);
return;
}
CheckValueType(value_, field);
CheckValueType(null_safety, value_, field);
}

DART_FORCE_INLINE
void CheckValueType(const Instance& value, const Field& field) {
if (!FLAG_null_safety && value.IsNull()) {
void CheckValueType(bool null_safety,
const Instance& value,
const Field& field) {
if (!null_safety && value.IsNull()) {
return;
}
type_ = field.type();
Expand Down
3 changes: 2 additions & 1 deletion runtime/vm/kernel_isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,8 @@ class KernelCompilationRequest : public ValueObject {

Dart_CObject null_safety;
null_safety.type = Dart_CObject_kBool;
null_safety.value.as_bool = FLAG_null_safety;
null_safety.value.as_bool =
isolate != NULL ? isolate->null_safety() : FLAG_null_safety;

intptr_t num_experimental_flags = experimental_flags->length();
Dart_CObject** experimental_flags_array =
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/kernel_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1030,13 +1030,13 @@ RawLibrary* KernelLoader::LoadLibrary(intptr_t index) {
library.set_is_nnbd(library_helper.IsNonNullableByDefault());
const NNBDCompiledMode mode =
library_helper.GetNonNullableByDefaultCompiledMode();
if (!FLAG_null_safety && mode == NNBDCompiledMode::kStrong) {
if (!I->null_safety() && mode == NNBDCompiledMode::kStrong) {
H.ReportError(
"Library '%s' was compiled with null safety (in strong mode) and it "
"requires --null-safety option at runtime",
String::Handle(library.url()).ToCString());
}
if (FLAG_null_safety && (mode == NNBDCompiledMode::kWeak ||
if (I->null_safety() && (mode == NNBDCompiledMode::kWeak ||
mode == NNBDCompiledMode::kDisabled)) {
H.ReportError(
"Library '%s' was compiled without null safety (in weak mode) and it "
Expand Down
41 changes: 26 additions & 15 deletions runtime/vm/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7499,7 +7499,9 @@ bool Function::AreValidArguments(const ArgumentsDescriptor& args_desc,
return false;
}
// Verify that all argument names are valid parameter names.
Zone* zone = Thread::Current()->zone();
Thread* thread = Thread::Current();
Isolate* isolate = thread->isolate();
Zone* zone = thread->zone();
String& argument_name = String::Handle(zone);
String& parameter_name = String::Handle(zone);
const intptr_t num_positional_args = num_arguments - num_named_arguments;
Expand Down Expand Up @@ -7528,7 +7530,7 @@ bool Function::AreValidArguments(const ArgumentsDescriptor& args_desc,
return false;
}
}
if (FLAG_null_safety) {
if (isolate->null_safety()) {
// Verify that all required named parameters are filled.
for (intptr_t j = num_positional_args; j < num_parameters; j++) {
if (IsRequiredAt(j)) {
Expand Down Expand Up @@ -17645,7 +17647,7 @@ bool Instance::IsAssignableTo(
// In weak mode type casts, whether in legacy or opted-in libraries, the null
// instance is detected and handled in inlined code and therefore cannot be
// encountered here as a Dart null receiver.
ASSERT(FLAG_null_safety || !IsNull());
ASSERT(Isolate::Current()->null_safety() || !IsNull());
// In strong mode, compute NNBD_SUBTYPE(runtimeType, other).
// In weak mode, compute LEGACY_SUBTYPE(runtimeType, other).
return RuntimeTypeIsSubtypeOf(other, other_instantiator_type_arguments,
Expand Down Expand Up @@ -17687,8 +17689,12 @@ bool Instance::NullIsInstanceOf(
}

bool Instance::NullIsAssignableTo(const AbstractType& other) {
Thread* thread = Thread::Current();
Isolate* isolate = thread->isolate();
Zone* zone = thread->zone();

// In weak mode, Null is a bottom type (according to LEGACY_SUBTYPE).
if (!FLAG_null_safety) {
if (!isolate->null_safety()) {
return true;
}
// "Left Null" rule: null is assignable when destination type is either
Expand All @@ -17698,7 +17704,8 @@ bool Instance::NullIsAssignableTo(const AbstractType& other) {
return true;
}
if (other.IsFutureOrType()) {
return NullIsAssignableTo(AbstractType::Handle(other.UnwrapFutureOr()));
return NullIsAssignableTo(
AbstractType::Handle(zone, other.UnwrapFutureOr()));
}
return false;
}
Expand All @@ -17715,12 +17722,13 @@ bool Instance::RuntimeTypeIsSubtypeOf(
if (other.IsTopType()) {
return true;
}
Thread* thread = Thread::Current();
Isolate* isolate = thread->isolate();
Zone* zone = thread->zone();
// In weak testing mode, Null type is a subtype of any type.
if (IsNull() && !FLAG_null_safety) {
if (IsNull() && !isolate->null_safety()) {
return true;
}
Thread* thread = Thread::Current();
Zone* zone = thread->zone();
const Class& cls = Class::Handle(zone, clazz());
if (cls.IsClosureClass()) {
if (other.IsDartFunctionType() || other.IsDartClosureType()) {
Expand Down Expand Up @@ -17782,7 +17790,7 @@ bool Instance::RuntimeTypeIsSubtypeOf(
return false;
}
if (IsNull()) {
ASSERT(FLAG_null_safety);
ASSERT(isolate->null_safety());
if (instantiated_other.IsNullType()) {
return true;
}
Expand Down Expand Up @@ -18481,7 +18489,7 @@ bool AbstractType::IsTopTypeForAssignability() const {
if (cid == kInstanceCid) { // Object type.
// NNBD weak mode uses LEGACY_SUBTYPE for assignability / 'as' tests,
// and non-nullable Object is a top type according to LEGACY_SUBTYPE.
return !FLAG_null_safety || !IsNonNullable();
return !Isolate::Current()->null_safety() || !IsNonNullable();
}
if (cid == kFutureOrCid) {
// FutureOr<T> where T is a top type behaves as a top type.
Expand Down Expand Up @@ -18592,6 +18600,7 @@ bool AbstractType::IsSubtypeOf(const AbstractType& other,
return Instance::NullIsAssignableTo(other);
}
Thread* thread = Thread::Current();
Isolate* isolate = thread->isolate();
Zone* zone = thread->zone();
// Type parameters cannot be handled by Class::IsSubtypeOf().
// When comparing two uninstantiated function types, one returning type
Expand Down Expand Up @@ -18628,7 +18637,7 @@ bool AbstractType::IsSubtypeOf(const AbstractType& other,
if (other.IsTypeParameter()) {
return false;
}
if (FLAG_null_safety && IsNullable() && other.IsNonNullable()) {
if (isolate->null_safety() && IsNullable() && other.IsNonNullable()) {
return false;
}
const Class& type_cls = Class::Handle(zone, type_class());
Expand Down Expand Up @@ -19033,8 +19042,12 @@ bool Type::IsEquivalent(const Instance& other,
}
Nullability this_type_nullability = nullability();
Nullability other_type_nullability = other_type.nullability();
Thread* thread = Thread::Current();
Isolate* isolate = thread->isolate();
Zone* zone = thread->zone();
if (kind == TypeEquality::kInSubtypeTest) {
if (FLAG_null_safety && this_type_nullability == Nullability::kNullable &&
if (isolate->null_safety() &&
this_type_nullability == Nullability::kNullable &&
other_type_nullability == Nullability::kNonNullable) {
return false;
}
Expand All @@ -19060,8 +19073,6 @@ bool Type::IsEquivalent(const Instance& other,
(signature() == other_type.signature())) {
return true;
}
Thread* thread = Thread::Current();
Zone* zone = thread->zone();
if (arguments() != other_type.arguments()) {
const Class& cls = Class::Handle(zone, type_class());
const intptr_t num_type_params = cls.NumTypeParameters(thread);
Expand Down Expand Up @@ -19754,7 +19765,7 @@ bool TypeParameter::IsEquivalent(const Instance& other,
Nullability this_type_param_nullability = nullability();
Nullability other_type_param_nullability = other_type_param.nullability();
if (kind == TypeEquality::kInSubtypeTest) {
if (FLAG_null_safety &&
if (Isolate::Current()->null_safety() &&
(this_type_param_nullability == Nullability::kNullable) &&
(other_type_param_nullability == Nullability::kNonNullable)) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/runtime_entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ DEFINE_RUNTIME_ENTRY(TypeCheck, 7) {
ASSERT(!dst_type.IsDynamicType()); // No need to check assignment.
// A null instance is already detected and allowed in inlined code, unless
// strong checking is enabled.
ASSERT(!src_instance.IsNull() || FLAG_null_safety);
ASSERT(!src_instance.IsNull() || isolate->null_safety());
const bool is_instance_of = src_instance.IsAssignableTo(
dst_type, instantiator_type_arguments, function_type_arguments);

Expand Down
Loading

0 comments on commit 1bab476

Please sign in to comment.