Skip to content

6.2: [MoveOnly] Fix consumption of opened existentials. #81044

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

Merged
merged 4 commits into from
Apr 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -7821,6 +7821,7 @@ class OpenExistentialAddrInst
}

OpenedExistentialAccess getAccessKind() const { return ForAccess; }
void setAccessKind(OpenedExistentialAccess kind) { ForAccess = kind; }

CanExistentialArchetypeType getDefinedOpenedArchetype() const {
const auto archetype = getOpenedArchetypeOf(getType().getASTType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1899,7 +1899,7 @@ shouldEmitPartialMutationError(UseState &useState, PartialMutation::Kind kind,
// Otherwise, walk one level towards our child type. We unconditionally
// unwrap since we should never fail here due to earlier checking.
std::tie(iterPair, iterType) =
*pair.walkOneLevelTowardsChild(iterPair, iterType, fn);
*pair.walkOneLevelTowardsChild(iterPair, iterType, targetType, fn);
}

return {};
Expand Down Expand Up @@ -3431,6 +3431,10 @@ void MoveOnlyAddressCheckerPImpl::rewriteUses(
auto accessPath = AccessPathWithBase::computeInScope(copy->getSrc());
if (auto *access = dyn_cast_or_null<BeginAccessInst>(accessPath.base))
access->setAccessKind(SILAccessKind::Modify);
if (auto *oeai =
dyn_cast_or_null<OpenExistentialAddrInst>(copy->getSrc())) {
oeai->setAccessKind(OpenedExistentialAccess::Mutable);
}
copy->setIsTakeOfSrc(IsTake);
continue;
}
Expand Down
15 changes: 2 additions & 13 deletions lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,6 @@ AvailableValues &Implementation::computeAvailableValues(SILBasicBlock *block) {
llvm::dbgs()
<< " Destructuring available values in preds to smallest size for bb"
<< block->getDebugID() << '\n');
auto *fn = block->getFunction();
IntervalMapAllocator::Map typeSpanToValue(getAllocator());
for (auto *predBlock : predsSkippingBackEdges) {
SWIFT_DEFER { typeSpanToValue.clear(); };
Expand Down Expand Up @@ -930,20 +929,10 @@ AvailableValues &Implementation::computeAvailableValues(SILBasicBlock *block) {
// smallest offset size could result in further destructuring that an
// earlier value required. Instead, we do a final loop afterwards using
// the interval map to update each available value.
auto iterType = iterValue->getType();
auto loc = getSafeLoc(predBlock->getTerminator());
SILBuilderWithScope builder(predBlock->getTerminator());

while (smallestOffsetSize->first.size < iterOffsetSize.size) {
TypeOffsetSizePair childOffsetSize;
SILType childType;

// We are returned an optional here and should never fail... so use a
// force unwrap.
std::tie(childOffsetSize, childType) =
*iterOffsetSize.walkOneLevelTowardsChild(iterOffsetSize, iterType,
fn);

// Before we destructure ourselves, erase our entire value from the
// map. We do not need to consider the possibility of there being holes
// in our range since we always store values whole to their entire
Expand Down Expand Up @@ -1401,8 +1390,8 @@ void Implementation::rewriteUses(InstructionDeleter *deleter) {

// Then walk one level towards our target type.
std::tie(iterOffsetSize, iterType) =
*useOffsetSize.walkOneLevelTowardsChild(parentOffsetSize,
iterType, fn);
*useOffsetSize.walkOneLevelTowardsChild(
parentOffsetSize, iterType, unwrappedOperandType, fn);

unsigned start = parentOffsetSize.startOffset;
consumeBuilder.emitDestructureValueOperation(
Expand Down
12 changes: 11 additions & 1 deletion lib/SILOptimizer/Mandatory/MoveOnlyTypeUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ static StructDecl *getFullyReferenceableStruct(SILType ktypeTy) {
std::optional<std::pair<TypeOffsetSizePair, SILType>>
TypeOffsetSizePair::walkOneLevelTowardsChild(
TypeOffsetSizePair ancestorOffsetSize, SILType ancestorType,
SILFunction *fn) const {
SILType childType, SILFunction *fn) const {
assert(ancestorOffsetSize.size >= size &&
"Too large to be a child of ancestorType");
assert((ancestorOffsetSize.startOffset <= startOffset &&
Expand Down Expand Up @@ -127,6 +127,16 @@ TypeOffsetSizePair::walkOneLevelTowardsChild(
llvm_unreachable("Not a child of this enum?!");
}

if (ancestorType.isExistentialType()) {
assert(childType);
auto childArchetypeType =
childType.getASTType()->getAs<ExistentialArchetypeType>();
assert(childArchetypeType);
assert(childArchetypeType->getExistentialType()->getCanonicalType() ==
ancestorType.getASTType());
return {{ancestorOffsetSize, childType}};
}

llvm_unreachable("Hit a leaf type?! Should have handled it earlier");
}

Expand Down
3 changes: 2 additions & 1 deletion lib/SILOptimizer/Mandatory/MoveOnlyTypeUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ struct TypeOffsetSizePair {
/// be a child type of \p ancestorType.
std::optional<std::pair<TypeOffsetSizePair, SILType>>
walkOneLevelTowardsChild(TypeOffsetSizePair ancestorOffsetSize,
SILType ancestorType, SILFunction *fn) const;
SILType ancestorType, SILType childType,
SILFunction *fn) const;

/// Given an ancestor offset \p ancestorOffset and a type called \p
/// ancestorType, walk one level towards this current type inserting on value,
Expand Down
21 changes: 21 additions & 0 deletions validation-test/SILOptimizer/rdar141279635.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// RUN: %target-build-swift %s

protocol P: ~Copyable {
var property: Bool { get }
consuming func function()
}

struct S: P, ~Copyable {
var property: Bool { false }
consuming func function() {}
}

func g(s: consuming any P & ~Copyable) {
let s = s
s.function()
}

func f() {
let s = S()
g(s: s)
}