Skip to content

GSB: Formalize the old hack where we rebuild a signature that had redundant conformance requirements #36454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Mar 16, 2021

When constructing a generic signature, any redundant explicit requirements
are dropped from the final signature.

We would assume this operation is idempotent, that is, building a new
GenericSignatureBuilder from the resulting minimized signature produces
an equivalent GenericSignatureBuilder to the original one.

Unfortunately, this is not true in the case of conformance requirements.

Namely, if a conformance requirement is made redundant by a superclass
or concrete same-type requirement, then dropping the conformance
requirement changes the canonical type computation.

For example, consider the following:

public protocol P {
    associatedtype Element
}

public class C<O: P>: P {
    public typealias Element = O.Element
}

public func toe<T, O, E>(_: T, _: O, _: E, _: T.Element)
    where T : P, O : P, O.Element == T.Element, T : C<E> {}

In the generic signature of toe(), the superclass requirement 'T : C'
implies the conformance requirement 'T : P' because C conforms to P.

However, the presence of the conformance requirement makes it so that
T.Element is the canonical representative, so previously this signature
was minimized down to:

<T : C<E>, O : P, T.Element == O.Element>

If we build the signature again from the above requirements, then we
see that T.Element is no longer the canonical representative; instead,
T.Element canonicalizes as E.Element.

For this reason, we must rebuild the signature to get the correct
canonical type computation.

I realized that this is not an artifact of incorrect design in the
current GSB; my new rewrite system formalism would produce the same
result. Rather, it is a subtle consequence of the specification of our
minimization algorithm, and therefore it must be formalized in this
manner.

We used to sort-of do this with the HadAnyRedundantRequirements hack,
but it was both overly broad (we only need to rebuild if a conformance
requirement was implied by a superclass or concrete same-type
requirement) and not sufficient (when rebuilding, we need to strip any
bound associated types from our requirements to ensure the canonical
type anchors are re-computed).

Fixes rdar://problem/65263302, rdar://problem/75010156,
rdar://problem/75171977.

@slavapestov slavapestov requested a review from DougGregor March 16, 2021 21:29
@slavapestov slavapestov force-pushed the rebuild-signature-without-redundant-conformances branch 2 times, most recently from 16f894a to 509592f Compare March 16, 2021 21:34
@slavapestov
Copy link
Contributor Author

This depends on #36460 first landing.

@slavapestov slavapestov force-pushed the rebuild-signature-without-redundant-conformances branch from 509592f to 23863e5 Compare March 17, 2021 04:25
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov force-pushed the rebuild-signature-without-redundant-conformances branch from 23863e5 to b19f22a Compare March 17, 2021 05:52
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

…rement implied a redundant requirement

Recall the meaning of an explicit requirement here:

- If the requirement is part of a root SCC, it is redundant
  unless it is the 'best' requirement from that root SCC.

- If the requirement is part of a non-root SCC, it is always
  redundant.

Instead of computing the set of redundant requirements, we
build a mapping.

The value in the mapping stores the set of root requirements
that imply the given redundant requirement. This mapping is
computed by traversing the graph from each root, recording
which components can be reached from each root.

For now, I'm using this to fix rdar://problem/65263302.

After fixing that bug, this will also allow us to radically
simplify the various callers of checkConstraintList().
@slavapestov slavapestov force-pushed the rebuild-signature-without-redundant-conformances branch from b19f22a to f1e517e Compare March 17, 2021 21:11
…undant conformance requirements

When constructing a generic signature, any redundant explicit requirements
are dropped from the final signature.

We would assume this operation is idempotent, that is, building a new
GenericSignatureBuilder from the resulting minimized signature produces
an equivalent GenericSignatureBuilder to the original one.

Unfortunately, this is not true in the case of conformance requirements.

Namely, if a conformance requirement is made redundant by a superclass
or concrete same-type requirement, then dropping the conformance
requirement changes the canonical type computation.

For example, consider the following:

    public protocol P {
        associatedtype Element
    }

    public class C<O: P>: P {
        public typealias Element = O.Element
    }

    public func toe<T, O, E>(_: T, _: O, _: E, _: T.Element)
        where T : P, O : P, O.Element == T.Element, T : C<E> {}

In the generic signature of toe(), the superclass requirement 'T : C<E>'
implies the conformance requirement 'T : P' because C conforms to P.

However, the presence of the conformance requirement makes it so that
T.Element is the canonical representative, so previously this signature
was minimized down to:

    <T : C<E>, O : P, T.Element == O.Element>

If we build the signature again from the above requirements, then we
see that T.Element is no longer the canonical representative; instead,
T.Element canonicalizes as E.Element.

For this reason, we must rebuild the signature to get the correct
canonical type computation.

I realized that this is not an artifact of incorrect design in the
current GSB; my new rewrite system formalism would produce the same
result. Rather, it is a subtle consequence of the specification of our
minimization algorithm, and therefore it must be formalized in this
manner.

We used to sort-of do this with the HadAnyRedundantRequirements hack,
but it was both overly broad (we only need to rebuild if a conformance
requirement was implied by a superclass or concrete same-type
requirement) and not sufficient (when rebuilding, we need to strip any
bound associated types from our requirements to ensure the canonical
type anchors are re-computed).

Fixes rdar://problem/65263302, rdar://problem/75010156,
rdar://problem/75171977.
@slavapestov slavapestov force-pushed the rebuild-signature-without-redundant-conformances branch from bab3ed4 to 504d4f5 Compare March 17, 2021 21:26
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test Linux

@slavapestov slavapestov merged commit 891e053 into swiftlang:main Mar 18, 2021
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.

1 participant