Skip to content

Commit

Permalink
Revert "[vm] Check prefix.loadLibrary is called and returns before pr…
Browse files Browse the repository at this point in the history
…efix members are used."

This reverts commit f049a41.

Reason for revert: Broke reload bots.

Original change's description:
> [vm] Check prefix.loadLibrary is called and returns before prefix members are used.
> 
> Not loading is actually deferred.
> 
> Bug: #41974
> Change-Id: I62688007bd36dbcb2e8ffb4a1fd2dceb1775b1c8
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149053
> Commit-Queue: Ryan Macnak <[email protected]>
> Reviewed-by: Siva Annamalai <[email protected]>

[email protected],[email protected],[email protected]

Change-Id: I923e339465fdf13199efc11a9cef4a842abebd67
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #41974
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149423
Reviewed-by: Aske Simon Christensen <[email protected]>
Commit-Queue: Aske Simon Christensen <[email protected]>
  • Loading branch information
askeksa authored and [email protected] committed May 29, 2020
1 parent f585be5 commit bbf3403
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 138 deletions.
13 changes: 0 additions & 13 deletions runtime/lib/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,19 +196,6 @@ DEFINE_NATIVE_ENTRY(Type_equality, 0, 2) {
return Bool::Get(type.IsEquivalent(other, TypeEquality::kSyntactical)).raw();
}

DEFINE_NATIVE_ENTRY(LibraryPrefix_isLoaded, 0, 1) {
const LibraryPrefix& prefix =
LibraryPrefix::CheckedHandle(zone, arguments->NativeArgAt(0));
return Bool::Get(prefix.is_loaded()).raw();
}

DEFINE_NATIVE_ENTRY(LibraryPrefix_setLoaded, 0, 1) {
const LibraryPrefix& prefix =
LibraryPrefix::CheckedHandle(zone, arguments->NativeArgAt(0));
prefix.set_is_loaded(true);
return Instance::null();
}

