Commit Graph

753 Commits

Author SHA1 Message Date
Slava Pestov
ad89fe5179 GSB: Diagnose redundant requirements on the rebuilt signature
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.
2021-05-25 21:50:54 -04:00
Slava Pestov
c746758c6e GSB: Don't infer requirements from conditional conformances when rebuilding a signature 2021-05-25 18:54:17 -04:00
Slava Pestov
3378ff953b GSB: Store explicit same-type requirements as ExplicitRequirements also 2021-05-25 18:41:09 -04:00
Slava Pestov
45dabb1b9c GSB: RequirementSources can just store a SourceLoc instead of a pointer union of TypeRepr/RequirementRepr 2021-05-25 18:41:09 -04:00
Slava Pestov
082b49a205 GSB: More accurate recursion check in GenericSignatureBuilder::resolve()
I found this by inspection. isExplicit() returns false for inferred
requirements, which is probably not what we want here. Using isDerived()
seems better instead.
2021-05-25 18:41:09 -04:00
Slava Pestov
18df1c19ac GSB: Simplify representation of FloatingRequirementSource 2021-05-25 18:41:09 -04:00
Slava Pestov
b717138c8e GSB: Simplify addConditionalRequirements() 2021-05-25 18:41:09 -04:00
Slava Pestov
4166d76f5e GSB: Workaround interator invalidation issue with getMinimalConformanceSource()
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.
2021-05-19 23:11:42 -04:00
Slava Pestov
af6ec5a871 GSB: When looking at protocol refinements, only consider other sources on 'Self'
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.
2021-05-17 21:55:11 -04:00
Slava Pestov
e45b566642 GSB: Try harder to prove derivation via redundant requirements
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.
2021-05-07 18:43:57 -04:00
Slava Pestov
3c136c00ea GSB: Remove obsolete hack for well-founded requirements
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.
2021-05-07 18:43:52 -04:00
Slava Pestov
f6359e9d6f GSB: Replace 'self derived' check with a more sound notion of well-foundedness
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.
2021-05-07 18:43:52 -04:00
Slava Pestov
2d77260d1a GSB: Clean up and comment redundant requirement algorithm some more 2021-05-07 18:43:52 -04:00
Slava Pestov
aa865a2133 GSB: Stop computing 'derived via concrete' sources 2021-05-07 18:43:52 -04:00
Slava Pestov
710621c2eb GSB: Remove checkConformanceConstraints() 2021-05-07 18:43:52 -04:00
Slava Pestov
a10c5ed8a6 GSB: Remove checkSuperclassConstraints() and checkLayoutConstraints() 2021-05-07 18:43:52 -04:00
Slava Pestov
4bf239827e GSB: Fix off-by-one error when merging layout requirements of two equivalence classes
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.
2021-05-07 18:43:52 -04:00
Slava Pestov
d311549246 GSB: Rebuild requirement signatures after dropping redundant conformance requirements
This fixes a regression from the new redundant requirements algorithm
and paves the way for removing the notion of 'derived via concrete'
requirements.
2021-05-07 18:43:52 -04:00
Slava Pestov
a49b93c61a GSB: New redundant requirements algorithm
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.
2021-05-07 18:43:52 -04:00
Slava Pestov
ac3cbe5858 GSB: Add a counter and clean up getConformanceAccessPath() a bit 2021-05-07 18:43:52 -04:00
Slava Pestov
e6ff771d59 GSB: Rewrite getConformanceAccessPath(), again
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.
2021-05-06 17:55:43 -04:00
Slava Pestov
826ba58880 GSB: More useful EquivalenceClass::dump() 2021-04-29 23:35:32 -04:00
Slava Pestov
930909f5b2 GSB: Clean up ExplicitRequirement a bit 2021-04-29 23:35:14 -04:00
Slava Pestov
35a2687afe GSB: Pass down requirement signature protocol to computeGenericSignature() 2021-04-29 23:35:14 -04:00
Slava Pestov
4652f06416 GSB: Extract out ExplicitRequirement::getSubjectType() 2021-04-29 23:35:14 -04:00
Slava Pestov
3296032f19 GSB: When rebuilding a signature, drop layout requirements implied by concrete as well
Otherwise, when we resolve the subject type while adding the requirement
to the new signature, we assert that it is not a dependent type.

This fixes a regression from 977d3a77cf.

Fixes rdar://problem/76750100.
2021-04-19 22:55:01 -04:00
Ben Barham
c54c9c7079 [Gardening] Extract basic source info structs from RawComment.h 2021-04-14 10:05:27 +10:00
Slava Pestov
e8c4418e57 GSB: We only care about 'derived from concrete' requirements when building requirement signatures
Once we can rebuild requirement signatures after dropping redundant
requirements, we won't need this at all.
2021-04-08 01:25:41 -04:00
Slava Pestov
977d3a77cf GSB: Narrow down the condition under which we rebuild a generic signature 2021-04-08 01:25:41 -04:00
Slava Pestov
bcee54b936 GSB: The combination of a superclass and conformance requirement might force a type to be concrete
A protocol can constrain an associated type to Self:

    protocol P {
      associatedtype A : Q where A.B == Self
    }

    protocol Q {
      associatedtype B
    }

