compiler: compare parse methods when indexing interface types for export
This change fixes a bug in which two interface types were being
incorrectly commoned (considered identical) in the initial stages of
writing out types to export data. The indexer does a walk to collect
candidates for export, inserting types into a table to eliminate
duplicates; as part of this process a local interface type T1 was
being commoned with a different interface type T2. This caused a cycle
in the exported type graph due to the way embedded interfaces are
handled.
The fix was to add a new flag to the Type::is_identical utility
routine to request that interface type comparison be done by examining
the original parse methods, as opposed to the expanded method set,
then use the new flag when creating the hash map for the exporter.
Fixes golang/go#30659.
Change-Id: I32efbbee0e66f5d8cdc1cf711011de2c3cd829cf
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/166638
Reviewed-by: Ian Lance Taylor <iant@golang.org>
diff --git a/go/export.cc b/go/export.cc
index d021ac4..963f0bf 100644
--- a/go/export.cc
+++ b/go/export.cc
@@ -60,6 +60,7 @@
return type->hash_for_method(NULL,
(Type::COMPARE_ERRORS
| Type::COMPARE_TAGS
+ | Type::COMPARE_EMBEDDED_INTERFACES
| Type::COMPARE_ALIASES));
}
};
@@ -73,6 +74,7 @@
return Type::are_identical(t1, t2,
(Type::COMPARE_ERRORS
| Type::COMPARE_TAGS
+ | Type::COMPARE_EMBEDDED_INTERFACES
| Type::COMPARE_ALIASES),
NULL);
}
@@ -295,6 +297,16 @@
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();
+ }
+
if (!this->exp_->set_type_index(type))
{
// We've already seen this type.
@@ -408,6 +420,9 @@
{
if (!(*p)->is_type())
continue;
+ Interface_type* it = (*p)->type_value()->interface_type();
+ if (it != NULL)
+ it->sort_embedded();
this->set_type_index((*p)->type_value());
}
diff --git a/go/types.cc b/go/types.cc
index e12e670..e9cbfd8 100644
--- a/go/types.cc
+++ b/go/types.cc
@@ -8808,10 +8808,19 @@
if (!this->methods_are_finalized_ || !t->methods_are_finalized_)
return false;
+ // Consult a flag to see whether we need to compare based on
+ // parse methods or all methods.
+ Typed_identifier_list* methods = (((flags & COMPARE_EMBEDDED_INTERFACES) != 0)
+ ? this->parse_methods_
+ : this->all_methods_);
+ Typed_identifier_list* tmethods = (((flags & COMPARE_EMBEDDED_INTERFACES) != 0)
+ ? t->parse_methods_
+ : t->all_methods_);
+
// We require the same methods with the same types. The methods
// have already been sorted.
- if (this->all_methods_ == NULL || t->all_methods_ == NULL)
- return this->all_methods_ == t->all_methods_;
+ if (methods == NULL || tmethods == NULL)
+ return methods == tmethods;
if (this->assume_identical(this, t) || t->assume_identical(t, this))
return true;
@@ -8823,11 +8832,11 @@
ai.next = hold_ai;
this->assume_identical_ = &ai;
- Typed_identifier_list::const_iterator p1 = this->all_methods_->begin();
+ Typed_identifier_list::const_iterator p1 = methods->begin();
Typed_identifier_list::const_iterator p2;
- for (p2 = t->all_methods_->begin(); p2 != t->all_methods_->end(); ++p1, ++p2)
+ for (p2 = tmethods->begin(); p2 != tmethods->end(); ++p1, ++p2)
{
- if (p1 == this->all_methods_->end())
+ if (p1 == methods->end())
break;
if (p1->name() != p2->name()
|| !Type::are_identical(p1->type(), p2->type(), flags, NULL))
@@ -8836,7 +8845,7 @@
this->assume_identical_ = hold_ai;
- return p1 == this->all_methods_->end() && p2 == t->all_methods_->end();
+ return p1 == methods->end() && p2 == tmethods->end();
}
// Return true if T1 and T2 are assumed to be identical during a type
diff --git a/go/types.h b/go/types.h
index 976d41a..07121dd 100644
--- a/go/types.h
+++ b/go/types.h
@@ -577,6 +577,11 @@
// Compare aliases: treat an alias to T as distinct from T.
static const int COMPARE_ALIASES = 4;
+ // When comparing interface types compare the interface embedding heirarchy,
+ // if any, rather than only comparing method sets. Useful primarily when
+ // exporting types.
+ static const int COMPARE_EMBEDDED_INTERFACES = 8;
+
// Return true if two types are identical. If this returns false,
// and REASON is not NULL, it may set *REASON.
static bool
@@ -3165,6 +3170,15 @@
methods_are_finalized() const
{ return this->methods_are_finalized_; }
+ // Sort embedded interfaces by name. Needed when we are preparing
+ // to emit types into the export data.
+ void
+ sort_embedded()
+ {
+ if (parse_methods_ != NULL)
+ parse_methods_->sort_by_name();
+ }
+
protected:
int
do_traverse(Traverse*);