DEFINE_NATIVE_ENTRY(Internal_inquireIs64Bit, 0, 0) {
#if defined(ARCH_IS_64_BIT)
return Bool::True().raw();
Expand Down
2 changes: 0 additions & 2 deletions runtime/vm/bootstrap_natives.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ namespace dart {
V(AbstractType_toString, 1) \
V(Type_getHashCode, 1) \
V(Type_equality, 2) \
V(LibraryPrefix_isLoaded, 1) \
V(LibraryPrefix_setLoaded, 1) \
V(Identical_comparison, 2) \
V(Integer_bitAndFromInteger, 2) \
V(Integer_bitOrFromInteger, 2) \
Expand Down
1 change: 0 additions & 1 deletion runtime/vm/clustered_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3248,7 +3248,6 @@ class LibraryPrefixDeserializationCluster : public DeserializationCluster {
ReadFromTo(prefix);
prefix->ptr()->num_imports_ = d->Read<uint16_t>();
prefix->ptr()->is_deferred_load_ = d->Read<bool>();
prefix->ptr()->is_loaded_ = false;
}
}
};
Expand Down
3 changes: 1 addition & 2 deletions runtime/vm/compiler/backend/il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1099,8 +1099,7 @@ ConstantInstr::ConstantInstr(const Object& value, TokenPosition token_pos)
// tables used for certain character classes are represented as TypedData,
// and so those values are also neither immutable (as there are no immutable
// TypedData values) or canonical.
ASSERT(value.IsTypeParameter() || value.IsArray() || value.IsTypedData() ||
value.IsLibraryPrefix());
ASSERT(value.IsTypeParameter() || value.IsArray() || value.IsTypedData());
}
#endif
}
Expand Down
24 changes: 2 additions & 22 deletions runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1363,9 +1363,9 @@ Fragment StreamingFlowGraphBuilder::BuildExpression(TokenPosition* position) {
case kInstantiation:
return BuildPartialTearoffInstantiation(position);
case kLoadLibrary:
return BuildLibraryPrefixAction(position, Symbols::LoadLibrary());
case kCheckLibraryIsLoaded:
return BuildLibraryPrefixAction(position, Symbols::CheckLoaded());
ReadUInt(); // skip library index
return BuildFutureNullValue(position);
case kConstStaticInvocation:
case kConstConstructorInvocation:
case kConstListLiteral:
Expand Down Expand Up @@ -4067,26 +4067,6 @@ Fragment StreamingFlowGraphBuilder::BuildPartialTearoffInstantiation(
return instructions;
}

Fragment StreamingFlowGraphBuilder::BuildLibraryPrefixAction(
TokenPosition* position,
const String& selector) {
const intptr_t dependency_index = ReadUInt();
const Library& current_library = Library::Handle(
Z, Class::Handle(Z, parsed_function()->function().Owner()).library());
const Array& dependencies = Array::Handle(Z, current_library.dependencies());
const LibraryPrefix& prefix =
LibraryPrefix::CheckedZoneHandle(Z, dependencies.At(dependency_index));
const Function& function =
Function::ZoneHandle(Z, Library::Handle(Z, Library::CoreLibrary())
.LookupFunctionAllowPrivate(selector));
ASSERT(!function.IsNull());
Fragment instructions;
instructions += Constant(prefix);
instructions +=
StaticCall(TokenPosition::kNoSource, function, 1, ICData::kStatic);
return instructions;
}

Fragment StreamingFlowGraphBuilder::BuildExpressionStatement() {
Fragment instructions = BuildExpression(); // read expression.
instructions += Drop();
Expand Down
2 changes: 0 additions & 2 deletions runtime/vm/compiler/frontend/kernel_binary_flowgraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,6 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
Fragment BuildFutureNullValue(TokenPosition* position);
Fragment BuildConstantExpression(TokenPosition* position, Tag tag);
Fragment BuildPartialTearoffInstantiation(TokenPosition* position);
Fragment BuildLibraryPrefixAction(TokenPosition* position,
const String& selector);

Fragment BuildExpressionStatement();
Fragment BuildBlock();
Expand Down
28 changes: 14 additions & 14 deletions runtime/vm/compiler/runtime_offsets_extracted.h
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ static constexpr dart::compiler::target::word Integer_InstanceSize = 4;
static constexpr dart::compiler::target::word KernelProgramInfo_InstanceSize =
64;
static constexpr dart::compiler::target::word LanguageError_InstanceSize = 28;
static constexpr dart::compiler::target::word Library_InstanceSize = 80;
static constexpr dart::compiler::target::word Library_InstanceSize = 76;
static constexpr dart::compiler::target::word LibraryPrefix_InstanceSize = 20;
static constexpr dart::compiler::target::word LinkedHashMap_InstanceSize = 28;
static constexpr dart::compiler::target::word LocalVarDescriptors_InstanceSize =
Expand Down Expand Up @@ -927,7 +927,7 @@ static constexpr dart::compiler::target::word Integer_InstanceSize = 8;
static constexpr dart::compiler::target::word KernelProgramInfo_InstanceSize =
128;
static constexpr dart::compiler::target::word LanguageError_InstanceSize = 48;
static constexpr dart::compiler::target::word Library_InstanceSize = 152;
static constexpr dart::compiler::target::word Library_InstanceSize = 144;
static constexpr dart::compiler::target::word LibraryPrefix_InstanceSize = 40;
static constexpr dart::compiler::target::word LinkedHashMap_InstanceSize = 56;
static constexpr dart::compiler::target::word LocalVarDescriptors_InstanceSize =
Expand Down Expand Up @@ -1404,7 +1404,7 @@ static constexpr dart::compiler::target::word Integer_InstanceSize = 4;
static constexpr dart::compiler::target::word KernelProgramInfo_InstanceSize =
64;
static constexpr dart::compiler::target::word LanguageError_InstanceSize = 28;
static constexpr dart::compiler::target::word Library_InstanceSize = 80;
static constexpr dart::compiler::target::word Library_InstanceSize = 76;
static constexpr dart::compiler::target::word LibraryPrefix_InstanceSize = 20;
static constexpr dart::compiler::target::word LinkedHashMap_InstanceSize = 28;
static constexpr dart::compiler::target::word LocalVarDescriptors_InstanceSize =
Expand Down Expand Up @@ -1890,7 +1890,7 @@ static constexpr dart::compiler::target::word Integer_InstanceSize = 8;
static constexpr dart::compiler::target::word KernelProgramInfo_InstanceSize =
128;
static constexpr dart::compiler::target::word LanguageError_InstanceSize = 48;
static constexpr dart::compiler::target::word Library_InstanceSize = 152;
static constexpr dart::compiler::target::word Library_InstanceSize = 144;
static constexpr dart::compiler::target::word LibraryPrefix_InstanceSize = 40;
static constexpr dart::compiler::target::word LinkedHashMap_InstanceSize = 56;
static constexpr dart::compiler::target::word LocalVarDescriptors_InstanceSize =
Expand Down Expand Up @@ -2366,7 +2366,7 @@ static constexpr dart::compiler::target::word Integer_InstanceSize = 4;
static constexpr dart::compiler::target::word KernelProgramInfo_InstanceSize =
64;
static constexpr dart::compiler::target::word LanguageError_InstanceSize = 28;
static constexpr dart::compiler::target::word Library_InstanceSize = 80;
static constexpr dart::compiler::target::word Library_InstanceSize = 76;
static constexpr dart::compiler::target::word LibraryPrefix_InstanceSize = 20;
static constexpr dart::compiler::target::word LinkedHashMap_InstanceSize = 28;
static constexpr dart::compiler::target::word LocalVarDescriptors_InstanceSize =
Expand Down Expand Up @@ -2845,7 +2845,7 @@ static constexpr dart::compiler::target::word Integer_InstanceSize = 8;
static constexpr dart::compiler::target::word KernelProgramInfo_InstanceSize =
128;
static constexpr dart::compiler::target::word LanguageError_InstanceSize = 48;
static constexpr dart::compiler::target::word Library_InstanceSize = 152;
static constexpr dart::compiler::target::word Library_InstanceSize = 144;
static constexpr dart::compiler::target::word LibraryPrefix_InstanceSize = 40;
static constexpr dart::compiler::target::word LinkedHashMap_InstanceSize = 56;
static constexpr dart::compiler::target::word LocalVarDescriptors_InstanceSize =
Expand Down Expand Up @@ -3316,7 +3316,7 @@ static constexpr dart::compiler::target::word Integer_InstanceSize = 4;
static constexpr dart::compiler::target::word KernelProgramInfo_InstanceSize =
64;
static constexpr dart::compiler::target::word LanguageError_InstanceSize = 28;
static constexpr dart::compiler::target::word Library_InstanceSize = 80;
static constexpr dart::compiler::target::word Library_InstanceSize = 76;
static constexpr dart::compiler::target::word LibraryPrefix_InstanceSize = 20;
static constexpr dart::compiler::target::word LinkedHashMap_InstanceSize = 28;
static constexpr dart::compiler::target::word LocalVarDescriptors_InstanceSize =
Expand Down Expand Up @@ -3796,7 +3796,7 @@ static constexpr dart::compiler::target::word Integer_InstanceSize = 8;
static constexpr dart::compiler::target::word KernelProgramInfo_InstanceSize =
128;
static constexpr dart::compiler::target::word LanguageError_InstanceSize = 48;
static constexpr dart::compiler::target::word Library_InstanceSize = 152;
static constexpr dart::compiler::target::word Library_InstanceSize = 144;
static constexpr dart::compiler::target::word LibraryPrefix_InstanceSize = 40;
static constexpr dart::compiler::target::word LinkedHashMap_InstanceSize = 56;
static constexpr dart::compiler::target::word LocalVarDescriptors_InstanceSize =
Expand Down Expand Up @@ -4307,7 +4307,7 @@ static constexpr dart::compiler::target::word
AOT_KernelProgramInfo_InstanceSize = 64;
static constexpr dart::compiler::target::word AOT_LanguageError_InstanceSize =
28;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 76;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 72;
static constexpr dart::compiler::target::word AOT_LibraryPrefix_InstanceSize =
20;
static constexpr dart::compiler::target::word AOT_LinkedHashMap_InstanceSize =
Expand Down Expand Up @@ -4833,7 +4833,7 @@ static constexpr dart::compiler::target::word
AOT_KernelProgramInfo_InstanceSize = 128;
static constexpr dart::compiler::target::word AOT_LanguageError_InstanceSize =
48;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 144;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 136;
static constexpr dart::compiler::target::word AOT_LibraryPrefix_InstanceSize =
40;
static constexpr dart::compiler::target::word AOT_LinkedHashMap_InstanceSize =
Expand Down Expand Up @@ -5363,7 +5363,7 @@ static constexpr dart::compiler::target::word
AOT_KernelProgramInfo_InstanceSize = 128;
static constexpr dart::compiler::target::word AOT_LanguageError_InstanceSize =
48;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 144;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 136;
static constexpr dart::compiler::target::word AOT_LibraryPrefix_InstanceSize =
40;
static constexpr dart::compiler::target::word AOT_LinkedHashMap_InstanceSize =
Expand Down Expand Up @@ -5880,7 +5880,7 @@ static constexpr dart::compiler::target::word
AOT_KernelProgramInfo_InstanceSize = 64;
static constexpr dart::compiler::target::word AOT_LanguageError_InstanceSize =
28;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 76;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 72;
static constexpr dart::compiler::target::word AOT_LibraryPrefix_InstanceSize =
20;
static constexpr dart::compiler::target::word AOT_LinkedHashMap_InstanceSize =
Expand Down Expand Up @@ -6399,7 +6399,7 @@ static constexpr dart::compiler::target::word
AOT_KernelProgramInfo_InstanceSize = 128;
static constexpr dart::compiler::target::word AOT_LanguageError_InstanceSize =
48;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 144;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 136;
static constexpr dart::compiler::target::word AOT_LibraryPrefix_InstanceSize =
40;
static constexpr dart::compiler::target::word AOT_LinkedHashMap_InstanceSize =
Expand Down Expand Up @@ -6922,7 +6922,7 @@ static constexpr dart::compiler::target::word
AOT_KernelProgramInfo_InstanceSize = 128;
static constexpr dart::compiler::target::word AOT_LanguageError_InstanceSize =
48;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 144;
static constexpr dart::compiler::target::word AOT_Library_InstanceSize = 136;
static constexpr dart::compiler::target::word AOT_LibraryPrefix_InstanceSize =
40;
static constexpr dart::compiler::target::word AOT_LinkedHashMap_InstanceSize =
Expand Down
10 changes: 0 additions & 10 deletions runtime/vm/kernel_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1305,7 +1305,6 @@ void KernelLoader::LoadLibraryImportsAndExports(Library* library,
LibraryPrefix& library_prefix = LibraryPrefix::Handle(Z);

const intptr_t deps_count = helper_.ReadListLength();
const Array& deps = Array::Handle(Array::New(deps_count));
for (intptr_t dep = 0; dep < deps_count; ++dep) {
LibraryDependencyHelper dependency_helper(&helper_);

Expand Down Expand Up @@ -1389,21 +1388,12 @@ void KernelLoader::LoadLibraryImportsAndExports(Library* library,
}
}
}

if (FLAG_enable_mirrors && dependency_helper.annotation_count_ > 0) {
ASSERT(annotations_kernel_offset > 0);
ns.AddMetadata(toplevel_class, TokenPosition::kNoSource,
annotations_kernel_offset);
}

if (prefix.IsNull()) {
deps.SetAt(dep, ns);
} else {
deps.SetAt(dep, library_prefix);
}
}

library->set_dependencies(deps);
}

