[TaskLocals] Enforce @TaskLocal may only be used on static props

Author: ktoso

include/swift/AST/DiagnosticsSema.def

@@ -5606,6 +5606,11 @@ ERROR(property_wrapper_dynamic_self_type, none,
       "property wrapper %select{wrapped value|projected value}0 cannot have "
       "dynamic Self type", (bool))
 
+ERROR(property_wrapper_var_must_be_static, none,
+      "property %0, must be static because property wrapper %1 can only "
+      "be applied to static properties",
+      (Identifier, Type))
+
 ERROR(property_wrapper_attribute_not_on_property, none,
       "property wrapper attribute %0 can only be applied to a property",
       (Identifier))

include/swift/AST/PropertyWrappers.h

@@ -88,6 +88,10 @@ struct PropertyWrapperTypeInfo {
   /// ability to reason about the enclosing "self".
   SubscriptDecl *enclosingInstanceProjectedSubscript = nullptr;
 
+  /// Forces that the property wrapper must be declared on a static, or
+  /// global–once supported–property.
+  bool requireNoEnclosingInstance = false;
+
   ///
   /// Whether this is a valid property wrapper.
   bool isValid() const {

lib/Sema/TypeCheckPropertyWrapper.cpp

@@ -244,6 +244,30 @@ findSuitableWrapperInit(ASTContext &ctx, NominalTypeDecl *nominal,
   return viableInitializers.empty() ? nullptr : viableInitializers.front();
 }
 
+/// Returns true if the enclosingInstance parameter is `Never`,
+/// implying that there should be NO enclosing instance.
+static bool enclosingInstanceTypeIsNever(ASTContext &ctx, SubscriptDecl *subscript) {
+  if (!subscript)
+    return false;
+
+  ParameterList *indices = subscript->getIndices();
+  assert(indices && indices->size() > 0);
+
+  ParamDecl *param = indices->get(0);
+  if (param->getArgumentName() != ctx.Id_enclosingInstance)
+    return false;
+
+  auto paramTy = param->getType();
+  auto neverTy = ctx.getNeverType();
+  paramTy->dump();
+  paramTy->getCanonicalType()->dump();
+  neverTy->dump();
+  neverTy->getCanonicalType()->dump();
+  param->dump();
+  param->getType()->dump();
+  return neverTy->isEqual(paramTy);
+}
+
 /// Determine whether we have a suitable static subscript to which we
 /// can pass along the enclosing self + key-paths.
 static SubscriptDecl *findEnclosingSelfSubscript(ASTContext &ctx,
@@ -385,6 +409,16 @@ PropertyWrapperTypeInfoRequest::evaluate(
     }
   }
 
+  result.requireNoEnclosingInstance =
+      enclosingInstanceTypeIsNever(ctx, result.enclosingInstanceWrappedSubscript);
+//  if (requireNoEnclosingInstance) { //  && !valueVar->isStatic()) {
+//    // this means that the property wrapper must be declared on a static property
+//    valueVar->diagnose(
+//        diag::property_wrapper_var_must_be_static, valueVar->getName());
+//    return PropertyWrapperTypeInfo();
+//    result
+//  }
+
   bool hasInvalidDynamicSelf = false;
   if (result.projectedValueVar &&
       result.projectedValueVar->getValueInterfaceType()->hasDynamicSelfType()) {
@@ -439,6 +473,18 @@ AttachedPropertyWrappersRequest::evaluate(Evaluator &evaluator,
       continue;
     }
 
+//    // If the property wrapper requested an `_enclosingInstance: Never`
+//    // it must be declared as static. TODO: or global once we allow wrappers on top-level code
+//    auto wrappedInfo = var->getPropertyWrapperTypeInfo();
+//    if (wrappedInfo.requireNoEnclosingInstance) {
+//      if (!var->isStatic()) {
+//        ctx.Diags.diagnose(var->getLocation(),
+//                           diag::property_wrapper_var_must_be_static,
+//                           var->getName());
+//        continue;
+//      }
+//    }
+
     // Check that the variable is part of a single-variable pattern.
     auto binding = var->getParentPatternBinding();
     if (binding && binding->getSingleVar() != var) {
@@ -601,6 +647,20 @@ PropertyWrapperBackingPropertyTypeRequest::evaluate(
     wrappedValue->setInterfaceType(computeWrappedValueType(var, type));
   }
 
+  {
+    auto *nominal = type->getDesugaredType()->getAnyNominal();
+    if (auto wrappedInfo = nominal->getPropertyWrapperTypeInfo()) {
+      if (wrappedInfo.requireNoEnclosingInstance &&
+          !var->isStatic()) {
+        ctx.Diags.diagnose(var->getNameLoc(),
+                           diag::property_wrapper_var_must_be_static,
+                           var->getName(), type);
+        // TODO: fixit insert static?
+        return Type();
+      }
+    }
+  }
+
   return type;
 }
 

stdlib/private/StdlibUnittest/StdlibUnittest.swift

@@ -723,13 +723,6 @@ public func expectCrash(withMessage message: String = "", executing: () -> Void)
   fatalError()
 }
 
-public func expectCrashAsync(withMessage message: String = "", executing: () async -> Void) async -> Never {
-  expectCrashLater(withMessage: message)
-  await executing()
-  expectUnreachable()
-  fatalError()
-}
-
 func _defaultTestSuiteFailedCallback() {
   abort()
 }

stdlib/public/Concurrency/TaskLocal.swift

@@ -166,12 +166,28 @@ public final class TaskLocal<Value: Sendable>: Sendable, CustomStringConvertible
       self
     }
 
-    @available(*, unavailable, message: "use 'myTaskLocal.withValue(_:do:)' instead")
+    @available(*, unavailable, message: "use '$myTaskLocal.withValue(_:do:)' instead")
     set {
       fatalError("Illegal attempt to set a \(Self.self) value, use `withValue(...) { ... }` instead.")
     }
   }
 
+  // This subscript is used to enforce that the property wrapper may only be used
+  // on static (or rather, "without enclosing instance") properties.
+  // This is done by marking the `_enclosingInstance` as `Never` which informs
+  // the type-checker that this property-wrapper never wants to have an enclosing
+  // instance (it is impossible to declare a property wrapper inside the `Never`
+  // type).
+  public static subscript(
+    _enclosingInstance object: Never,
+    wrapped wrappedKeyPath: ReferenceWritableKeyPath<Never, Value>,
+    storage storageKeyPath: ReferenceWritableKeyPath<Never, TaskLocal<Value>>
+  ) -> Value {
+    get {
+      fatalError()
+    }
+  }
+
   public var wrappedValue: Value {
     self.get()
   }

test/Concurrency/task_local.swift

@@ -2,17 +2,19 @@
 // REQUIRES: concurrency
 
 @available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
-enum TL {
+struct TL {
   @TaskLocal
   static var number: Int = 0
 
   @TaskLocal
   static var someNil: Int?
 
-  // TODO: ban non-static task-local definitions
-//  @TaskLocal(default: 0)
-//  @available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *)
-//  var notStatic
+  @TaskLocal
+  static var noValue: Int // expected-error{{'static var' declaration requires an initializer expression or an explicitly stated getter}}
+  // expected-note@-1{{add an initializer to silence this error}}
+
+  @TaskLocal
+  var notStatic: String? // expected-error{{property 'notStatic', must be static because property wrapper 'TaskLocal<String?>' can only be applied to static properties}}
 }
 
 @TaskLocal // expected-error{{property wrappers are not yet supported in top-level code}}