Require matching lifetimes in protocol conformance checking#87281
Require matching lifetimes in protocol conformance checking#87281aidan-hall wants to merge 4 commits intoswiftlang:mainfrom
Conversation
aidan-hall
force-pushed
the
lifedep-protocol-check
branch
2 times, most recently
from
73da449 to
15c3b8c
Compare
aidan-hall
requested review from
AnthonyLatsis,
hborla,
slavapestov and
xedin
as code owners
|
@swift-ci smoke test |
|
@swift-ci test |
|
@swift-ci Please Test Source Compatibility |
aidan-hall
force-pushed
the
lifedep-protocol-check
branch
from
15c3b8c to
9a15561
Compare
|
@swift-ci test |
|
@swift-ci smoke test |
|
@swift-ci test source compatibility |
| namespace swift { | ||
| bool checkLifetimeConformanceApproximately( | ||
| const ArrayRef<LifetimeDependenceInfo> from, | ||
| const ArrayRef<LifetimeDependenceInfo> to) { |
There was a problem hiding this comment.
checkLifetimeConformanceApproximately doesn't seem to be needed in addition to checkLifetimeConformanceCarefully. Why not just have checkLifetimeConformanceCarefully ?
lib/AST/LifetimeDependence.cpp
Outdated
There was a problem hiding this comment.
checkLifetimeConformanceCarefully should probably be renamed to something on the lines of matchLifetimeDependencies.
The early exit condition above should also be added here:
if (from.size() != to.size())
return false;
Another early exit with:
if (from == to)
return true;
There was a problem hiding this comment.
if (from == to)
I believe ArrayRef::operator== performs element-wise equality testing. Did you mean that we could do a base-pointer comparison?
I.e.
if (from.data() == to.data()) return true;That would be an effective and cheap early exit check.
There was a problem hiding this comment.
if (from.size() != to.size()) return false;
👍
| // If a dependency is present for the same target in both types, then | ||
| // the dependency must match. | ||
| if (fromDep != *toDep) { | ||
| if (!fromDep.convertibleTo(*toDep)) { |
There was a problem hiding this comment.
Looks like the entire for loop can be collapsed into checkLifetimeConformanceCarefully
There was a problem hiding this comment.
This loop does have a similar structure to checkLifetimeConformanceCarefully, but it needs to add constraints for each fromDep that doesn't have a matching toDep, so it can't exit early.
|
This test fails because a function with an immortal lifetime dependence is used to conform to the protocol. This should be safe to do. To maintain source compatibility, we will need to introduce a lifetime subtyping relationship, as described in #87187. |
aidan-hall
force-pushed
the
lifedep-protocol-check
branch
from
8557dca to
1a64dc6
Compare
aidan-hall
force-pushed
the
lifedep-protocol-check
branch
from
1a64dc6 to
10aa07f
Compare
aidan-hall
force-pushed
the
lifedep-protocol-check
branch
from
e2e7bce to
eca9e8c
Compare
2 participants
rdar://160970376