Skip to content

6.0: [ConsumeAddrChecker] Diagnose consumes of borrows. #74373

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
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
4 changes: 2 additions & 2 deletions include/swift/SIL/MemAccessUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -887,10 +887,10 @@ namespace swift {
/// For convenience, encapsulate and AccessStorage value along with its
/// accessed base address.
struct AccessStorageWithBase {
/// Identical to AccessStorage::compute but preserves the access base.
/// Identical to AccessStorage::computeInScope but walks through begin_access.
static AccessStorageWithBase compute(SILValue sourceAddress);

/// Identical to AccessStorage::computeInScope but preserves the base.
/// Identical to AccessStorage::compute but stops at begin_access
static AccessStorageWithBase computeInScope(SILValue sourceAddress);

AccessStorage storage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2079,12 +2079,98 @@ bool ConsumeOperatorCopyableAddressesChecker::performClosureDataflow(
closureConsumes);
}

struct MoveConstraint {
enum Value : uint8_t {
None,
RequiresReinit,
Illegal,
} value;

operator Value() { return value; }
MoveConstraint(Value value) : value(value) {}

static MoveConstraint forGuaranteed(bool guaranteed) {
return guaranteed ? Illegal : None;
}

bool isIllegal() { return value == Illegal; }
};

static MoveConstraint getMoveConstraint(SILValue addr) {
assert(addr->getType().isAddress());
auto access = AccessPathWithBase::computeInScope(addr);
auto base = access.getAccessBase();
switch (access.accessPath.getStorage().getKind()) {
case AccessRepresentation::Kind::Box:
// Even if the box is guaranteed, it may be permitted to consume its
// storage.
return MoveConstraint::None;
case AccessRepresentation::Kind::Stack: {
// An alloc_stack is guaranteed if it's a "store_borrow destination".
auto *asi = cast<AllocStackInst>(base.getBaseAddress());
return MoveConstraint::forGuaranteed(
!asi->getUsersOfType<StoreBorrowInst>().empty());
}
case AccessRepresentation::Kind::Global:
// A global can be consumed if it's reinitialized.
return MoveConstraint::RequiresReinit;
case AccessRepresentation::Kind::Class:
// A class field can be consumed if it's reinitialized.
return MoveConstraint::RequiresReinit;
case AccessRepresentation::Kind::Tail:
// A class field can be consumed if it's reinitialized.
return MoveConstraint::RequiresReinit;
case AccessRepresentation::Kind::Argument: {
// An indirect argument is guaranteed if it's @in_guaranteed.
auto *arg = base.getArgument();
return MoveConstraint::forGuaranteed(
arg->getArgumentConvention().isGuaranteedConvention());
}
case AccessRepresentation::Kind::Yield: {
auto baseAddr = base.getBaseAddress();
auto *bai = cast<BeginApplyInst>(
cast<MultipleValueInstructionResult>(baseAddr)->getParent());
auto index = *bai->getIndexOfResult(baseAddr);
auto info = bai->getSubstCalleeConv().getYieldInfoForOperandIndex(index);
return MoveConstraint::forGuaranteed(!info.isConsumed());
}
case AccessRepresentation::Kind::Nested: {
auto *bai = cast<BeginAccessInst>(base.getBaseAddress());
if (bai->getAccessKind() == SILAccessKind::Init ||
bai->getAccessKind() == SILAccessKind::Read)
return MoveConstraint::Illegal;
// Allow moves from both modify and deinit.
return MoveConstraint::None;
}
case AccessRepresentation::Kind::Unidentified:
// Conservatively reject for now.
return MoveConstraint::Illegal;
}
}

// Returns true if we emitted a diagnostic and handled the single block
// case. Returns false if we visited all of the uses and seeded the UseState
// struct with the information needed to perform our interprocedural dataflow.
bool ConsumeOperatorCopyableAddressesChecker::performSingleBasicBlockAnalysis(
SILValue address, DebugVarCarryingInst addressDebugInst,
MarkUnresolvedMoveAddrInst *mvi) {
if (getMoveConstraint(mvi->getSrc()).isIllegal()) {
auto &astCtx = mvi->getFunction()->getASTContext();
StringRef name = getDebugVarName(address);
diagnose(astCtx, getSourceLocFromValue(address),
diag::sil_movechecking_guaranteed_value_consumed, name);
diagnose(astCtx, mvi->getLoc().getSourceLoc(),
diag::sil_movechecking_consuming_use_here);

// Replace the marker instruction with a copy_addr to avoid subsequent
// diagnostics.
SILBuilderWithScope builder(mvi);
builder.createCopyAddr(mvi->getLoc(), mvi->getSrc(), mvi->getDest(),
IsNotTake, IsInitialization);
mvi->eraseFromParent();

return true;
}
// First scan downwards to make sure we are move out of this block.
auto &useState = dataflowState.useState;
auto &applySiteToPromotedArgIndices =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-swift-frontend -verify %s -parse-stdlib -emit-sil -o /dev/null
// RUN: %target-swift-frontend -sil-verify-all -verify %s -parse-stdlib -emit-sil -o /dev/null


import Swift
Expand Down Expand Up @@ -146,8 +146,8 @@ public func conditionalBadConsumingUseLoop2<T>(_ x: T) {
// Parameters

// This is ok, no uses after.
public func simpleMoveOfParameter<T>(_ x: T) -> () {
let _ = consume x
public func simpleMoveOfParameter<T>(_ x: T) -> () { // expected-error {{'x' is borrowed and cannot be consumed}}
let _ = consume x // expected-note {{consumed here}}
}

public func simpleMoveOfOwnedParameter<T>(_ x: __owned T) -> () {
Expand Down Expand Up @@ -214,12 +214,12 @@ func consumeOwned<T>(_ k: __owned T) {
_ = consume k
}

func consumeShared<T>(_ k: __shared T) {
_ = consume k
func consumeShared<T>(_ k: __shared T) { // expected-error {{'k' is borrowed and cannot be consumed}}
_ = consume k // expected-note {{consumed here}}
}

func consumeBare<T>(_ k: T) {
_ = consume k
func consumeBare<T>(_ k: T) { // expected-error {{'k' is borrowed and cannot be consumed}}
_ = consume k // expected-note {{consumed here}}
}

////////////////////////
Expand Down Expand Up @@ -384,6 +384,20 @@ public func castTestIfLet2(_ x : __owned EnumWithKlass) { // expected-error {{'x
}
}

enum rdar125817827<A, B> {
case a(A)
case b(B)
}

extension rdar125817827 {
func foo() { // expected-error {{'self' is borrowed and cannot be consumed}}
switch consume self { // expected-note {{consumed here}}
case let .a(a): print(a)
case let .b(b): print(b)
}
}
}

/////////////////////////
// Partial Apply Tests //
/////////////////////////
Expand Down