Skip to content

[move-keyword] Change from using a move function -> move keyword. #60404

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
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
2 changes: 2 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -6428,6 +6428,8 @@ ERROR(concurrency_task_to_thread_model_global_actor_annotation,none,

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

#define UNDEFINE_DIAGNOSTIC_MACROS
#include "DefineDiagnosticMacros.h"
27 changes: 27 additions & 0 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -2106,6 +2106,33 @@ class AwaitExpr final : public IdentityExpr {
}
};

/// MoveExpr - A 'move' surrounding an lvalue expression marking the lvalue as
/// needing to be moved.
///
/// getSemanticsProvidingExpr() looks through this because it doesn't
/// provide the value and only very specific clients care where the
/// 'move' was written.
class MoveExpr final : public IdentityExpr {
SourceLoc MoveLoc;

public:
MoveExpr(SourceLoc moveLoc, Expr *sub, Type type = Type(),
bool implicit = false)
: IdentityExpr(ExprKind::Move, sub, type, implicit), MoveLoc(moveLoc) {}

static MoveExpr *createImplicit(ASTContext &ctx, SourceLoc moveLoc, Expr *sub,
Type type = Type()) {
return new (ctx) MoveExpr(moveLoc, sub, type, /*implicit=*/true);
}

SourceLoc getLoc() const { return MoveLoc; }

SourceLoc getStartLoc() const { return MoveLoc; }
SourceLoc getEndLoc() const { return getSubExpr()->getEndLoc(); }

static bool classof(const Expr *e) { return e->getKind() == ExprKind::Move; }
};