And a class might conform to this protocol:

    class C : P {
      typealias A = D
    }

    class D : Q {
      typealias B = C
    }

The generic signature <Self where Self : P, Self : C> is built during
conformance checking. Since Self : C, we must have that Self.A == D;
since D.B == C, the requierement 'A.B == Self' in protocol P implies
that 'Self == C'. So the correct minimized signature here is
<Self where Self == C>.

This wasn't handled properly before, because of assumptions in
removeSelfDerived() and a couple of other places.

Fixes rdar://71677712, rdar://76155506, https://bugs.swift.org/browse/SR-10033,
https://bugs.swift.org/browse/SR-13884.
2021-04-03 23:04:39 -04:00
Slava Pestov
7e17aaf094 GSB: Remove one of the two overloads of checkConstraintList() 2021-04-03 22:33:14 -04:00
Slava Pestov
61f796da5a GSB: Remove unused parameter from lookupConformance() 2021-04-03 22:33:14 -04:00
Slava Pestov
bc1717b7b3 GSB: Factor out updateLayout() from addLayoutRequirementDirect() 2021-04-03 22:33:14 -04:00
Slava Pestov
cb25f2ba47 GSB: Diagnose conflicts between concrete-type and AnyObject requirements 2021-04-03 22:33:14 -04:00
Slava Pestov
f190e51f8d GSB: Diagnose redundant superclass requirements using the redundant requirement graph 2021-04-03 22:33:14 -04:00
Slava Pestov
4ae755e6fb GSB: Fix handling of layout requirements in stripBoundDependentMemberTypes()
There was a silly typo here. Also add some asserts to the
Requirement constructor to catch this if it happens again.

Fixes rdar://problem/75691385.
2021-03-28 00:10:06 -04:00
Slava Pestov
1d9b202719 GSB: Fix inconsistent 'redundant conformance' diagnostic 2021-03-27 00:35:19 -04:00
Slava Pestov
299ff02684 GSB: Diagnose redundant layout requirements using the redundant requirement graph 2021-03-27 00:35:19 -04:00
Slava Pestov
1617727e12 GSB: Refactor computeRedundantRequirements() in preparation for redoing conflict checking 2021-03-27 00:35:19 -04:00
Slava Pestov
524e588bf3 GSB: Skip same-type requirement minimization if there are no same-type requirements
The logic here is pretty expensive, and there's no reason to
do it at all if we aren't going to end up with any same-type
requirements.
2021-03-26 01:29:42 -04:00
Slava Pestov
a7f2e5c50b GSB: Use explicit requirement list in enumerateRequirements() 2021-03-26 01:29:42 -04:00
Slava Pestov
74b29caed3 GSB: Fix bug with concrete same-type requirements to self-conforming protocols
If a type parameter both conformed to Error and was required to equal
Error, we didn't record a concrete source for the Error conformance.

This still produced the right minimized signature because the code in
enumerateRequirements() happens to skip all other requirements if an
equivalence class was concrete, but we shouldn't rely on that.

The bug also meant that we didn't diagnose the redundant conformance
requirement.
2021-03-26 01:29:42 -04:00
Slava Pestov
389a082985 GSB: Assert that concrete types should not have any other requirements
This sort of thing is not valid and should never happen:

    <T where T : P, T == Int>
2021-03-26 01:29:42 -04:00
Slava Pestov
7442e3ad04 GSB: Use a SmallSetVector to store the explicit requirements
We can literally have duplicate requirements (same source and RHS)
when abstract signatures are built from scratch. They're not
flagged as "redundant" since there's no way to distinguish them
in the requirement graph, so just filter them out entirely.
2021-03-26 01:29:42 -04:00
Slava Pestov
99d49f0ccb GSB: When rebuilding a signature, use the as-written requirements instead of the minimized ones
We rebuild a signature after dropping redundant conformance requirements,
since conformance requirements change the canonical type and conformance
access path computation.

When doing this, instead of using the canonical requirements from the
signature, use the original as-written requirements.

This fixes some weirdness around concrete same-type requirements.

There's a philosophical benefit here too -- since we rebuild the
signature without ever having built the old signature, we never create
an invalid GenericSignature in the ASTContext.

Fixes rdar://problem/75690903.
2021-03-25 17:35:52 -04:00
Slava Pestov
7d3a8911b1 GSB: Don't canonicalize concrete type or superclass of equivalence class 2021-03-25 17:33:31 -04:00
Slava Pestov
319bc071bd GSB: Use compareRequirements() in enumerateRequirements() 2021-03-25 17:33:31 -04:00
Slava Pestov
1fe4092b10 GSB: Extract out compareRequirements() from checkGenericSignature()
I want to use the same predicate for sorting requirements as what
we use to assert that the signature was valid.
2021-03-24 18:52:48 -04:00
Slava Pestov
ae133bc39b GSB: Use explicit requirement list when diagnosing redundant requirements 2021-03-24 18:45:04 -04:00
Slava Pestov
1bc4a4f7e2 GSB: Refactor enumerateRequirements() a bit
Extract out createRequirement(), since I'm going to be using it soon.

Also, merge the two loops together to do a single walk over all the
equivalence classes, instead of pointlessly building a list of
SameTypeComponentRefs first.
2021-03-24 18:45:04 -04:00