Skip to content

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented Sep 3, 2017

Rework how we handle nested-type-name-match constraints to fix several problems with generic signature minimization involving recursive constraints:

  • When the two nested types of a given potential archetype have the same name, introduce a (quietly) inferred constraint rather than considering the equivalence derived. This allows us future growth if we want to stop inferring the constraint, e.g., because two unrelated protocols with same-named associated types need to be composed (which we don't currently support).
  • Detect self-derived nested-type-name-match constraints so we can remove them appropriately; without this change, the self-derived constraints would end up squashing important constraints, leaving to incorrect (overly-minimized) generic signatures.
  • Improve the computation of connected components for same-type constraints to minimize same-type constraints.
  • Kill off one underscored protocol in the standard library (_UnicodeEncoding_).

Fixes SR-5841, SR-5601 and SR-5610.

@DougGregor
Copy link
Member Author

@swift-ci please test source compatibility

@DougGregor
Copy link
Member Author

@huonw you might be interested in how this turned out, since you've been helping with the self-derived checking algorithm all along

@DougGregor
Copy link
Member Author

Well, you might be more interested if it actually worked :)

@DougGregor
Copy link
Member Author

Should also fix SR-5841.

@DougGregor DougGregor force-pushed the gsb-nested-same-type-match branch 2 times, most recently from 842986e to 986df37 Compare September 11, 2017 14:13
@DougGregor
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - b4919e655fabb9163c59e198470efb7e84146d9a

@DougGregor
Copy link
Member Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 986df37

When we check type inferred type witnesses against the requirement
signature of the protocol, first "sanitize" the requirements to remap
any dependent member types into the equivalent dependent member types
within that protocol, dropping any requirements that can't be
remapped. This eliminates some extraneous recursion introduced by
checking requirements, and isolates us somewhat from the specific
choice of "canonical" associated types.
…ial archetype.

When we have two nested types of a given potential archetype that have
the same name, introduce a (quietly) inferred constraint. This is
a future-proofing step for canonicalization, for a possible future
where we no longer require all of the nested types of a given name
to be equivalent, e.g., because we have a proper disambiguation
mechanism.
Nested-type-name-match constraints are odd because they are
effectively artifacts of the GenericSignatureBuilder's algorithm,
i.e., they occur when we have two PotentialArchetypes representing the
same type, and each of those PA's has a nested type based on the same
associated type. Because of this, nested-type-name-match constraints
have no useful requirement sources, so the normal means of detecting
self-derived constraints doesn't suffice, and we end up with
self-derived nested-type-name-match constraints eliminating the
constraints they depend on, causing spurious "redundant same-type
constraint" diagnostics and minimized generic signatures that drop
important requirements.

Handle nested-type-name-match constraints by first keeping them out of
the connected-components algorithm used to compute same-type
constraints within an equivalence class. Then, detect self-derived
nested-type-name-match constraints by determining whether any of the
ancestor potential archetypes are in the same equivalence class... and
redundant with the edge that makes the ancestor potential archetypes
equivalent. Remove such self-derived edges, and treat all other
nested-type-name-match edges as derived, providing a minimized
signature.

Fixes SR-5841, SR-5601, and SR-5610
…a conformance.

NFC for now; we are going to re-use this code.
…ponents.

Same-type constraints are rederived based on the potential archetypes
within an equivalence class and the same-type constraints that link
them. In some cases, there may be parts that connect those same-type
constraints that are stored in "delayed" requirements (for which one
or both end-points have not been realized in a potential archetype);
consider those as well for connectedness.

In a sense, this is an artifact of the generic signature builder's
approach of realizing potential archetypes too readily, and this can
likely be simplified in the future. For now, it's important for the
minimization of same-type constraints in generic signatures.
Perform the actual "collapse" of same-type components by looking
through the delayed requirements of an equivalence class.
When there are to "structurally" equivalent potential archetypes in
different components of an equivalence class, as determined by their
nested-name structure, collapse the two components. This addresses the
remaining known issues due to the eager explosion of potential
archetypes in the generic signature builder.
@DougGregor DougGregor force-pushed the gsb-nested-same-type-match branch from 986df37 to c428f36 Compare September 12, 2017 13:25
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

@DougGregor DougGregor changed the title [WIP] [GSB] Rework nested-type-name-match constraints. [GSB] Rework nested-type-name-match constraints. Sep 12, 2017
This protocol was only used to fake recursive protocol conformances;
collapse it into _UnicodeEncoding.
@DougGregor DougGregor force-pushed the gsb-nested-same-type-match branch from c428f36 to 3b47b16 Compare September 12, 2017 13:50
@DougGregor
Copy link
Member Author

Drat, a line for the removal of _UnicodeEncoding_ got mis-attributed.

@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

1 similar comment
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants