Skip to content

[move-only] Implement very initial work for move only #60187

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 3 commits into from
Jul 22, 2022
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
6 changes: 6 additions & 0 deletions include/swift/AST/Attr.def
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,12 @@ CONTEXTUAL_SIMPLE_DECL_ATTR(_local, KnownToBeLocal,
APIBreakingToAdd | APIBreakingToRemove,
130)

SIMPLE_DECL_ATTR(_moveOnly, MoveOnly,
OnNominalType |
UserInaccessible |
ABIBreakingToAdd | ABIBreakingToRemove | APIBreakingToAdd | APIBreakingToRemove,
131)

// If you're adding a new underscored attribute here, please document it in
// docs/ReferenceGuides/UnderscoredAttributes.md.

Expand Down
10 changes: 10 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2262,12 +2262,19 @@ class ValueDecl : public Decl {
/// Whether this declaration produces an implicitly unwrapped
/// optional result.
unsigned isIUO : 1;

/// Whether the "isMoveOnly" bit has been computed yet.
unsigned isMoveOnlyComputed : 1;

/// Whether this declaration can not be copied and thus is move only.
unsigned isMoveOnly : 1;
} LazySemanticInfo = { };

friend class DynamicallyReplacedDeclRequest;
friend class OverriddenDeclsRequest;
friend class IsObjCRequest;
friend class IsFinalRequest;
friend class IsMoveOnlyRequest;
friend class IsDynamicRequest;
friend class IsImplicitlyUnwrappedOptionalRequest;
friend class InterfaceTypeRequest;
Expand Down Expand Up @@ -2542,6 +2549,9 @@ class ValueDecl : public Decl {
/// Is this declaration 'final'?
bool isFinal() const;

/// Is this declaration 'moveOnly'?
bool isMoveOnly() const;

/// Is this declaration marked with 'dynamic'?
bool isDynamic() const;

Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -6396,5 +6396,8 @@ ERROR(concurrency_task_to_thread_model_global_actor_annotation,none,
"annotating a type with a global actor %0 is not permitted within %1",
(TypeRepr*, StringRef))

ERROR(moveOnly_not_allowed_here,none,
"'moveOnly' may only be applied to classes, structs, and enums", ())

#define UNDEFINE_DIAGNOSTIC_MACROS
#include "DefineDiagnosticMacros.h"
20 changes: 20 additions & 0 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,26 @@ class IsFinalRequest :
void cacheResult(bool value) const;
};

/// Determine whether the given declaration is 'moveOnly'.
class IsMoveOnlyRequest
: public SimpleRequest<IsMoveOnlyRequest, bool(ValueDecl *),
RequestFlags::SeparatelyCached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

// Evaluation.
bool evaluate(Evaluator &evaluator, ValueDecl *decl) const;

public:
// Separate caching.
bool isCached() const { return true; }
Optional<bool> getCachedResult() const;
void cacheResult(bool value) const;
};

