I'm looking at a bug where we end with a signature like
protocol B { ... }
protocol BB : B { }
<Self where Self : B, Self : BB>
While this one be a one-off bug, it's easy enough to check for this
condition with an assert here.
This enabled a gross idiom that should not have been allowed in the first place:
typealias G<T> = Any where T : P
protocol P {}
protocol Q : G<Self> {} // Q inherits from P now!
I'd like to ban this, assuming nothing is actually relying on this behavior.
Returning a null GenericSignature is not the right way to break a cycle,
because then callers have to be careful to handle the case of a null
GenericSignature together with a non-null GenericParamList, for example
in applyGenericArguments().
An even worse problem can occur when a GenericSignatureRequest for a
nested generic declaration requests the signature of the parent context,
which hits a cycle. In this case, we would build a signature where
the first generic parameter did not have depth 0.
This makes the requirement machine upset, so this patch implements a new
strategy to break such cycles. Instead of returning a null
GenericSignature, we build a signature with the correct generic
parameters, but no requirements. The generic parameters can be computed
just by traversing GenericParamLists, which does not trigger more
GenericSignatureRequests, so this should be safe.
I'm about to fix the same bug in the RequirementMachine, and to avoid spurious cross-checking
failures in -requirement-machine=verify mode, just fix this in the GSB as well.
After we drop redundant conformance requirements, the left hand side
of a concrete same-type requirement might become unresolvable:
protocol P {
associatedtype T where T == Self
}
struct S : P {}
extension P where T == S {}
Here, we begin with <Self where Self : P, Self.T == S>, and then we
drop (Self : P). However, <Self where Self.T == S> is no longer a
valid generic signature.
We can canonicalize Self.T down to Self before we build the new
signature, but we must only do this for concrete same-type requirements,
since canonicalizing the subject type of an abstract same-type
requirement might lose information produce a trivial requirement of
the form 'T == T'.
This is really unprincipled, and no doubt other counter-examples
exist. The entire procedure for rebuilding a generic signature needs
to be re-designed from first principles.
Fixes rdar://problem/80503090.
A protocol conformance requirement together with a superclass requirement
can collapse down to a same-type requirement if the protocol itself has
a 'where' clause:
protocol P {
associatedtype T where T == Self
}
class C : P {}
extension P where T : C {}
(Self : P) and (Self.T : C) imply that (Self == C), because protocol P
says that Self.T == Self, and the witness of P.T in C is C, so when we
substitute that in we get Self == C.
Part of rdar://problem/80503090.
Generally we say that a conformance requirement in a generic signature
is redundant if there is some other way to derive this conformance by
starting from another conformance requirement in the same signature,
and possibly following one or more conformance requirements on nested
types defined in protocols.
The notion of a 'valid derivation path' comes into play when you have
something like this:
protocol P {
associatedtype A
}
protocol Q {
associatedtype B : P
}
<T where T : P, T.A : P, T.A.B == T>
Here, we don't want to conclude that (T : P) can be derived from
(T.A : P)(Self.B : P), because if we drop (T : P) from the signature,
we end up with
<T where T.A : P, T.A.B == T>
Now in order to recover the witness table for T : P, we need to start
from T.A : P, which requires the type metadata for T.A, which can
only be recovered from the witness table for T : P, etc. We're stuck.
What we want to do is say that T : P is not redundant, because we
cannot derive it from T.A : P, because T.A depends on T : P.
However, this check was also too strict. Consider this example:
protocol P {
associatedtype T where T == Self
}
protocol Q : P {}
<T where T : P, T.T : Q>
The naive algorithm would conclude that T : P is redundant because
it can be derived as (T.T : Q)(Self.T == Self). However, the valid
derivation path check would fail since T.T is derived from (T : P).
The problem is that since T.T is equivalent to T via (Self.T == T),
we would end up with this minimized signature:
<T where T : P, T : Q>
The derivation path check should canonicalize the type first.
I'm still not 100% convinced this logic is correct, but at least,
we have another test case and maybe it's _more_ correct now.
Fixes part of rdar://problem/80503090.
Start treating the null {Can}GenericSignature as a regular signature
with no requirements and no parameters. This not only makes for a much
safer abstraction, but allows us to simplify a lot of the clients of
GenericSignature that would previously have to check for null before
using the abstraction.
If we don't set this flag, we can end up making an invalid GSB into
the canonical builder for some signature. This was caught by
requirement machine cross-checking on the compiler_crashers suite.
When merging two type parameters T1 and T2, if T2 had a concrete type
and T1 had conformance requirements, we did not "concretize" the
conformances.
As a result, the conformance requirements were not marked as redundant,
which would cause a crash (no assert build) or assertion failure (in an
assert build) later on inside rebuildSignatureWithoutRedundantRequirements().
Fixes rdar://problem/79570734.
When a protocol's requirement signature is being computed by
GenericSignatureBuilder::computeGenericSignature(), we call
checkGenericSignature(), which contains various assertions
that call GenericSignature::isCanonicalTypeInContext().
One of these assertions forgot to pass the GSB instance down
as the 'builder' parameter.
The end result is that we would create a new GSB from the
protocol's requirement signature, which is generally not
well-formed since it does not have a conformance requirement
on 'Self'.
It seems this was harmless, other than the wasted CPU cycles,
but it was caught by some RequirementMachine assertions.
We rebuild a generic signature after dropping conformance requirements
made redundant by a superclass or concrete type requirement.
When rebuilding the signature, preserve the source locations of the
original requirements, and only perform diagnostics on the rebuilt
signature.
This fixes an issue where we would emit a redundant requirement
warning even though the requirement in question was not actually
redundant.
This also avoids some unnecessary work. Most of the code in
finalize() does not need to be run twice, once before and once after
rebuilding the signature. Now we only run it after rebuilding the
signature.
Note that this regresses diagnostics in one narrow case where we
would previously diagnose a conflicting concrete type requirement.
This will be fixed once concrete type diagnostics are moved over
to use the new ExplicitRequirement infrastructure, just like all
other kinds already do.
Fixes rdar://problem/77462797.
I found this by inspection. isExplicit() returns false for inferred
requirements, which is probably not what we want here. Using isDerived()
seems better instead.
In a few places we call getMinimalConformanceSource() while iterating
over all equivalence classes, or the conformances of a specific
equivalence class.
In both cases, getMinimalConformanceSource() can modify the storage
of the collection being iterated over, because it can end up calling
maybeResolveEquivalenceClass(), which can introduce new constraints.
Work around this by iterating a copy of the list of equivalence
classes first, calling getMinimalConformanceSource() and discarding
the result on each conformance source.
This is a terrible workaround, but should address the issue in the
meantime until I finish removing getMinimalConformanceSource().
Fixes rdar://problem/46420886 / https://bugs.swift.org/browse/SR-9398,
rdar://problem/77807692.
In a protocol requirement signature, a conformance requirement on 'Self'
can only be considered redundant if it is derived entirely from other
sources only involving 'Self' as a subject type.
What this means in practice is that we will never try to build
a witness table for an inherited conformance from an associated
conformance.
This is important since witness tables for inherited conformances
are realized eagerly before the witness table for the original
conformance, and allowing more complex requirement sources for
inherited conformances could introduce cycles at runtime.
What used to happen is that we would emit a bogus 'redundant conformance'
warning if a refined protocol conformance was also reachable via a
same-type requirement involving 'Self'. Fixing this warning by removing
the requirement could produce code that type checked, however it could
crash at runtime.
In addition to the runtime issue, such 'overly minimized' protocols
could also break name lookup at compile-time, since name lookup
only consults the inheritance clause of a protocol declaration instead
of computing its generic signature to determine the set of refined
protocols.
Now, we will no longer emit the bogus redundant conformance warnings.
Instead, we produce a warning if a protocol is 'overly minimized'.
Since the 'overly minimized' protocols that were obtained by fixing
the bogus warning would in some cases still work, we cannot diagnose
the new violation as an error, to preserve source and ABI compatibility.
Currently this warning fires for the SIMDScalar protocol in the
standard library, and _ErrorCodeProtocol in the Foundation overlay.
We should see if we can fix the protocols in an ABI-compatible way,
or just add a hack to muffle the warning.
Fixes https://bugs.swift.org/browse/SR-8198 / rdar://problem/77358480,
https://bugs.swift.org/browse/SR-10941 / rdar://problem/77358477,
https://bugs.swift.org/browse/SR-11670 / rdar://problem/77358476.
Consider a signature with a conformance requirement, and two
identical superclass requirements, both of which make the
conformance requirement redundant.
The conformance will have two requirement sources, the explicit
source and a derived source based on one of the two superclass
requirements, whichever one was processed first.
If we end up marking the *other* superclass requirement as
redundant, we would incorrectly conclude that the conformance
was not redundant. Instead, if a derived source is based on a
redundant requirement, we can't just discard it right away;
instead, we have to check if that source could have been
derived some other way.
This used to be necessary to handle this case:
protocol P1 {
associatedtype A
}
protocol P2 {
associatedtype A : P1
}
func foo<T : P1>(_: T) where T.A : P2, T.A.A == T {}
We don't want to mark 'T : P1' as redundant, even though it could
be recovered from T.A.A, because it uses the requirement T.A, and
we cannot recover the type metadata for T.A without the witness
table for T : P1.
Now this is handled via a more general mechanism, so we no longer
need this hack.
Consider this example:
protocol P1 {
associatedtype A : P2
}
protocol P2 {
associatedtype A
}
func foo<T : P2>(_: T) where T.A : P1, T.A.A == T {}
We cannot drop 'T : P2', even though it can be derived from
T.A.A == T and Self.A : P2 in protocol P1, because we actually
need the conformance T : P2 in order to recover the concrete
type for T.A.
This was handled via a hack which would ensure that the
conformance path (T.A : P1)(Self.A : P2) was not a candidate
derivation for T : P2, because T : P2 is one of the conformances
used to construct the subject type T.A in the conformance path.
This hack did not generalize to "cycles" of length greater than 1
however:
func foo<T : P2, U : P2>(_: T, _: U)
where T.A : P1, T.A.A == U, U.A : P1, U.A.A == T {}
We used to drop both T : P2 and U : P2, because each one had a
conformance path (U.A : P1)(Self.A : P2) and (T.A : P1)(Self.A : P2),
respectively; however, once we drop T : P2 and U : P2, these two
paths become invalid for the same reason that the concrete type
for T.A and U.A can no longer be recovered.
The correct fix is to recursively visit the subject type of each
base requirement when evaluating a conformance path, and not
consider any requirement whose subject type's parent type cannot
be recovered. This proceeds recursively.
In the worst case, this algorithm is exponential in the number of
conformance requirements, but it is not always exponential, and
a generic signature is not going to have a large number of
conformance requirements anyway (hopefully). Also, the old logic
in getMinimalConformanceSource() wasn't cheap either.
Perhaps some clever minimization can speed this up too, but I'm
going to focus on correctness first, before looking at performance
here.
Fixes rdar://problem/74890907.
Literally an off-by-one error -- we were skipping over the first
requirement and not copying it over. I think this is because the
updateLayout() call used to be addLayoutRequirementDirect(),
which would add the first layout constraint, and I forgot to
remove the '+ 1' when I refactored this.
This fixes a regression from the new redundant requirements algorithm
and paves the way for removing the notion of 'derived via concrete'
requirements.
This rewrites the existing redundant requirements algorithm
to be simpler, and fix an incorrect behavior in the case where
we're building a protocol requirement signature.
Consider the following example:
protocol P {
associatedtype A : P
associatedtype B : P where A.B == B
}
The requirement B : P has two conformance paths here:
(B : P)
(A : P)(B : P)
The naive redundancy algorithm would conclude that (B : P) is
redundant because it can be derived as (A : P)(B : P). However,
if we drop (B : P), we lose the derived conformance path
as well, since it involves the same requirement (B : P).
The above example actually worked before this change, because we
handled this case in getMinimalConformanceSource() by dropping any
derived conformance paths that involve the requirement itself
appearing in the "middle" of the path.
However, this is insufficient because you can have a "cycle"
here with length more than 1. For example,
protocol P {
associatedtype A : P where A == B.C
associatedtype B : P where B == A.C
associatedtype C : P where C == A.B
}
The requirement A : P has two conformance paths here:
(A : P)
(B : P)(C : P)
Similarly, B : P has these two paths:
(B : P)
(A : P)(C : P)
And C : P has these two paths:
(C : P)
(A : P)(B : P)
Since each one of A : P, B : P and C : P has a derived conformance
path that does not involve itself, we would conclude that all three
were redundant. But this was wrong; while (B : P)(C : P) is a valid
derived path for A : P that allows us to drop A : P, once we commit to
dropping A : P, we can no longer use the other derived paths
(A : P)(C : P) for B : P, and (A : P)(B : P) for C : P, respectively,
because they involve A : P, which we dropped.
The problem is that we were losing information here. The explicit
requirement A : P can be derived as (B : P)(C : P), but we would
just say that it was implied by B : P alone.
For non-protocol generic signatures, just looking at the root is
still sufficient.
However, when building a requirement signature of a self-recursive
protocol, instead of looking at the root explicit requirement only,
we need to look at _all_ intermediate steps in the path that involve
the same protocol.
This is implemented in a new getBaseRequirements() method, which
generalizes the operation of getting the explicit requirement at
the root of a derived conformance path by returning a vector of
one or more explicit requirements that appear in the path.
Also the new algorithm computes redundancy online instead of building
a directed graph and then computing SCCs. This is possible by
recording newly-discovered redundant requirements immediately,
and then using the set of so-far-redundant requirements when
evaluating a path.
This commit introduces a small regression in an existing test case
involving a protocol with a 'derived via concrete' requirement.
Subsequent commits in this PR fix the regression and remove the
'derived via concrete' mechanism, since it is no longer necessary.
Fixes https://bugs.swift.org/browse/SR-14510 / rdar://problem/76883924.
The new approach is to not look at RequirementSources at all. Instead,
we exhaustively enumerate all conformance access paths, beginning
from the root conformance requirements in the signature, then doing
all conformance requirements from those protocols' requirement
signatures, and so on.
We enumerate conformance access paths in breadth first order by
length until we find the one we want. The results are memoized.
This fixes a regression with another change I'm working on. The
test case does not fail with this PR alone, but I'm adding it now
anyway.
