Merge pull request #60521 from kavon/preconcurrency-var-access

swift-ci · web-flow · commit a66951aa3da3 · 2022-08-30T00:58:49.000-07:00
Add missing function conversion for member access to preconcurrency vardecl.
diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h
@@ -3097,6 +3097,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.
diff --git a/include/swift/AST/ExprNodes.def b/include/swift/AST/ExprNodes.def
@@ -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)
diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h
@@ -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.
   ///
@@ -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();
diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp
@@ -2298,6 +2298,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());
diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp
@@ -4811,6 +4811,9 @@ void PrintAST::visitConstructorRefCallExpr(ConstructorRefCallExpr *expr) {
   }
 }
 
+void PrintAST::visitABISafeConversionExpr(ABISafeConversionExpr *expr) {
+}
+
 void PrintAST::visitFunctionConversionExpr(FunctionConversionExpr *expr) {
 }
 
diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp
@@ -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) {}
@@ -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";
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
@@ -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);
@@ -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:
@@ -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:
diff --git a/lib/SILGen/LValue.h b/lib/SILGen/LValue.h
@@ -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
diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
@@ -453,6 +453,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(
diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp
@@ -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
   
@@ -2194,6 +2197,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
@@ -3724,6 +3750,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,
diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -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);
@@ -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;
     }
@@ -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
diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp
@@ -1250,7 +1250,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 {
@@ -1263,22 +1264,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.
@@ -2302,9 +2306,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();
@@ -2425,6 +2434,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.
diff --git a/test/SILGen/Inputs/objc_preconcurrency.h b/test/SILGen/Inputs/objc_preconcurrency.h
@@ -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
diff --git a/test/SILGen/objc_preconcurrency.swift b/test/SILGen/objc_preconcurrency.swift

Original file line number	Diff line number	Diff line change
`@@ -4811,6 +4811,9 @@ void PrintAST::visitConstructorRefCallExpr(ConstructorRefCallExpr *expr) {`
`4811`	`4811`	`}`
`4812`	`4812`	`}`
`4813`	`4813`
	`4814`	`+void PrintAST::visitABISafeConversionExpr(ABISafeConversionExpr *expr) {`
	`4815`	`+}`
	`4816`	`+`
`4814`	`4817`	`void PrintAST::visitFunctionConversionExpr(FunctionConversionExpr *expr) {`
`4815`	`4818`	`}`
`4816`	`4819`