/// Determine whether the given declaration is 'dynamic''.
class IsDynamicRequest :
public SimpleRequest<IsDynamicRequest,
Expand Down
2 changes: 2 additions & 0 deletions include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ SWIFT_REQUEST(TypeChecker, IsDynamicRequest, bool(ValueDecl *),
SeparatelyCached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, IsFinalRequest, bool(ValueDecl *), SeparatelyCached,
NoLocationInfo)
SWIFT_REQUEST(TypeChecker, IsMoveOnlyRequest, bool(ValueDecl *), SeparatelyCached,
NoLocationInfo)
SWIFT_REQUEST(TypeChecker, IsGetterMutatingRequest, bool(AbstractStorageDecl *),
SeparatelyCached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, IsImplicitlyUnwrappedOptionalRequest,
Expand Down
4 changes: 2 additions & 2 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -7587,8 +7587,8 @@ class MarkMustCheckInst
OwnershipForwardingMixin(SILInstructionKind::MarkMustCheckInst,
operand->getOwnershipKind()),
kind(checkKind) {
assert(operand->getType().isMoveOnlyWrapped() &&
"mark_must_check can only take a move only wrapped value");
assert(operand->getType().isMoveOnly() &&
"mark_must_check can only take a move only typed value");
}

public:
Expand Down
4 changes: 4 additions & 0 deletions include/swift/SIL/SILType.h
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,10 @@ class SILType {
/// Returns true if this is the AnyObject SILType;
bool isAnyObject() const { return getASTType()->isAnyObject(); }

/// Returns true if this type is a first class move only type or a move only
/// wrapped type.
bool isMoveOnly() const;

/// Returns true if this SILType is a move only wrapper type.
///
/// Canonical way to check if a SILType is move only. Using is/getAs/castTo
Expand Down
6 changes: 6 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3102,6 +3102,12 @@ bool ValueDecl::isFinal() const {
getAttrs().hasAttribute<FinalAttr>());
}

bool ValueDecl::isMoveOnly() const {
return evaluateOrDefault(getASTContext().evaluator,
IsMoveOnlyRequest{const_cast<ValueDecl *>(this)},
getAttrs().hasAttribute<MoveOnlyAttr>());
}

bool ValueDecl::isDynamic() const {
ASTContext &ctx = getASTContext();
return evaluateOrDefault(ctx.evaluator,
Expand Down
23 changes: 23 additions & 0 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,29 @@ void IsFinalRequest::cacheResult(bool value) const {
decl->getAttrs().add(new (decl->getASTContext()) FinalAttr(/*Implicit=*/true));
}

//----------------------------------------------------------------------------//
// isMoveOnly computation.
//----------------------------------------------------------------------------//

Optional<bool> IsMoveOnlyRequest::getCachedResult() const {
auto decl = std::get<0>(getStorage());
if (decl->LazySemanticInfo.isMoveOnlyComputed)
return decl->LazySemanticInfo.isMoveOnly;

return None;
}

void IsMoveOnlyRequest::cacheResult(bool value) const {
auto decl = std::get<0>(getStorage());
decl->LazySemanticInfo.isMoveOnlyComputed = true;
decl->LazySemanticInfo.isMoveOnly = value;

// Add an attribute for printing
if (value && !decl->getAttrs().hasAttribute<MoveOnlyAttr>())
decl->getAttrs().add(new (decl->getASTContext())
MoveOnlyAttr(/*Implicit=*/true));
}

//----------------------------------------------------------------------------//
// isDynamic computation.
//----------------------------------------------------------------------------//
Expand Down
8 changes: 8 additions & 0 deletions lib/SIL/IR/SILType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -924,3 +924,11 @@ SILType::getSingletonAggregateFieldType(SILModule &M,

return SILType();
}

// TODO: Create isPureMoveOnly.
bool SILType::isMoveOnly() const {
if (auto *nom = getNominalOrBoundGenericNominal())
if (nom->isMoveOnly())
return true;
return isMoveOnlyWrapped();
}
4 changes: 2 additions & 2 deletions lib/SIL/Verifier/SILVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2701,8 +2701,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
"copy_value is only valid in functions with qualified "
"ownership");
require(I->getModule().getStage() == SILStage::Raw ||
!I->getOperand()->getType().isMoveOnlyWrapped(),
"@moveOnly types can only be copied in Raw SIL?!");
!I->getOperand()->getType().isMoveOnly(),
"'MoveOnly' types can only be copied in Raw SIL?!");
}