/// TupleExpr - Parenthesized expressions like '(a: x+x)' and '(x, y, 4)'. Note
/// that expressions like '(4)' are represented with a ParenExpr.
class TupleExpr final : public Expr,
Expand Down
1 change: 1 addition & 0 deletions include/swift/AST/ExprNodes.def
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ ABSTRACT_EXPR(Identity, Expr)
EXPR(Paren, IdentityExpr)
EXPR(DotSelf, IdentityExpr)
EXPR(Await, IdentityExpr)
EXPR(Move, IdentityExpr)
EXPR(UnresolvedMemberChainResult, IdentityExpr)
EXPR_RANGE(Identity, Paren, UnresolvedMemberChainResult)
ABSTRACT_EXPR(AnyTry, Expr)
Expand Down
6 changes: 6 additions & 0 deletions lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2076,6 +2076,12 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
printRec(E->getSubExpr());
PrintWithColorRAII(OS, ParenthesisColor) << ')';
}
void visitMoveExpr(MoveExpr *E) {
printCommon(E, "move_expr");
OS << '\n';
printRec(E->getSubExpr());
PrintWithColorRAII(OS, ParenthesisColor) << ')';
}
void visitUnresolvedMemberChainResultExpr(UnresolvedMemberChainResultExpr *E){
printCommon(E, "unresolved_member_chain_expr");
OS << '\n';
Expand Down
5 changes: 5 additions & 0 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4526,6 +4526,11 @@ void PrintAST::visitAwaitExpr(AwaitExpr *expr) {
visit(expr->getSubExpr());
}

void PrintAST::visitMoveExpr(MoveExpr *expr) {
Printer << "move ";
visit(expr->getSubExpr());
}

void PrintAST::visitInOutExpr(InOutExpr *expr) {
visit(expr->getSubExpr());
}
Expand Down
3 changes: 3 additions & 0 deletions lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ ConcreteDeclRef Expr::getReferencedDecl(bool stopAtParenExpr) const {
PASS_THROUGH_REFERENCE(UnresolvedMemberChainResult, getSubExpr);
PASS_THROUGH_REFERENCE(DotSelf, getSubExpr);
PASS_THROUGH_REFERENCE(Await, getSubExpr);
PASS_THROUGH_REFERENCE(Move, getSubExpr);
PASS_THROUGH_REFERENCE(Try, getSubExpr);
PASS_THROUGH_REFERENCE(ForceTry, getSubExpr);
PASS_THROUGH_REFERENCE(OptionalTry, getSubExpr);
Expand Down Expand Up @@ -703,6 +704,7 @@ bool Expr::canAppendPostfixExpression(bool appendingPostfixOperator) const {
return true;

case ExprKind::Await:
case ExprKind::Move:
case ExprKind::Try:
case ExprKind::ForceTry:
case ExprKind::OptionalTry:
Expand Down Expand Up @@ -881,6 +883,7 @@ bool Expr::isValidParentOfTypeExpr(Expr *typeExpr) const {
case ExprKind::Sequence:
case ExprKind::Paren:
case ExprKind::Await:
case ExprKind::Move:
case ExprKind::UnresolvedMemberChainResult:
case ExprKind::Try:
case ExprKind::ForceTry:
Expand Down
14 changes: 14 additions & 0 deletions lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ ParserResult<Expr> Parser::parseExprSequence(Diag<> Message,
/// 'try' expr-sequence-element(Mode)
/// 'try' '?' expr-sequence-element(Mode)
/// 'try' '!' expr-sequence-element(Mode)
/// '_move' expr-sequence-element(Mode)
/// expr-unary(Mode)
///
/// 'try' is not actually allowed at an arbitrary position of a
Expand Down Expand Up @@ -432,6 +433,19 @@ ParserResult<Expr> Parser::parseExprSequenceElement(Diag<> message,
return sub;
}

if (Tok.isContextualKeyword("_move")) {
Tok.setKind(tok::contextual_keyword);
SourceLoc awaitLoc = consumeToken();
ParserResult<Expr> sub =
parseExprSequenceElement(diag::expected_expr_after_await, isExprBasic);
if (!sub.hasCodeCompletion() && !sub.isNull()) {
ElementContext.setCreateSyntax(SyntaxKind::MoveExpr);
sub = makeParserResult(new (Context) MoveExpr(awaitLoc, sub.get()));
}

return sub;
}

SourceLoc tryLoc;
bool hadTry = consumeIf(tok::kw_try, tryLoc);
Optional<Token> trySuffix;
Expand Down
36 changes: 36 additions & 0 deletions lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "LValue.h"
#include "RValue.h"
#include "ResultPlan.h"
#include "SGFContext.h"
#include "SILGen.h"
#include "SILGenDynamicCast.h"
#include "SILGenFunctionBuilder.h"
Expand All @@ -42,6 +43,7 @@
#include "swift/Basic/type_traits.h"
#include "swift/SIL/DynamicCasts.h"
#include "swift/SIL/SILArgument.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SIL/SILUndef.h"
#include "swift/SIL/TypeLowering.h"
#include "llvm/ADT/STLExtras.h"
Expand Down Expand Up @@ -537,6 +539,7 @@ namespace {
LinearFunctionExtractOriginalExpr *E, SGFContext C);
RValue visitLinearToDifferentiableFunctionExpr(
LinearToDifferentiableFunctionExpr *E, SGFContext C);
RValue visitMoveExpr(MoveExpr *E, SGFContext C);
};
} // end anonymous namespace

Expand Down Expand Up @@ -5848,6 +5851,39 @@ RValue RValueEmitter::visitErrorExpr(ErrorExpr *E, SGFContext C) {
llvm::report_fatal_error("Found an ErrorExpr but didn't emit an error?");
}

RValue RValueEmitter::visitMoveExpr(MoveExpr *E, SGFContext C) {
auto *subExpr = cast<DeclRefExpr>(E->getSubExpr());
auto subASTType = subExpr->getType()->getCanonicalType();

auto subType = SGF.getLoweredType(subASTType);

if (subType.isLoadable(SGF.F)) {
auto mv = SGF.emitRValue(subExpr).getAsSingleValue(SGF, subExpr);
if (mv.getType().isTrivial(SGF.F))
return RValue(SGF, {mv}, subType.getASTType());
mv = SGF.B.createMoveValue(E, mv);
auto *movedValue = cast<MoveValueInst>(mv.getValue());
movedValue->setAllowsDiagnostics(true /*set allows diagnostics*/);
return RValue(SGF, {mv}, subType.getASTType());
}

// If we aren't loadable, then create a temporary initialization and
// mark_unresolved_move into that.
std::unique_ptr<TemporaryInitialization> optTemp;
optTemp = SGF.emitTemporary(E, SGF.getTypeLowering(subType));
SILValue toAddr = optTemp->getAddressForInPlaceInitialization(SGF, E);
assert(!isa<LValueType>(E->getType()->getCanonicalType()) &&
"Shouldn't see an lvalue type here");

ManagedValue mv =
SGF.emitRValue(subExpr, SGFContext(SGFContext::AllowImmediatePlusZero))
.getAsSingleValue(SGF, subExpr);
assert(mv.getType().isAddress());
SGF.B.createMarkUnresolvedMoveAddr(subExpr, mv.getValue(), toAddr);
optTemp->finishInitialization(SGF);
return RValue(SGF, {optTemp->getManagedAddress()}, subType.getASTType());
}

RValue SILGenFunction::emitRValue(Expr *E, SGFContext C) {
assert(!E->getType()->hasLValueType() &&
"l-values must be emitted with emitLValue");
Expand Down
25 changes: 25 additions & 0 deletions lib/SILGen/SILGenLValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "LValue.h"
#include "RValue.h"
#include "SILGen.h"
#include "SILGenFunction.h"
#include "Scope.h"
#include "swift/AST/DiagnosticsCommon.h"
#include "swift/AST/DiagnosticsSIL.h"
Expand All @@ -35,6 +36,7 @@
#include "swift/SIL/SILArgument.h"
#include "swift/SIL/SILUndef.h"
#include "swift/SIL/TypeLowering.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/raw_ostream.h"
using namespace swift;
using namespace Lowering;
Expand Down Expand Up @@ -323,6 +325,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenLValue
LValue visitKeyPathApplicationExpr(KeyPathApplicationExpr *e,
SGFAccessKind accessKind,
LValueOptions options);
LValue visitMoveExpr(MoveExpr *e, SGFAccessKind accessKind,
LValueOptions options);

// Expressions that wrap lvalues

Expand Down Expand Up @@ -3709,6 +3713,27 @@ LValue SILGenLValue::visitInOutExpr(InOutExpr *e, SGFAccessKind accessKind,
return visitRec(e->getSubExpr(), accessKind, options);
}

LValue SILGenLValue::visitMoveExpr(MoveExpr *e, SGFAccessKind accessKind,
LValueOptions options) {
// Do formal evaluation of the base l-value.
LValue baseLV = visitRec(e->getSubExpr(), SGFAccessKind::ReadWrite,
options.forComputedBaseLValue());

ManagedValue addr = SGF.emitAddressOfLValue(e, std::move(baseLV));

// Now create the temporary and
auto temp =
SGF.emitFormalAccessTemporary(e, SGF.F.getTypeLowering(addr.getType()));
auto toAddr = temp->getAddressForInPlaceInitialization(SGF, e);
SGF.B.createMarkUnresolvedMoveAddr(e, addr.getValue(), toAddr);
temp->finishInitialization(SGF);

// Now return the temporary in a value component.
return LValue::forValue(SGFAccessKind::BorrowedAddressRead,
temp->getManagedAddress(),
toAddr->getType().getASTType());
}

/// Emit an lvalue that refers to the given property. This is
/// designed to work with ManagedValue 'base's that are either +0 or +1.
LValue SILGenFunction::emitPropertyLValue(SILLocation loc, ManagedValue base,
Expand Down
19 changes: 18 additions & 1 deletion lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@
#include "TypeCheckConcurrency.h"
#include "TypeChecker.h"
#include "swift/AST/ASTWalker.h"
#include "swift/AST/DiagnosticsSema.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/Expr.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/NameLookupRequests.h"
#include "swift/AST/Pattern.h"
#include "swift/AST/SourceFile.h"
#include "swift/AST/Stmt.h"
#include "swift/AST/TypeCheckRequests.h"
#include "swift/AST/Types.h"
#include "swift/Basic/Defer.h"
#include "swift/Basic/SourceManager.h"
#include "swift/Basic/Statistic.h"
Expand Down Expand Up @@ -97,6 +100,7 @@ bool BaseDiagnosticWalker::shouldWalkIntoDeclInClosureContext(Decl *D) {
/// invalid positions.
/// - Marker protocols cannot occur as the type of an as? or is expression.
/// - KeyPath expressions cannot refer to effectful properties / subscripts
/// - Move expressions must have a declref expr subvalue.
///
static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
bool isExprStmt) {
Expand Down Expand Up @@ -317,7 +321,13 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
if (auto cast = dyn_cast<CheckedCastExpr>(E)) {
checkCheckedCastExpr(cast);
}


// Diagnose move expression uses where the sub expression is not a declref
// expr.
if (auto *moveExpr = dyn_cast<MoveExpr>(E)) {
checkMoveExpr(moveExpr);
}

return { true, E };
}

Expand Down Expand Up @@ -357,6 +367,13 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
}
}

void checkMoveExpr(MoveExpr *moveExpr) {
if (!isa<DeclRefExpr>(moveExpr->getSubExpr())) {
Ctx.Diags.diagnose(moveExpr->getLoc(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jckarter do you think I need to check for more things here? When I talked with @DougGregor he mentioned to check that this check also catches field accesses (which it does)... but I figured I would ask out of an abundance of caution.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that we need an ironclad AST-level diagnostic here, since we will still need a diagnostic in SIL anyway to detect lvalues that can't be statically moved. So I think this is fine to reject obviously unsupportable cases.

diag::move_expression_not_passed_lvalue);
}
}

static Expr *lookThroughArgument(Expr *arg) {
while (1) {
if (auto conv = dyn_cast<ImplicitConversionExpr>(arg))
Expand Down
29 changes: 0 additions & 29 deletions stdlib/public/core/LifetimeManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -166,35 +166,6 @@ extension String {
}
}

/// Takes in a value at +1 and performs a Builtin.move upon it.
///
/// IMPLEMENTATION NOTES: During transparent inlining, Builtin.move becomes the
/// move_value instruction if we are inlining into a context where the
/// specialized type is loadable. If the transparent function is called in a
/// context where the inlined function specializes such that the specialized
/// type is still not loadable, the compiler aborts (a). Once we have opaque
/// values, this restriction will be lifted since after that address only types
/// at SILGen time will be loadable objects.
///
/// (a). This is implemented by requiring that Builtin.move only be called
/// within a function marked with the semantic tag "lifetimemanagement.move"
/// which conveniently is only the function we are defining here: _move.
///
/// NOTE: We mark this _alwaysEmitIntoClient to ensure that we are not creating
/// new ABI that the stdlib must maintain if a user calls this ignoring the '_'
/// implying it is stdlib SPI.
@_alwaysEmitIntoClient
@inlinable
@_transparent
@_semantics("lifetimemanagement.move")
public func _move<T>(_ value: __owned T) -> T {
#if $BuiltinMove
Builtin.move(value)
#else
value
#endif
}

/// Takes in a value at +0 and performs a Builtin.copy upon it.
///
/// IMPLEMENTATION NOTES: During transparent inlining, Builtin.copy becomes the
Expand Down
Loading