Type substitution eliminates dependencies with Escapable targets. #80438
Conversation
jckarter
requested review from
hborla,
slavapestov,
xedin and
eeckstein
as code owners
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
include/swift/AST/TypeTransform.h
Outdated
| /// Returns true if the set of output lifetime dependencies is different from | ||
| /// the set of input lifetime dependencies, false if there was no change. | ||
| inline bool | ||
| filterEscapableLifetimeDependencies(GenericSignature sig, |
There was a problem hiding this comment.
Maybe this belongs in LifetimeDependenceInfo.h?
include/swift/AST/TypeTransform.h
Outdated
| } else { | ||
| // The last lifetime dependency targets the result at the end | ||
| // of the curried chain. | ||
| assert(targetIndex == params.size() |
There was a problem hiding this comment.
Let's use ASSERT in new code
include/swift/AST/TypeTransform.h
Outdated
| auto extInfo = fnTy->getExtInfo(); | ||
| SmallVector<LifetimeDependenceInfo, 2> substDependenceInfos; | ||
| bool didRemoveLifetimeDependencies | ||
| = filterEscapableLifetimeDependencies(GenericSignature(), |
There was a problem hiding this comment.
Type::transform() is a very hot code path. Can you think of a way to skip all of this new work if the function type does not have lifetime dependencies?
There was a problem hiding this comment.
I misunderstood your code; if there are no lifetime dependencies in the function type it does appear everything is skipped.
There was a problem hiding this comment.
filterEscapableLifetimeDependencies doesn't do anything if the input list is empty, since it just runs in a for loop. I can hoist that check out at each use point if you think it'll make a difference.
| @@ -916,3 +916,11 @@ bool TypeBase::isEscapable() { | |||
| auto canType = preprocessTypeForInvertibleQuery(this); | |||
| return conformsToInvertible(canType, InvertibleProtocolKind::Escapable); | |||
| } | |||
|
|
|||
| bool TypeBase::isEscapable(GenericSignature sig) { | |||
There was a problem hiding this comment.
Is this new utility method really necessary? If you know type is an interface type, this will work whether sig is null or not:
GenericEnvironment::mapTypeIntoContext(sig.getGenericEnvironment(), type)->isEscapable()
There was a problem hiding this comment.
To me, it seems like having only the version that takes a GenericSignature would be more robust, since that would lead to call sites passing a GenericSignature proactively. As is it's easy to reach for isEscapable and write code that crashes as soon as it's used in a generic context.
|
@swift-ci Please test macOS |
When a generic function has potentially Escapable outputs, those outputs declare lifetime dependencies, which have no effect when substitution leads to those types becoming `Escapable` in a concrete context. This means that type substitution should canonically eliminate lifetime dependencies targeting Escapable parameters or returns, and that type checking should allow a function value with potentially-Escapable lifetime dependencies to bind to a function type without those dependencies when the target of the dependencies is Escapable. Fixes rdar://147533059.
jckarter
force-pushed
the
substitute-away-escapable-lifetime-deps
branch
from
54f195f to
6b605f4
Compare
|
@swift-ci Please test |
Comments in PR swiftlang#80438, where this code comes from, already suggest using ASSERT() instead of assert().
Comments in PR swiftlang#80438, where this code comes from, already suggest using ASSERT() instead of assert().
Comments in PR swiftlang#80438, where this code comes from, already suggest using ASSERT() instead of assert().
When a generic function has potentially Escapable outputs, those outputs declare lifetime dependencies, which have no effect when substitution leads to those types becoming
Escapablein a concrete context. This means that type substitution should canonically eliminate lifetime dependencies targeting Escapable parameters or returns, and that type checking should allow a function value with potentially-Escapable lifetime dependencies to bind to a function type without those dependencies when the target of the dependencies is Escapable.Fixes rdar://147533059.