void checkDestroyValueInst(DestroyValueInst *I) {
Expand Down
7 changes: 7 additions & 0 deletions lib/SILGen/SILGenConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "swift/AST/PropertyWrappers.h"
#include "swift/Basic/Defer.h"
#include "swift/SIL/SILArgument.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SIL/SILUndef.h"
#include "swift/SIL/TypeLowering.h"

Expand Down Expand Up @@ -386,6 +387,7 @@ void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {

// Allocate the local variable for 'self'.
emitLocalVariableWithCleanup(selfDecl, MUIKind)->finishInitialization(*this);

SILValue selfLV = VarLocs[selfDecl].value;

// Emit the prolog.
Expand Down Expand Up @@ -831,6 +833,11 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
StoreOwnershipQualifier::Init);
} else {
selfArg = B.createMarkUninitialized(selfDecl, selfArg, MUKind);
if (selfArg.getType().isMoveOnly()) {
assert(selfArg.getOwnershipKind() == OwnershipKind::Owned);
selfArg = B.createMarkMustCheckInst(
selfDecl, selfArg, MarkMustCheckInst::CheckKind::NoImplicitCopy);
}
VarLocs[selfDecl] = VarLoc::get(selfArg.getValue());
}
}
Expand Down
14 changes: 10 additions & 4 deletions lib/SILGen/SILGenDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -572,15 +572,21 @@ class LetValueInitialization : public Initialization {
// why we want this is even if we are only performing a borrow for our
// lexical lifetime, we want to ensure that our defs see this
// initialization as consuming this value.
if (value->getType().isMoveOnlyWrapped() &&
value->getOwnershipKind() == OwnershipKind::Owned) {
if (value->getOwnershipKind() == OwnershipKind::Owned) {
assert(wasPlusOne);
// NOTE: If our type is trivial when not wrapped in a
// SILMoveOnlyWrappedType, this will return a trivial value. We rely
// on the checker to determine if this is an acceptable use of the
// value.
value = SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(PrologueLoc,
value);
if (value->getType().isMoveOnly()) {
if (value->getType().isMoveOnlyWrapped()) {
value = SGF.B.createOwnedMoveOnlyWrapperToCopyableValue(
PrologueLoc, value);
} else {
// Change this to be lexical and get rid of borrow scope?
value = SGF.B.createMoveValue(PrologueLoc, value);
}
}
}

// If we still have a non-trivial thing, emit code that will need to
Expand Down
10 changes: 7 additions & 3 deletions lib/SILGen/SILGenPattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2835,9 +2835,13 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) {
auto subject = ([&]() -> ConsumableManagedValue {
// If we have a move only value, ensure plus one and convert it. Switches
// always consume move only values.
if (subjectMV.getType().isMoveOnlyWrapped()) {
subjectMV = B.createOwnedMoveOnlyWrapperToCopyableValue(
S, subjectMV.ensurePlusOne(*this, S));
if (subjectMV.getType().isMoveOnly()) {
if (subjectMV.getType().isMoveOnlyWrapped()) {
subjectMV = B.createOwnedMoveOnlyWrapperToCopyableValue(
S, subjectMV.ensurePlusOne(*this, S));
} else {
subjectMV = B.createMoveValue(S, subjectMV.ensurePlusOne(*this, S));
}
}

// If we have a plus one value...
Expand Down
14 changes: 14 additions & 0 deletions lib/SILGen/SILGenProlog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,20 @@ struct ArgumentInitHelper {
loc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
SGF.emitManagedRValueWithCleanup(value);
}
} else if (value->getType().isMoveOnly()) {
if (value->getOwnershipKind() == OwnershipKind::Owned) {
value = SGF.B.createMoveValue(loc, argrv.forward(SGF),
/*isLexical*/ true);
value = SGF.B.createMarkMustCheckInst(
loc, value, MarkMustCheckInst::CheckKind::NoImplicitCopy);
SGF.emitManagedRValueWithCleanup(value);
} else {
assert(value->getOwnershipKind() == OwnershipKind::Guaranteed);
value = SGF.B.createCopyValue(loc, value);
value = SGF.B.createMarkMustCheckInst(
loc, value, MarkMustCheckInst::CheckKind::NoCopy);
SGF.emitManagedRValueWithCleanup(value);
}
} else {
if (value->getOwnershipKind() == OwnershipKind::Owned) {
value = SILValue(
Expand Down
9 changes: 6 additions & 3 deletions lib/SILOptimizer/Mandatory/DIMemoryUseCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@
//===----------------------------------------------------------------------===//

#define DEBUG_TYPE "definite-init"

#include "DIMemoryUseCollector.h"
#include "swift/AST/Expr.h"
#include "swift/SIL/ApplySite.h"
#include "swift/SIL/InstructionUtils.h"
#include "swift/SIL/SILArgument.h"
#include "swift/SIL/SILBuilder.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SILOptimizer/Utils/DistributedActor.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Debug.h"
Expand Down Expand Up @@ -1211,6 +1213,7 @@ void ElementUseCollector::collectClassSelfUses(SILValue ClassPointer) {
Uses.append(beginAccess->getUses().begin(), beginAccess->getUses().end());
continue;
}

if (isa<EndAccessInst>(User))
continue;

Expand Down Expand Up @@ -1489,9 +1492,9 @@ void ElementUseCollector::collectClassSelfUses(

// Look through begin_borrow, upcast, unchecked_ref_cast
// and copy_value.
if (isa<BeginBorrowInst>(User) || isa<BeginAccessInst>(User)
|| isa<UpcastInst>(User) || isa<UncheckedRefCastInst>(User)
|| isa<CopyValueInst>(User)) {
if (isa<BeginBorrowInst>(User) || isa<BeginAccessInst>(User) ||
isa<UpcastInst>(User) || isa<UncheckedRefCastInst>(User) ||
isa<CopyValueInst>(User) || isa<MarkMustCheckInst>(User)) {
auto value = cast<SingleValueInstruction>(User);
std::copy(value->use_begin(), value->use_end(),
std::back_inserter(Worklist));
Expand Down
40 changes: 40 additions & 0 deletions lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,46 @@ bool MoveOnlyChecker::searchForCandidateMarkMustChecks() {
if (!mmci || !mmci->hasMoveCheckerKind())
continue;

// Handle guaranteed/owned move only typed arguments.
//
// We are pattern matching against these patterns:
//
// bb0(%0 : @guaranteed $T):
// %1 = copy_value %0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gottesmm if an argument is marked @moveOnly, then we need to be sure no copy_value exists in the argument, at least by the time this diagnostic runs. This is true for both owned and guaranteed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talked with Andy about this, what actually happens here is that as part of the diagnostic I eliminate the copy_value and the mark_must_check. I did it this way since today copy propagation doesn't support guaranteed parameters (although we could extend it, I decided to just hack around it).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The right thing here is I think to just see if we can get copy propagation working on guaranteed things and rip this out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gottesmm for future reference, copy propagation always handled guaranteed function arguments. It's done in RewriteInnerBorrowUses and CanonicalizeBorrowScope::visitBorrowScopeUses.

// %2 = mark_must_check [no_copy] %1
// bb0(%0 : @owned $T):
// %1 = copy_value %0
// %2 = mark_must_check [no_copy] %1
if (mmci->getOperand()->getType().isMoveOnly() &&
!mmci->getOperand()->getType().isMoveOnlyWrapped()) {
if (auto *cvi = dyn_cast<CopyValueInst>(mmci->getOperand())) {
if (auto *arg = dyn_cast<SILFunctionArgument>(cvi->getOperand())) {
if (arg->getOwnershipKind() == OwnershipKind::Guaranteed) {
moveIntroducersToProcess.insert(mmci);
continue;
}
}
}

if (auto *mvi = dyn_cast<MoveValueInst>(mmci->getOperand())) {
if (mvi->isLexical()) {
if (auto *arg = dyn_cast<SILFunctionArgument>(mvi->getOperand())) {
if (arg->getOwnershipKind() == OwnershipKind::Owned) {
moveIntroducersToProcess.insert(mmci);
continue;
}
}
}
}

if (auto *arg = dyn_cast<SILFunctionArgument>(mmci->getOperand())) {
if (arg->getOwnershipKind() == OwnershipKind::Owned) {
moveIntroducersToProcess.insert(mmci);
continue;
}
}
}

// Handle guaranteed arguments.
//
// We are pattern matching this pattern:
Expand Down
9 changes: 9 additions & 0 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
}

void visitFinalAttr(FinalAttr *attr);
void visitMoveOnlyAttr(MoveOnlyAttr *attr);
void visitCompileTimeConstAttr(CompileTimeConstAttr *attr) {}
void visitIBActionAttr(IBActionAttr *attr);
void visitIBSegueActionAttr(IBSegueActionAttr *attr);
Expand Down Expand Up @@ -1909,6 +1910,14 @@ void AttributeChecker::visitFinalAttr(FinalAttr *attr) {
}
}

void AttributeChecker::visitMoveOnlyAttr(MoveOnlyAttr *attr) {
if (isa<NominalTypeDecl>(D))
return;

diagnose(attr->getLocation(), diag::moveOnly_not_allowed_here)
.fixItRemove(attr->getRange());
}

/// Return true if this is a builtin operator that cannot be defined in user
/// code.
static bool isBuiltinOperator(StringRef name, DeclAttribute *attr) {
Expand Down
Loading