Skip to content

Add missing function conversion for member access to preconcurrency vardecl. #60521

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
Aug 30, 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
13 changes: 13 additions & 0 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -3157,6 +3157,19 @@ class LoadExpr : public ImplicitConversionExpr {
static bool classof(const Expr *E) { return E->getKind() == ExprKind::Load; }
};

/// ABISafeConversion - models a type conversion on an l-value that has no
/// material affect on the ABI of the type, while *preserving* the l-valueness
/// of the type.
class ABISafeConversionExpr : public ImplicitConversionExpr {
public:
ABISafeConversionExpr(Expr *subExpr, Type type)
: ImplicitConversionExpr(ExprKind::ABISafeConversion, subExpr, type) {}

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

/// This is a conversion from an expression of UnresolvedType to an arbitrary
/// other type, and from an arbitrary type to UnresolvedType. This node does
/// not appear in valid code, only in code involving diagnostics.
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 @@ -152,6 +152,7 @@ ABSTRACT_EXPR(Apply, Expr)
EXPR_RANGE(Apply, Call, ConstructorRefCall)
ABSTRACT_EXPR(ImplicitConversion, Expr)
EXPR(Load, ImplicitConversionExpr)
EXPR(ABISafeConversion, ImplicitConversionExpr)
EXPR(DestructureTuple, ImplicitConversionExpr)
EXPR(UnresolvedTypeConversion, ImplicitConversionExpr)
EXPR(FunctionConversion, ImplicitConversionExpr)
Expand Down
4 changes: 3 additions & 1 deletion include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -4764,7 +4764,8 @@ class ConstraintSystem {
Type getUnopenedTypeOfReference(VarDecl *value, Type baseType,
DeclContext *UseDC,
ConstraintLocator *memberLocator = nullptr,
bool wantInterfaceType = false);
bool wantInterfaceType = false,
bool adjustForPreconcurrency = true);

/// Return the type-of-reference of the given value.
///
Expand All @@ -4786,6 +4787,7 @@ class ConstraintSystem {
llvm::function_ref<Type(VarDecl *)> getType,
ConstraintLocator *memberLocator = nullptr,
bool wantInterfaceType = false,
bool adjustForPreconcurrency = true,
llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType =
[](const AbstractClosureExpr *) {
return Type();
Expand Down
5 changes: 5 additions & 0 deletions lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2276,6 +2276,11 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
printRec(E->getSubExpr());
PrintWithColorRAII(OS, ParenthesisColor) << ')';
}
void visitABISafeConversionExpr(ABISafeConversionExpr *E) {
printCommon(E, "abi_safe_conversion_expr") << '\n';
printRec(E->getSubExpr());
PrintWithColorRAII(OS, ParenthesisColor) << ')';
}
void visitMetatypeConversionExpr(MetatypeConversionExpr *E) {
printCommon(E, "metatype_conversion_expr") << '\n';
printRec(E->getSubExpr());
Expand Down
3 changes: 3 additions & 0 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4793,6 +4793,9 @@ void PrintAST::visitConstructorRefCallExpr(ConstructorRefCallExpr *expr) {
}
}

void PrintAST::visitABISafeConversionExpr(ABISafeConversionExpr *expr) {
}

void PrintAST::visitFunctionConversionExpr(FunctionConversionExpr *expr) {
}

Expand Down
39 changes: 39 additions & 0 deletions lib/AST/ASTVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,17 @@ class Verifier : public ASTWalker {
pushScope(DC);
}

/// Emit an error message and abort, optionally dumping the expression.
/// \param E if non-null, the expression to dump() followed by a new-line.
void error(llvm::StringRef msg, Expr *E = nullptr) {
Out << msg << "\n";
if (E) {
E->dump(Out);
Out << "\n";
}
abort();
}

public:
Verifier(ModuleDecl *M, DeclContext *DC)
: Verifier(PointerUnion<ModuleDecl *, SourceFile *>(M), DC) {}
Expand Down Expand Up @@ -2306,6 +2317,34 @@ class Verifier : public ASTWalker {
verifyCheckedBase(E);
}

void verifyChecked(ABISafeConversionExpr *E) {
PrettyStackTraceExpr debugStack(Ctx, "verify ABISafeConversionExpr", E);

auto toType = E->getType();
auto fromType = E->getSubExpr()->getType();

if (!fromType->hasLValueType())
error("conversion source must be an l-value", E);

if (!toType->hasLValueType())
error("conversion result must be an l-value", E);

{
// At the moment, "ABI Safe" means concurrency features can be stripped.
// Since we don't know how deeply the stripping is happening, to verify
// in a fuzzy way, strip everything to see if they're the same type.
auto strippedFrom = fromType->getRValueType()
->stripConcurrency(/*recurse*/true,
/*dropGlobalActor*/true);
auto strippedTo = toType->getRValueType()
->stripConcurrency(/*recurse*/true,
/*dropGlobalActor*/true);

if (!strippedFrom->isEqual(strippedTo))
error("possibly non-ABI safe conversion", E);
}
}

void verifyChecked(ValueDecl *VD) {
if (VD->getInterfaceType()->hasError()) {
Out << "checked decl cannot have error type\n";
Expand Down
3 changes: 3 additions & 0 deletions lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ ConcreteDeclRef Expr::getReferencedDecl(bool stopAtParenExpr) const {
PASS_THROUGH_REFERENCE(Load, getSubExpr);
NO_REFERENCE(DestructureTuple);
NO_REFERENCE(UnresolvedTypeConversion);
PASS_THROUGH_REFERENCE(ABISafeConversion, getSubExpr);
PASS_THROUGH_REFERENCE(FunctionConversion, getSubExpr);
PASS_THROUGH_REFERENCE(CovariantFunctionConversion, getSubExpr);
PASS_THROUGH_REFERENCE(CovariantReturnConversion, getSubExpr);
Expand Down Expand Up @@ -741,6 +742,7 @@ bool Expr::canAppendPostfixExpression(bool appendingPostfixOperator) const {
return false;

case ExprKind::Load:
case ExprKind::ABISafeConversion:
case ExprKind::DestructureTuple:
case ExprKind::UnresolvedTypeConversion:
case ExprKind::FunctionConversion:
Expand Down Expand Up @@ -914,6 +916,7 @@ bool Expr::isValidParentOfTypeExpr(Expr *typeExpr) const {
case ExprKind::Load:
case ExprKind::DestructureTuple:
case ExprKind::UnresolvedTypeConversion:
case ExprKind::ABISafeConversion:
case ExprKind::FunctionConversion:
case ExprKind::CovariantFunctionConversion:
case ExprKind::CovariantReturnConversion:
Expand Down
1 change: 1 addition & 0 deletions lib/SILGen/LValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class PathComponent {
CoroutineAccessorKind, // coroutine accessor
ValueKind, // random base pointer as an lvalue
PhysicalKeyPathApplicationKind, // applying a key path
ABISafeConversionKind, // unchecked_addr_cast

// Logical LValue kinds
GetterSetterKind, // property or subscript getter/setter
Expand Down
3 changes: 3 additions & 0 deletions lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,9 @@ namespace {
RValue visitArchetypeToSuperExpr(ArchetypeToSuperExpr *E, SGFContext C);
RValue visitUnresolvedTypeConversionExpr(UnresolvedTypeConversionExpr *E,
SGFContext C);
RValue visitABISafeConversionExpr(ABISafeConversionExpr *E, SGFContext C) {
llvm_unreachable("cannot appear in rvalue");
}
RValue visitFunctionConversionExpr(FunctionConversionExpr *E,
SGFContext C);
RValue visitCovariantFunctionConversionExpr(
Expand Down
37 changes: 37 additions & 0 deletions lib/SILGen/SILGenLValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenLValue
LValueOptions options);
LValue visitMoveExpr(MoveExpr *e, SGFAccessKind accessKind,
LValueOptions options);
LValue visitABISafeConversionExpr(ABISafeConversionExpr *e,
SGFAccessKind accessKind,
LValueOptions options);

// Expressions that wrap lvalues

Expand Down Expand Up @@ -2204,6 +2207,29 @@ namespace {
OS.indent(indent) << "PhysicalKeyPathApplicationComponent\n";
}
};

/// A physical component which performs an unchecked_addr_cast
class ABISafeConversionComponent final : public PhysicalPathComponent {
public:
ABISafeConversionComponent(LValueTypeData typeData)
: PhysicalPathComponent(typeData, ABISafeConversionKind,
/*actorIsolation=*/None) {}

ManagedValue project(SILGenFunction &SGF, SILLocation loc,
ManagedValue base) && override {
auto toType = SGF.getLoweredType(getTypeData().SubstFormalType)
.getAddressType();

if (base.getType() == toType)
return base; // nothing to do

return SGF.B.createUncheckedAddrCast(loc, base, toType);
}

void dump(raw_ostream &OS, unsigned indent) const override {
OS.indent(indent) << "ABISafeConversionComponent\n";
}
};
} // end anonymous namespace

RValue
Expand Down Expand Up @@ -3734,6 +3760,17 @@ LValue SILGenLValue::visitMoveExpr(MoveExpr *e, SGFAccessKind accessKind,
toAddr->getType().getASTType());
}

LValue SILGenLValue::visitABISafeConversionExpr(ABISafeConversionExpr *e,
SGFAccessKind accessKind,
LValueOptions options) {
LValue lval = visitRec(e->getSubExpr(), accessKind, options);
auto typeData = getValueTypeData(SGF, accessKind, e);

lval.add<ABISafeConversionComponent>(typeData);

return lval;
}

/// 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
37 changes: 27 additions & 10 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,24 @@ namespace {

auto &context = cs.getASTContext();

// For an RValue function type, use a standard function conversion.
if (openedType->is<AnyFunctionType>()) {
expr = new (context) FunctionConversionExpr(
expr, getNewType(adjustedOpenedType));
cs.cacheType(expr);
return expr;
}

// For any kind of LValue, use an ABISafeConversion.
if (openedType->hasLValueType()) {
assert(adjustedOpenedType->hasLValueType() && "lvalueness mismatch?");

expr = new (context) ABISafeConversionExpr(
expr, getNewType(adjustedOpenedType));
cs.cacheType(expr);
return expr;
}

// If we have an optional type, wrap it up in a monadic '?' and recurse.
if (Type objectType = openedType->getOptionalObjectType()) {
Type adjustedRefType = getNewType(adjustedOpenedType);
Expand All @@ -935,14 +953,6 @@ namespace {
return expr;
}

// For a function type, perform a function conversion.
if (openedType->is<AnyFunctionType>()) {
expr = new (context) FunctionConversionExpr(
expr, getNewType(adjustedOpenedType));
cs.cacheType(expr);
return expr;
}

assert(false && "Unhandled adjustment");
return expr;
}
Expand Down Expand Up @@ -1669,10 +1679,17 @@ namespace {
adjustedRefTy = adjustedRefTy->replaceCovariantResultType(
containerTy, 1);
}
cs.setType(memberRefExpr, refTy->castTo<FunctionType>()->getResult());

// \returns result of the given function type
auto resultType = [](Type fnTy) -> Type {
return fnTy->castTo<FunctionType>()->getResult();
};

cs.setType(memberRefExpr, resultType(refTy));

Expr *result = memberRefExpr;
result = adjustTypeForDeclReference(result, refTy, adjustedRefTy);
result = adjustTypeForDeclReference(result, resultType(refTy),
resultType(adjustedRefTy));
closeExistentials(result, locator);

// If the property is of dynamic 'Self' type, wrap an implicit
Expand Down
33 changes: 26 additions & 7 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,8 @@ ClosureIsolatedByPreconcurrency::operator()(const ClosureExpr *expr) const {

Type ConstraintSystem::getUnopenedTypeOfReference(
VarDecl *value, Type baseType, DeclContext *UseDC,
ConstraintLocator *memberLocator, bool wantInterfaceType) {
ConstraintLocator *memberLocator, bool wantInterfaceType,
bool adjustForPreconcurrency) {
return ConstraintSystem::getUnopenedTypeOfReference(
value, baseType, UseDC,
[&](VarDecl *var) -> Type {
Expand All @@ -1272,22 +1273,25 @@ Type ConstraintSystem::getUnopenedTypeOfReference(

return wantInterfaceType ? var->getInterfaceType() : var->getType();
},
memberLocator, wantInterfaceType, GetClosureType{*this},
memberLocator, wantInterfaceType, adjustForPreconcurrency,
GetClosureType{*this},
ClosureIsolatedByPreconcurrency{*this});
}

Type ConstraintSystem::getUnopenedTypeOfReference(
VarDecl *value, Type baseType, DeclContext *UseDC,
llvm::function_ref<Type(VarDecl *)> getType,
ConstraintLocator *memberLocator, bool wantInterfaceType,
ConstraintLocator *memberLocator,
bool wantInterfaceType, bool adjustForPreconcurrency,
llvm::function_ref<Type(const AbstractClosureExpr *)> getClosureType,
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency) {
Type requestedType =
getType(value)->getWithoutSpecifierType()->getReferenceStorageReferent();

// Adjust the type for concurrency.
requestedType = adjustVarTypeForConcurrency(
requestedType, value, UseDC, getClosureType, isolatedByPreconcurrency);
// Adjust the type for concurrency if requested.
if (adjustForPreconcurrency)
requestedType = adjustVarTypeForConcurrency(
requestedType, value, UseDC, getClosureType, isolatedByPreconcurrency);

// If we're dealing with contextual types, and we referenced this type from
// a different context, map the type.
Expand Down Expand Up @@ -2309,9 +2313,14 @@ ConstraintSystem::getTypeOfMemberReference(
FunctionType::ExtInfo info;
refType = FunctionType::get(indices, elementTy, info);
} else {
// Delay the adjustment for preconcurrency until after we've formed
// the function type for this kind of reference. Otherwise we will lose
// track of the adjustment in the formed function's return type.

refType = getUnopenedTypeOfReference(cast<VarDecl>(value), baseTy, useDC,
locator,
/*wantInterfaceType=*/true);
/*wantInterfaceType=*/true,
/*adjustForPreconcurrency=*/false);
}

auto selfTy = outerDC->getSelfInterfaceType();
Expand Down Expand Up @@ -2432,6 +2441,16 @@ ConstraintSystem::getTypeOfMemberReference(
openedType = adjustFunctionTypeForConcurrency(
origOpenedType->castTo<AnyFunctionType>(), subscript, useDC,
/*numApplies=*/2, /*isMainDispatchQueue=*/false, replacements);
} else if (auto var = dyn_cast<VarDecl>(value)) {
// Adjust the function's result type, since that's the Var's actual type.
auto origFnType = origOpenedType->castTo<AnyFunctionType>();

auto resultTy = adjustVarTypeForConcurrency(
origFnType->getResult(), var, useDC, GetClosureType{*this},
ClosureIsolatedByPreconcurrency{*this});

openedType = FunctionType::get(
origFnType->getParams(), resultTy, origFnType->getExtInfo());
}

// Compute the type of the reference.
Expand Down
9 changes: 9 additions & 0 deletions test/SILGen/Inputs/objc_preconcurrency.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
@import Foundation;

#define SENDABLE __attribute__((__swift_attr__("@Sendable")))

SENDABLE
@interface NSTouchGrass : NSObject
@property (nullable, copy) void (SENDABLE ^cancellationHandler)(void);
@property (nonnull, copy) void (SENDABLE ^exceptionHandler)(void);
@end
Loading