Skip to content

[5.9][move-only] Make it an error if we attempt to destructure/partially invalidate through a field with a deinit. #65131

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.

Sign up for GitHub

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 1 commit into from
Apr 13, 2023
Merged

[5.9][move-only] Make it an error if we attempt to destructure/partially invalidate through a field with a deinit. #65131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions include/swift/AST/DiagnosticsSIL.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,12 @@ ERROR(sil_moveonlychecker_notconsumable_but_assignable_was_consumed_escaping_var
(StringRef))
ERROR(sil_moveonlychecker_let_capture_consumed, none,
"'%0' was consumed but it is illegal to consume a noncopyable immutable capture of an escaping closure. One can only read from it", (StringRef))
ERROR(sil_moveonlychecker_cannot_destructure_deinit_nominal_type_self, none,
"Cannot partially consume '%0' since it has a user defined deinit",
(StringRef))
ERROR(sil_moveonlychecker_cannot_destructure_deinit_nominal_type_field, none,
"Cannot partially consume '%0' since it contains field '%1.%2' whose type %3 has a user defined deinit",
(StringRef, StringRef, StringRef, DeclBaseName))

NOTE(sil_moveonlychecker_moveonly_field_consumed_here, none,
"move only field consumed here", ())
Expand All @@ -784,6 +790,8 @@ NOTE(sil_moveonlychecker_nonconsuming_use_here, none,
"non-consuming use here", ())
NOTE(sil_movekillscopyablevalue_value_cyclic_consumed_in_loop_here, none,
"consuming in loop use here", ())
NOTE(sil_moveonlychecker_deinit_here, none,
"deinit declared here", ())

