From: Ian Lance Taylor Date: Wed, 3 Jul 2019 00:56:35 +0000 (+0000) Subject: compiler: rework type and package tracking in exporter X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=61a02d1e97aa9b2cb3c4e3e5c823d3a8a4c5834c;p=gcc.git compiler: rework type and package tracking in exporter Revamps the way the exporter tracks exported types and imported packages that need to be mentioned in the export data. The previous implementation wasn't properly handling the case where an exported non-inlinable function refers to an imported type whose method set includes an inlinable function whose body makes a call to a function in another package that's not directly used in the original package. This patch integrates together two existing traversal helper classes, "Collect_references_from_inline" and "Find_types_to_prepare" into a single helper "Collect_export_references", so as to have common/shared code that looks for indirectly imported packages. Fixes golang/go#32778 Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/183850 From-SVN: r272955 --- diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index 11cba0fa891..c39a36d269a 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -1,4 +1,4 @@ -7f753feb8df400d6ed17cdbdfb364f7f3a42fb31 +aebd2d6303e4bb970b088e84f6c66279095dfea6 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. diff --git a/gcc/go/gofrontend/export.cc b/gcc/go/gofrontend/export.cc index a1bd8cc5364..8cbddac1446 100644 --- a/gcc/go/gofrontend/export.cc +++ b/gcc/go/gofrontend/export.cc @@ -99,29 +99,83 @@ Export::~Export() } // A traversal class to collect functions and global variables -// referenced by inlined functions. +// referenced by inlined functions, and also to gather up +// referenced types that need to be included in the exports. -class Collect_references_from_inline : public Traverse +class Collect_export_references : public Traverse { public: - Collect_references_from_inline(Unordered_set(Named_object*)* exports, - std::vector* check_inline_refs) - : Traverse(traverse_expressions), - exports_(exports), check_inline_refs_(check_inline_refs) + Collect_export_references(Export* exp, + Unordered_set(Named_object*)* exports, + Unordered_set(const Package*)* imports) + : Traverse(traverse_expressions + | traverse_types), + exp_(exp), exports_(exports), imports_(imports), + inline_fcn_worklist_(NULL) { } + // Initial entry point; performs a walk to expand the exports set. + void + expand_exports(std::vector* inlinable_functions); + + // Second entry point (called after the method above), to find + // all types referenced by exports. + void + prepare_types(); + + protected: + // Override of parent class method. int expression(Expression**); + // Override of parent class method. + int + type(Type* type); + + // Traverse the components of a function type. + void + traverse_function_type(Function_type*); + + // Traverse the methods of a named type, and register its package. + void + traverse_named_type(Named_type*); + private: + // The exporter. + Export* exp_; // The set of named objects to export. Unordered_set(Named_object*)* exports_; - // Functions we are exporting with inline bodies that need to be checked. - std::vector* check_inline_refs_; + // Set containing all directly and indirectly imported packages. + Unordered_set(const Package*)* imports_; + // Functions we've already traversed and don't need to visit again. + Unordered_set(Named_object*) checked_functions_; + // Worklist of functions we are exporting with inline bodies that need + // to be checked. + std::vector* inline_fcn_worklist_; }; +void +Collect_export_references::expand_exports(std::vector* fcns) +{ + this->inline_fcn_worklist_ = fcns; + while (!this->inline_fcn_worklist_->empty()) + { + Named_object* no = this->inline_fcn_worklist_->back(); + this->inline_fcn_worklist_->pop_back(); + std::pair ins = + this->checked_functions_.insert(no); + if (ins.second) + { + // This traversal may add new objects to this->exports_ and new + // functions to this->inline_fcn_worklist_. + no->func_value()->block()->traverse(this); + } + } + this->inline_fcn_worklist_ = NULL; +} + int -Collect_references_from_inline::expression(Expression** pexpr) +Collect_export_references::expression(Expression** pexpr) { const Expression* expr = *pexpr; @@ -131,6 +185,10 @@ Collect_references_from_inline::expression(Expression** pexpr) Named_object* no = ve->named_object(); if (no->is_variable() && no->var_value()->is_global()) { + const Package* var_package = no->package(); + if (var_package != NULL) + this->imports_->insert(var_package); + this->exports_->insert(no); no->var_value()->set_is_referenced_by_inline(); } @@ -142,24 +200,31 @@ Collect_references_from_inline::expression(Expression** pexpr) { Named_object* no = fe->named_object(); + const Package* func_package = fe->named_object()->package(); + if (func_package != NULL) + this->imports_->insert(func_package); + if (no->is_function_declaration() && no->func_declaration_value()->type()->is_builtin()) return TRAVERSE_CONTINUE; - std::pair ins = - this->exports_->insert(no); + if (this->inline_fcn_worklist_ != NULL) + { + std::pair ins = + this->exports_->insert(no); - if (no->is_function()) - no->func_value()->set_is_referenced_by_inline(); + if (no->is_function()) + no->func_value()->set_is_referenced_by_inline(); - // If ins.second is false then this object was already in - // exports_, in which case it was already added to - // check_inline_refs_ the first time we added it to exports_, so - // we don't need to add it again. - if (ins.second - && no->is_function() - && no->func_value()->export_for_inlining()) - this->check_inline_refs_->push_back(no); + // If ins.second is false then this object was already in + // exports_, in which case it was already added to + // check_inline_refs_ the first time we added it to exports_, so + // we don't need to add it again. + if (ins.second + && no->is_function() + && no->func_value()->export_for_inlining()) + this->inline_fcn_worklist_->push_back(no); + } return TRAVERSE_CONTINUE; } @@ -167,25 +232,184 @@ Collect_references_from_inline::expression(Expression** pexpr) return TRAVERSE_CONTINUE; } -// A functor to sort Named_object pointers by name. +// Collect up the set of types mentioned in things we're exporting, and collect +// all the packages encountered during type traversal, to make sure we can +// declare things referered to indirectly (for example, in the body of an +// exported inline function from another package). -struct Sort_bindings +void +Collect_export_references::prepare_types() { - bool - operator()(const Named_object* n1, const Named_object* n2) const - { - if (n1->package() != n2->package()) - { - if (n1->package() == NULL) - return true; - if (n2->package() == NULL) - return false; - return n1->package()->pkgpath() < n2->package()->pkgpath(); - } + // Iterate through the exported objects and traverse any types encountered. + for (Unordered_set(Named_object*)::iterator p = this->exports_->begin(); + p != this->exports_->end(); + ++p) + { + Named_object* no = *p; + switch (no->classification()) + { + case Named_object::NAMED_OBJECT_CONST: + { + Type* t = no->const_value()->type(); + if (t != NULL && !t->is_abstract()) + Type::traverse(t, this); + } + break; - return n1->name() < n2->name(); - } -}; + case Named_object::NAMED_OBJECT_TYPE: + Type::traverse(no->type_value()->real_type(), this); + this->traverse_named_type(no->type_value()); + break; + + case Named_object::NAMED_OBJECT_VAR: + Type::traverse(no->var_value()->type(), this); + break; + + case Named_object::NAMED_OBJECT_FUNC: + { + Function* fn = no->func_value(); + this->traverse_function_type(fn->type()); + if (fn->export_for_inlining()) + fn->block()->traverse(this); + } + break; + + case Named_object::NAMED_OBJECT_FUNC_DECLARATION: + this->traverse_function_type(no->func_declaration_value()->type()); + break; + + default: + // We shouldn't see anything else. If we do we'll give an + // error later when we try to actually export it. + break; + } + } +} + +// Record referenced type, record package imports, and make sure we traverse +// methods of named types. + +int +Collect_export_references::type(Type* type) +{ + // Skip forwarders; don't try to give them a type index. + if (type->forward_declaration_type() != NULL) + return TRAVERSE_CONTINUE; + + // Skip the void type, which we'll see when exporting + // unsafe.Pointer. The void type is not itself exported, because + // Pointer_type::do_export checks for it. + if (type->is_void_type()) + return TRAVERSE_SKIP_COMPONENTS; + + // Skip abstract types. We should never see these in real code, + // only in things like const declarations. + if (type->is_abstract()) + return TRAVERSE_SKIP_COMPONENTS; + + // For interfaces make sure that embedded methods are sorted, since the + // comparison function we use for indexing types relies on it (this call has + // to happen before the record_type call below). + if (type->classification() == Type::TYPE_INTERFACE) + { + Interface_type* it = type->interface_type(); + if (it != NULL) + it->sort_embedded(); + } + + if (!this->exp_->record_type(type)) + { + // We've already seen this type. + return TRAVERSE_SKIP_COMPONENTS; + } + + // At this stage of compilation traversing interface types traverses + // the final list of methods, but we export the locally defined + // methods. If there is an embedded interface type we need to make + // sure to export that. Check classification, rather than calling + // the interface_type method, because we want to handle named types + // below. + if (type->classification() == Type::TYPE_INTERFACE) + { + Interface_type* it = type->interface_type(); + const Typed_identifier_list* methods = it->local_methods(); + if (methods != NULL) + { + for (Typed_identifier_list::const_iterator p = methods->begin(); + p != methods->end(); + ++p) + { + if (p->name().empty()) + Type::traverse(p->type(), this); + else + this->traverse_function_type(p->type()->function_type()); + } + } + return TRAVERSE_SKIP_COMPONENTS; + } + + Named_type* nt = type->named_type(); + if (nt != NULL) + this->traverse_named_type(nt); + + return TRAVERSE_CONTINUE; +} + +void +Collect_export_references::traverse_named_type(Named_type* nt) +{ + const Package* package = nt->named_object()->package(); + if (package != NULL) + this->imports_->insert(package); + + // We have to traverse the methods of named types, because we are + // going to export them. This is not done by ordinary type + // traversal. + const Bindings* methods = nt->local_methods(); + if (methods != NULL) + { + for (Bindings::const_definitions_iterator pm = + methods->begin_definitions(); + pm != methods->end_definitions(); + ++pm) + { + Function* fn = (*pm)->func_value(); + this->traverse_function_type(fn->type()); + if (fn->export_for_inlining()) + fn->block()->traverse(this); + } + + for (Bindings::const_declarations_iterator pm = + methods->begin_declarations(); + pm != methods->end_declarations(); + ++pm) + { + Named_object* mno = pm->second; + if (mno->is_function_declaration()) + this->traverse_function_type(mno->func_declaration_value()->type()); + } + } +} + +// Traverse the types in a function type. We don't need the function +// type itself, just the receiver, parameter, and result types. + +void +Collect_export_references::traverse_function_type(Function_type* type) +{ + go_assert(type != NULL); + if (this->remember_type(type)) + return; + const Typed_identifier* receiver = type->receiver(); + if (receiver != NULL) + Type::traverse(receiver->type(), this); + const Typed_identifier_list* parameters = type->parameters(); + if (parameters != NULL) + parameters->traverse(this); + const Typed_identifier_list* results = type->results(); + if (results != NULL) + results->traverse(this); +} // Return true if we should export NO. @@ -224,6 +448,54 @@ should_export(Named_object* no) return true; } +// A functor to sort Named_object pointers by name. + +struct Sort_bindings +{ + bool + operator()(const Named_object* n1, const Named_object* n2) const + { + if (n1->package() != n2->package()) + { + if (n1->package() == NULL) + return true; + if (n2->package() == NULL) + return false; + return n1->package()->pkgpath() < n2->package()->pkgpath(); + } + + return n1->name() < n2->name(); + } +}; + +// A functor to sort types for export. + +struct Sort_types +{ + bool + operator()(const Type* t1, const Type* t2) const + { + const Named_type* nt1 = t1->named_type(); + const Named_type* nt2 = t2->named_type(); + if (nt1 != NULL) + { + if (nt2 != NULL) + { + Sort_bindings sb; + return sb(nt1->named_object(), nt2->named_object()); + } + else + return true; + } + else if (nt2 != NULL) + return false; + if (t1->classification() != t2->classification()) + return t1->classification() < t2->classification(); + Gogo* gogo = go_get_gogo(); + return gogo->type_descriptor_name(t1, NULL).compare(gogo->type_descriptor_name(t2, NULL)) < 0; + } +}; + // Export those identifiers marked for exporting. void @@ -291,29 +563,15 @@ Export::export_globals(const std::string& package_name, exports.insert(p->second); } - // Look through the bodies of the functions in CHECK_INLINE_REFS to - // find other names we may need to export, to satisfy those - // references. Use CHECKED to skip checking function bodies more - // than once. - Unordered_set(Named_object*) checked; - Collect_references_from_inline refs(&exports, &check_inline_refs); - while (!check_inline_refs.empty()) - { - Named_object* no = check_inline_refs.back(); - check_inline_refs.pop_back(); - std::pair ins = - checked.insert(no); - if (ins.second) - { - // This traversal may add new objects to EXPORTS and new - // functions to CHECK_INLINE_REFS. - no->func_value()->block()->traverse(&refs); - } - } - // Track all imported packages mentioned in export data. Unordered_set(const Package*) all_imports; + Collect_export_references collect(this, &exports, &all_imports); + + // Walk the set of inlinable routine bodies collected above. This + // can potentially expand the exports set. + collect.expand_exports(&check_inline_refs); + // Export the symbols in sorted order. That will reduce cases where // irrelevant changes to the source code affect the exported // interface. @@ -333,10 +591,14 @@ Export::export_globals(const std::string& package_name, std::sort(sorted_exports.begin(), sorted_exports.end(), Sort_bindings()); + // Collect up the set of types mentioned in things we're exporting, + // and any packages that may be referred to indirectly. + collect.prepare_types(); + // Assign indexes to all exported types and types referenced by - // exported types, and collect all packages mentioned. - int unexported_type_index = this->prepare_types(&sorted_exports, - &all_imports); + // things we're exporting. Return value is index of first non-exported + // type. + int unexported_type_index = this->assign_type_indices(sorted_exports); // Although the export data is readable, at least this version is, // it is conceptually a binary format. Start with a four byte @@ -401,175 +663,53 @@ Export::export_globals(const std::string& package_name, this->stream_->write_checksum(s); } -// Traversal class to find referenced types. - -class Find_types_to_prepare : public Traverse -{ - public: - Find_types_to_prepare(Export* exp, - Unordered_set(const Package*)* imports) - : Traverse(traverse_types), - exp_(exp), imports_(imports) - { } - - int - type(Type* type); - - // Traverse the components of a function type. - void - traverse_function(Function_type*); - - // Traverse the methods of a named type, and register its package. - void - traverse_named_type(Named_type*); - - private: - // Exporters. - Export* exp_; - // List of packages we are building. - Unordered_set(const Package*)* imports_; -}; - -// Set type index of referenced type, record package imports, and make -// sure we traverse methods of named types. +// Record a type in the "to be indexed" set. Return true if the type +// was not already in the set, false otherwise. -int -Find_types_to_prepare::type(Type* type) +bool +Export::record_type(Type* type) { - // Skip forwarders; don't try to give them a type index. - if (type->forward_declaration_type() != NULL) - return TRAVERSE_CONTINUE; - - // Skip the void type, which we'll see when exporting - // unsafe.Pointer. The void type is not itself exported, because - // Pointer_type::do_export checks for it. - if (type->is_void_type()) - return TRAVERSE_SKIP_COMPONENTS; - - // Skip abstract types. We should never see these in real code, - // only in things like const declarations. - if (type->is_abstract()) - return TRAVERSE_SKIP_COMPONENTS; - - // For interfaces make sure that embedded methods are sorted, since the - // comparison function we use for indexing types relies on it (this call has - // to happen before the set_type_index call below). - if (type->classification() == Type::TYPE_INTERFACE) - { - Interface_type* it = type->interface_type(); - if (it != NULL) - it->sort_embedded(); - } + type = type->forwarded(); - if (!this->exp_->set_type_index(type)) + std::pair ins = + this->impl_->type_refs.insert(std::make_pair(type, 0)); + if (!ins.second) { // We've already seen this type. - return TRAVERSE_SKIP_COMPONENTS; - } - - // At this stage of compilation traversing interface types traverses - // the final list of methods, but we export the locally defined - // methods. If there is an embedded interface type we need to make - // sure to export that. Check classification, rather than calling - // the interface_type method, because we want to handle named types - // below. - if (type->classification() == Type::TYPE_INTERFACE) - { - Interface_type* it = type->interface_type(); - const Typed_identifier_list* methods = it->local_methods(); - if (methods != NULL) - { - for (Typed_identifier_list::const_iterator p = methods->begin(); - p != methods->end(); - ++p) - { - if (p->name().empty()) - Type::traverse(p->type(), this); - else - this->traverse_function(p->type()->function_type()); - } - } - return TRAVERSE_SKIP_COMPONENTS; + return false; } + ins.first->second = 0; - Named_type* nt = type->named_type(); - if (nt != NULL) - this->traverse_named_type(nt); - - return TRAVERSE_CONTINUE; -} - -// Traverse the types in a function type. We don't need the function -// type itself, just the receiver, parameter, and result types. - -void -Find_types_to_prepare::traverse_function(Function_type* type) -{ - go_assert(type != NULL); - if (this->remember_type(type)) - return; - const Typed_identifier* receiver = type->receiver(); - if (receiver != NULL) - Type::traverse(receiver->type(), this); - const Typed_identifier_list* parameters = type->parameters(); - if (parameters != NULL) - parameters->traverse(this); - const Typed_identifier_list* results = type->results(); - if (results != NULL) - results->traverse(this); + return true; } -// Traverse the methods of a named type, and record its package. +// Assign the specified type an index. void -Find_types_to_prepare::traverse_named_type(Named_type* nt) +Export::set_type_index(const Type* type) { - const Package* package = nt->named_object()->package(); - if (package != NULL) - this->imports_->insert(package); - - // We have to traverse the methods of named types, because we are - // going to export them. This is not done by ordinary type - // traversal. - const Bindings* methods = nt->local_methods(); - if (methods != NULL) - { - for (Bindings::const_definitions_iterator pm = - methods->begin_definitions(); - pm != methods->end_definitions(); - ++pm) - { - Function* fn = (*pm)->func_value(); - this->traverse_function(fn->type()); - if (fn->export_for_inlining()) - fn->block()->traverse(this); - } - - for (Bindings::const_declarations_iterator pm = - methods->begin_declarations(); - pm != methods->end_declarations(); - ++pm) - { - Named_object* mno = pm->second; - if (mno->is_function_declaration()) - this->traverse_function(mno->func_declaration_value()->type()); - } - } + type = type->forwarded(); + std::pair ins = + this->impl_->type_refs.insert(std::make_pair(type, 0)); + go_assert(!ins.second); + int index = this->type_index_; + ++this->type_index_; + go_assert(ins.first->second == 0); + ins.first->second = index; } -// Prepare to export types by assigning a type index to every exported -// type and every type referenced by an exported type. Also collect -// all the packages we see in types, so that if we refer to any types -// from indirectly imported packages we can tell the importer about -// the package. This returns the number of exported types. +// This helper assigns type indices to all types mentioned directly or +// indirectly in the things we're exporting. Actual exported types are given +// indices according to where the appear on the sorted exports list; all other +// types appear afterwards. Return value is the total number of exported types +// plus 1, e.g. the index of the 1st non-exported type. int -Export::prepare_types(const std::vector* exports, - Unordered_set(const Package*)* imports) +Export::assign_type_indices(const std::vector& sorted_exports) { // Assign indexes to all the exported types. - for (std::vector::const_iterator p = exports->begin(); - p != exports->end(); + for (std::vector::const_iterator p = sorted_exports.begin(); + p != sorted_exports.end(); ++p) { if (!(*p)->is_type()) @@ -577,85 +717,34 @@ Export::prepare_types(const std::vector* exports, Interface_type* it = (*p)->type_value()->interface_type(); if (it != NULL) it->sort_embedded(); + this->record_type((*p)->type_value()); this->set_type_index((*p)->type_value()); } - int ret = this->type_index_; - // Use a single instance of the traversal class because traversal - // classes keep track of which types they've already seen. That - // lets us avoid type reference loops. - Find_types_to_prepare find(this, imports); - - // Traverse all the exported objects and assign indexes to all types. - for (std::vector::const_iterator p = exports->begin(); - p != exports->end(); + // Collect export-referenced, non-builtin types. + std::vector types; + types.reserve(this->impl_->type_refs.size()); + for (Type_refs::const_iterator p = this->impl_->type_refs.begin(); + p != this->impl_->type_refs.end(); ++p) { - Named_object* no = *p; - switch (no->classification()) - { - case Named_object::NAMED_OBJECT_CONST: - { - Type* t = no->const_value()->type(); - if (t != NULL && !t->is_abstract()) - Type::traverse(t, &find); - } - break; - - case Named_object::NAMED_OBJECT_TYPE: - Type::traverse(no->type_value()->real_type(), &find); - find.traverse_named_type(no->type_value()); - break; - - case Named_object::NAMED_OBJECT_VAR: - Type::traverse(no->var_value()->type(), &find); - break; - - case Named_object::NAMED_OBJECT_FUNC: - { - Function* fn = no->func_value(); - find.traverse_function(fn->type()); - if (fn->export_for_inlining()) - fn->block()->traverse(&find); - } - break; - - case Named_object::NAMED_OBJECT_FUNC_DECLARATION: - find.traverse_function(no->func_declaration_value()->type()); - break; - - default: - // We shouldn't see anything else. If we do we'll give an - // error later when we try to actually export it. - break; - } + const Type* t = p->first; + if (p->second != 0) + continue; + types.push_back(t); } - return ret; -} - -// Give a type an index if it doesn't already have one. Return true -// if we set the type index, false if it was already known. - -bool -Export::set_type_index(Type* type) -{ - type = type->forwarded(); - - std::pair ins = - this->impl_->type_refs.insert(std::make_pair(type, 0)); - if (!ins.second) - { - // We've already seen this type. - return false; - } + // Sort the types. + std::sort(types.begin(), types.end(), Sort_types()); - int index = this->type_index_; - ++this->type_index_; - ins.first->second = index; + // Assign numbers to the sorted list. + for (std::vector::const_iterator p = types.begin(); + p != types.end(); + ++p) + this->set_type_index((*p)); - return true; + return ret; } // Sort packages. diff --git a/gcc/go/gofrontend/export.h b/gcc/go/gofrontend/export.h index 74bdd9415b6..1af386c1755 100644 --- a/gcc/go/gofrontend/export.h +++ b/gcc/go/gofrontend/export.h @@ -160,9 +160,15 @@ class Export : public String_dump const Import_init_set& imported_init_fns, const Bindings* bindings); - // Set the index of a type. + // Record a type that is mentioned in export data. Return value is + // TRUE for newly visited types, FALSE for types that have been seen + // previously. bool - set_type_index(Type*); + record_type(Type*); + + // Assign type indices to types mentioned in export data. + int + assign_type_indices(const std::vector& sorted_exports); // Write a string to the export stream. void @@ -213,11 +219,6 @@ class Export : public String_dump Export(const Export&); Export& operator=(const Export&); - // Prepare types for exporting. - int - prepare_types(const std::vector* exports, - Unordered_set(const Package*)* imports); - // Write out all known packages. void write_packages(const std::map& packages); @@ -258,6 +259,10 @@ class Export : public String_dump int type_index(const Type*); + // Set the index of a type. + void + set_type_index(const Type*); + // The stream to which we are writing data. Stream* stream_; // Index number of next type. diff --git a/gcc/go/gofrontend/gogo.h b/gcc/go/gofrontend/gogo.h index e50314b6209..84b6e8e37d0 100644 --- a/gcc/go/gofrontend/gogo.h +++ b/gcc/go/gofrontend/gogo.h @@ -914,7 +914,7 @@ class Gogo // Return the name for a type descriptor symbol. std::string - type_descriptor_name(Type*, Named_type*); + type_descriptor_name(const Type*, Named_type*); // Return the name of the type descriptor list symbol of a package. std::string diff --git a/gcc/go/gofrontend/names.cc b/gcc/go/gofrontend/names.cc index e109cfc6e88..c622067c220 100644 --- a/gcc/go/gofrontend/names.cc +++ b/gcc/go/gofrontend/names.cc @@ -936,7 +936,7 @@ Named_type::append_mangled_type_name(Gogo* gogo, bool use_alias, // it is the name to use. std::string -Gogo::type_descriptor_name(Type* type, Named_type* nt) +Gogo::type_descriptor_name(const Type* type, Named_type* nt) { // The type descriptor symbol for the unsafe.Pointer type is defined // in libgo/runtime/go-unsafe-pointer.c, so just use a reference to