void KernelLoader::LoadPreliminaryClass(ClassHelper* class_helper,
Expand Down
29 changes: 23 additions & 6 deletions runtime/vm/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11047,6 +11047,27 @@ void ClassDictionaryIterator::MoveToNextClass() {
}
}

LibraryPrefixIterator::LibraryPrefixIterator(const Library& library)
: DictionaryIterator(library) {
Advance();
}

LibraryPrefixPtr LibraryPrefixIterator::GetNext() {
ASSERT(HasNext());
int ix = next_ix_++;
Object& obj = Object::Handle(array_.At(ix));
Advance();
return LibraryPrefix::Cast(obj).raw();
}

void LibraryPrefixIterator::Advance() {
Object& obj = Object::Handle(array_.At(next_ix_));
while (!obj.IsLibraryPrefix() && HasNext()) {
next_ix_++;
obj = array_.At(next_ix_);
}
}

static void ReportTooManyImports(const Library& lib) {
const String& url = String::Handle(lib.url());
Report::MessageF(Report::kError, Script::Handle(lib.LookupScript(url)),
Expand Down Expand Up @@ -12020,10 +12041,6 @@ void Library::set_toplevel_class(const Class& value) const {
StorePointer(&raw_ptr()->toplevel_class_, value.raw());
}

void Library::set_dependencies(const Array& deps) const {
StorePointer(&raw_ptr()->dependencies_, deps.raw());
}

void Library::set_metadata(const GrowableObjectArray& value) const {
StorePointer(&raw_ptr()->metadata_, value.raw());
}
Expand Down Expand Up @@ -12892,7 +12909,6 @@ LibraryPrefixPtr LibraryPrefix::New(const String& name,
result.set_num_imports(0);
result.set_importer(importer);
result.StoreNonPointer(&result.raw_ptr()->is_deferred_load_, deferred_load);
result.StoreNonPointer(&result.raw_ptr()->is_loaded_, false);
result.set_imports(Array::Handle(Array::New(kInitialSize)));
result.AddImport(import);
return result.raw();
Expand Down Expand Up @@ -12920,7 +12936,8 @@ void LibraryPrefix::set_importer(const Library& value) const {

const char* LibraryPrefix::ToCString() const {
const String& prefix = String::Handle(name());
return prefix.ToCString();
return OS::SCreate(Thread::Current()->zone(), "LibraryPrefix:'%s'",
prefix.ToCString());
}

void Namespace::set_metadata_field(const Field& value) const {
Expand Down
18 changes: 11 additions & 7 deletions runtime/vm/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -4519,6 +4519,7 @@ class DictionaryIterator : public ValueObject {
int next_ix_; // Index of next element.

friend class ClassDictionaryIterator;
friend class LibraryPrefixIterator;
DISALLOW_COPY_AND_ASSIGN(DictionaryIterator);
};

Expand Down Expand Up @@ -4549,6 +4550,16 @@ class ClassDictionaryIterator : public DictionaryIterator {
DISALLOW_COPY_AND_ASSIGN(ClassDictionaryIterator);
};

class LibraryPrefixIterator : public DictionaryIterator {
public:
explicit LibraryPrefixIterator(const Library& library);
LibraryPrefixPtr GetNext();

private:
void Advance();
DISALLOW_COPY_AND_ASSIGN(LibraryPrefixIterator);
};

class Library : public Object {
public:
StringPtr name() const { return raw_ptr()->name_; }
Expand Down Expand Up @@ -4703,9 +4714,6 @@ class Library : public Object {
NamespacePtr ImportAt(intptr_t index) const;
LibraryPtr ImportLibraryAt(intptr_t index) const;

ArrayPtr dependencies() const { return raw_ptr()->dependencies_; }
void set_dependencies(const Array& deps) const;

void DropDependenciesAndCaches() const;

// Resolving native methods for script loaded in the library.
Expand Down Expand Up @@ -7311,10 +7319,6 @@ class LibraryPrefix : public Instance {
void AddImport(const Namespace& import) const;

bool is_deferred_load() const { return raw_ptr()->is_deferred_load_; }
bool is_loaded() const { return raw_ptr()->is_loaded_; }
void set_is_loaded(bool value) const {
return StoreNonPointer(&raw_ptr()->is_loaded_, value);
}

static intptr_t InstanceSize() {
return RoundedAllocationSize(sizeof(LibraryPrefixLayout));
Expand Down
4 changes: 1 addition & 3 deletions runtime/vm/raw_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -1305,12 +1305,11 @@ class LibraryLayout : public ObjectLayout {
GrowableObjectArrayPtr used_scripts_;
ArrayPtr imports_; // List of Namespaces imported without prefix.
ArrayPtr exports_; // List of re-exported Namespaces.
ArrayPtr dependencies_;
ExternalTypedDataPtr kernel_data_;
ObjectPtr* to_snapshot(Snapshot::Kind kind) {
switch (kind) {
case Snapshot::kFullAOT:
return reinterpret_cast<ObjectPtr*>(&dependencies_);
return reinterpret_cast<ObjectPtr*>(&exports_);
case Snapshot::kFull:
case Snapshot::kFullJIT:
return reinterpret_cast<ObjectPtr*>(&kernel_data_);
Expand Down Expand Up @@ -2131,7 +2130,6 @@ class LibraryPrefixLayout : public InstanceLayout {
}
uint16_t num_imports_; // Number of library entries in libraries_.
bool is_deferred_load_;
bool is_loaded_;
};

class TypeArgumentsLayout : public InstanceLayout {
Expand Down
2 changes: 0 additions & 2 deletions runtime/vm/symbols.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ class ObjectPointerVisitor;
V(Call, "call") \
V(Cancel, "cancel") \
V(CastError, "_CastError") \
V(CheckLoaded, "_checkLoaded") \
V(Class, "Class") \
V(ClassID, "ClassID") \
V(ClearAsyncThreadStackTrace, "_clearAsyncThreadStackTrace") \
Expand Down Expand Up @@ -206,7 +205,6 @@ class ObjectPointerVisitor;
V(ListFactory, "List.") \
V(ListFilledFactory, "List.filled") \
V(ListLiteralFactory, "List._fromLiteral") \
V(LoadLibrary, "_loadLibrary") \
V(LocalVarDescriptors, "LocalVarDescriptors") \
V(Map, "Map") \
V(MapLiteralFactory, "Map._fromLiteral") \
Expand Down
Loading

0 comments on commit bbf3403

Please sign in to comment.