ERROR(sil_moveonlychecker_not_understand_consumable_and_assignable, none,
"Usage of @noImplicitCopy that the move checker does not know how to "
Expand Down
11 changes: 6 additions & 5 deletions lib/SILOptimizer/Mandatory/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@ target_sources(swiftSILOptimizer PRIVATE
LexicalLifetimeEliminator.cpp
LowerHopToActor.cpp
MandatoryInlining.cpp
MovedAsyncVarDebugInfoPropagator.cpp
MoveOnlyAddressCheckerUtils.cpp
MoveOnlyAddressCheckerTester.cpp
MoveOnlyBorrowToDestructureUtils.cpp
MoveOnlyAddressCheckerUtils.cpp
MoveOnlyBorrowToDestructureTester.cpp
MoveOnlyBorrowToDestructureUtils.cpp
MoveOnlyChecker.cpp
MoveOnlyDeinitInsertion.cpp
MoveOnlyDiagnostics.cpp
MoveOnlyObjectCheckerUtils.cpp
MoveOnlyObjectCheckerTester.cpp
MoveOnlyChecker.cpp
MoveOnlyObjectCheckerUtils.cpp
MoveOnlyTypeUtils.cpp
MoveOnlyUtils.cpp
MovedAsyncVarDebugInfoPropagator.cpp
NestedSemanticFunctionCheck.cpp
OptimizeHopToExecutor.cpp
PerformanceDiagnostics.cpp
Expand Down
382 changes: 255 additions & 127 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
View file
Open in desktop

Large diffs are not rendered by default.

231 changes: 1 addition & 230 deletions lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "MoveOnlyBorrowToDestructureUtils.h"
#include "MoveOnlyDiagnostics.h"
#include "MoveOnlyObjectCheckerUtils.h"
#include "MoveOnlyTypeUtils.h"

#include "swift/Basic/BlotSetVector.h"
#include "swift/Basic/Defer.h"
Expand Down Expand Up @@ -544,236 +545,6 @@ void Implementation::checkDestructureUsesOnBoundary() const {
}
}

static StructDecl *getFullyReferenceableStruct(SILType ktypeTy) {
auto structDecl = ktypeTy.getStructOrBoundGenericStruct();
if (!structDecl || structDecl->hasUnreferenceableStorage())
return nullptr;
return structDecl;
}

namespace {

struct TypeOffsetSizePair {
SubElementOffset startOffset = 0;
TypeSubElementCount size = 0;

TypeOffsetSizePair() : startOffset(0), size(0) {}
TypeOffsetSizePair(SILType baseType, SILFunction *fn)
: startOffset(0), size(baseType, fn) {}
TypeOffsetSizePair(SubElementOffset offset, TypeSubElementCount size)
: startOffset(offset), size(size) {}
TypeOffsetSizePair(SILValue projection, SILValue base)
: startOffset(*SubElementOffset::compute(projection, base)),
size(TypeSubElementCount(projection)) {}

IntRange<unsigned> getRange() const {
return range(startOffset, getEndOffset());
}

SubElementOffset getEndOffset() const {
return SubElementOffset(startOffset + size);
}

bool operator==(const TypeOffsetSizePair &other) const {
return startOffset == other.startOffset && size == other.size;
}

bool operator!=(const TypeOffsetSizePair &other) const {
return !(*this == other);
}

/// Given an ancestor offset \p ancestorOffset and a type called \p
/// ancestorType, walk one level towards this current type which is assumed to
/// be a child type of \p ancestorType.
Optional<std::pair<TypeOffsetSizePair, SILType>>
walkOneLevelTowardsChild(TypeOffsetSizePair ancestorOffsetSize,
SILType ancestorType, SILFunction *fn) const {
assert(ancestorOffsetSize.size >= size &&
"Too large to be a child of ancestorType");
assert((ancestorOffsetSize.startOffset <= startOffset &&
startOffset <
(ancestorOffsetSize.startOffset + ancestorOffsetSize.size)) &&
"Not within the offset range of ancestor");

if (auto tupleType = ancestorType.getAs<TupleType>()) {
// Before we do anything, see if we have a single element tuple. If we do,
// just return that.
if (tupleType->getNumElements() == 1) {
return {{ancestorOffsetSize, ancestorType.getTupleElementType(0)}};
}

assert(ancestorOffsetSize.size > size &&
"Too large to be a child of ancestorType");

unsigned childOffset = ancestorOffsetSize.startOffset;

for (auto index : indices(tupleType->getElementTypes())) {
SILType newType = ancestorType.getTupleElementType(index);
unsigned newSize = TypeSubElementCount(newType, fn);

// childOffset + size(tupleChild) is the offset of the next tuple
// element. If our target offset is less than that, then we know that
// the target type must be a descendent of this tuple element type.
if (childOffset + newSize > startOffset) {
return {{{childOffset, newSize}, newType}};
}

// Otherwise, add the new size of this field to iterOffset so we visit
// our sibling type next.
childOffset += newSize;
}

// At this point, we know that our type is not a subtype of this
// type. Some sort of logic error occurred.
llvm_unreachable("Not a child of this type?!");
}

if (auto *structDecl = getFullyReferenceableStruct(ancestorType)) {
// Before we do anything, see if we have a single element struct. If we
// do, just return that.
auto storedProperties = structDecl->getStoredProperties();
if (storedProperties.size() == 1) {
return {{ancestorOffsetSize,
ancestorType.getFieldType(storedProperties[0], fn)}};
}

assert(ancestorOffsetSize.size > size &&
"Too large to be a child of ancestorType");

unsigned childOffset = ancestorOffsetSize.startOffset;
for (auto *fieldDecl : storedProperties) {
SILType newType = ancestorType.getFieldType(fieldDecl, fn);
unsigned newSize = TypeSubElementCount(newType, fn);

// iterOffset + size(tupleChild) is the offset of the next tuple
// element. If our target offset is less than that, then we know that
// the target type must be a child of this tuple element type.
if (childOffset + newSize > startOffset) {
return {{{childOffset, newSize}, newType}};
}

// Otherwise, add the new size of this field to iterOffset so we visit
// our sibling type next.
childOffset += newSize;
}

// At this point, we know that our type is not a subtype of this
// type. Some sort of logic error occurred.
llvm_unreachable("Not a child of this type?!");
}

if (auto *enumDecl = ancestorType.getEnumOrBoundGenericEnum()) {
llvm_unreachable("Cannot find child type of enum!\n");
}

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

/// Given an ancestor offset \p ancestorOffset and a type called \p
/// ancestorType, walk one level towards this current type inserting on value,
/// the relevant projection.
Optional<std::pair<TypeOffsetSizePair, SILValue>>
walkOneLevelTowardsChild(SILBuilderWithScope &builder, SILLocation loc,
TypeOffsetSizePair ancestorOffsetSize,
SILValue ancestorValue) const {
auto *fn = ancestorValue->getFunction();
SILType ancestorType = ancestorValue->getType();

assert(ancestorOffsetSize.size >= size &&
"Too large to be a child of ancestorType");
assert((ancestorOffsetSize.startOffset <= startOffset &&
startOffset <
(ancestorOffsetSize.startOffset + ancestorOffsetSize.size)) &&
"Not within the offset range of ancestor");
if (auto tupleType = ancestorType.getAs<TupleType>()) {
// Before we do anything, see if we have a single element tuple. If we do,
// just return that.
if (tupleType->getNumElements() == 1) {
auto *newValue = builder.createTupleExtract(loc, ancestorValue, 0);
return {{ancestorOffsetSize, newValue}};
}

assert(ancestorOffsetSize.size > size &&
"Too large to be a child of ancestorType");

unsigned childOffset = ancestorOffsetSize.startOffset;

for (auto index : indices(tupleType->getElementTypes())) {
SILType newType = ancestorType.getTupleElementType(index);
unsigned newSize = TypeSubElementCount(newType, fn);

// childOffset + size(tupleChild) is the offset of the next tuple
// element. If our target offset is less than that, then we know that
// the target type must be a descendent of this tuple element type.
if (childOffset + newSize > startOffset) {
auto *newValue =
builder.createTupleExtract(loc, ancestorValue, index);
return {{{childOffset, newSize}, newValue}};
}

// Otherwise, add the new size of this field to iterOffset so we visit
// our sibling type next.
childOffset += newSize;
}

// At this point, we know that our type is not a subtype of this
// type. Some sort of logic error occurred.
llvm_unreachable("Not a child of this type?!");
}

if (auto *structDecl = getFullyReferenceableStruct(ancestorType)) {
// Before we do anything, see if we have a single element struct. If we
// do, just return that.
auto storedProperties = structDecl->getStoredProperties();
if (storedProperties.size() == 1) {
auto *newValue = builder.createStructExtract(loc, ancestorValue,
storedProperties[0]);
return {{ancestorOffsetSize, newValue}};
}

assert(ancestorOffsetSize.size > size &&
"Too large to be a child of ancestorType");

unsigned childOffset = ancestorOffsetSize.startOffset;
for (auto *fieldDecl : structDecl->getStoredProperties()) {
SILType newType = ancestorType.getFieldType(fieldDecl, fn);
unsigned newSize = TypeSubElementCount(newType, fn);

// iterOffset + size(tupleChild) is the offset of the next tuple
// element. If our target offset is less than that, then we know that
// the target type must be a child of this tuple element type.
if (childOffset + newSize > startOffset) {
auto *newValue =
builder.createStructExtract(loc, ancestorValue, fieldDecl);
return {{{childOffset, newSize}, newValue}};
}

// Otherwise, add the new size of this field to iterOffset so we visit
// our sibling type next.
childOffset += newSize;
}

// At this point, we know that our type is not a subtype of this
// type. Some sort of logic error occurred.
llvm_unreachable("Not a child of this type?!");
}

if (auto *enumDecl = ancestorType.getEnumOrBoundGenericEnum()) {
llvm_unreachable("Cannot find child type of enum!\n");
}

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

llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
const TypeOffsetSizePair &other) {
return os << "(startOffset: " << other.startOffset << ", size: " << other.size
<< ")";
}

} // anonymous namespace

#ifndef NDEBUG
static void dumpSmallestTypeAvailable(
SmallVectorImpl<Optional<std::pair<TypeOffsetSizePair, SILType>>>
Expand Down
25 changes: 25 additions & 0 deletions lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,3 +750,28 @@ void DiagnosticEmitter::emitPromotedBoxArgumentError(
diagnose(astContext, user, diag::sil_moveonlychecker_consuming_use_here);
}
}

void DiagnosticEmitter::emitCannotDestructureDeinitNominalError(
MarkMustCheckInst *markedValue, StringRef pathString,
NominalTypeDecl *deinitedNominal, SILInstruction *consumingUser) {
auto &astContext = fn->getASTContext();
SmallString<64> varName;
getVariableNameForValue(markedValue, varName);

registerDiagnosticEmitted(markedValue);

if (pathString.empty()) {
diagnose(
astContext, markedValue,
diag::sil_moveonlychecker_cannot_destructure_deinit_nominal_type_self,
varName);
} else {
diagnose(
astContext, markedValue,
diag::sil_moveonlychecker_cannot_destructure_deinit_nominal_type_field,
varName, varName, pathString.drop_front(),
deinitedNominal->getBaseName());
}
diagnose(astContext, consumingUser,
diag::sil_moveonlychecker_consuming_use_here);
}
5 changes: 5 additions & 0 deletions lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ class DiagnosticEmitter {
void emitPromotedBoxArgumentError(MarkMustCheckInst *markedValue,
SILFunctionArgument *arg);

void emitCannotDestructureDeinitNominalError(MarkMustCheckInst *markedValue,
StringRef pathString,
NominalTypeDecl *deinitedNominal,
SILInstruction *consumingUser);

private:
/// Emit diagnostics for the final consuming uses and consuming uses needing
/// copy. If filter is non-null, allow for the caller to pre-process operands
Expand Down
Loading