Skip to content

[cherry-pick 5.5] Task locals revisions #37158

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 14 commits into from
Apr 30, 2021
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
11 changes: 5 additions & 6 deletions include/swift/ABI/Task.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class alignas(2 * alignof(void*)) Job : public HeapObject {
NextWaitingTaskIndex = 0,

// The Dispatch object header is one pointer and two ints, which is
// equvialent to three pointers on 32-bit and two pointers 64-bit. Set the
// equivalent to three pointers on 32-bit and two pointers 64-bit. Set the
// indexes accordingly so that DispatchLinkageIndex points to where Dispatch
// expects.
DispatchHasLongObjectHeader = sizeof(void *) == sizeof(int),
Expand Down Expand Up @@ -217,14 +217,13 @@ class AsyncTask : public Job {

// ==== Task Local Values ----------------------------------------------------

void localValuePush(const Metadata *keyType,
void localValuePush(const HeapObject *key,
/* +1 */ OpaqueValue *value, const Metadata *valueType) {
Local.pushValue(this, keyType, value, valueType);
Local.pushValue(this, key, value, valueType);
}

OpaqueValue* localValueGet(const Metadata *keyType,
TaskLocal::TaskLocalInheritance inherit) {
return Local.getValue(this, keyType, inherit);
OpaqueValue* localValueGet(const HeapObject *key) {
return Local.getValue(this, key);
}

void localValuePop() {
Expand Down
65 changes: 20 additions & 45 deletions include/swift/ABI/TaskLocal.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,73 +34,50 @@ class TaskLocal {
public:
/// Type of the pointed at `next` task local item.
enum class NextLinkType : uintptr_t {
/// This task is known to be a "terminal" node in the lookup of task locals.
/// In other words, even if it had a parent, the parent (and its parents)
/// are known to not contain any any more task locals, and thus any further
/// search beyond this task.
IsTerminal = 0b00,
/// The storage pointer points at the next TaskLocal::Item in this task.
IsNext = 0b01,
IsNext = 0b00,
/// The storage pointer points at a item stored by another AsyncTask.
///
/// Note that this may not necessarily be the same as the task's parent
/// task -- we may point to a super-parent if we know / that the parent
/// does not "contribute" any task local values. This is to speed up
/// lookups by skipping empty parent tasks during get(), and explained
/// in depth in `createParentLink`.
IsParent = 0b11
};

/// Values must match `TaskLocalInheritance` declared in `TaskLocal.swift`.
enum class TaskLocalInheritance : uint8_t {
/// Default task local value behavior
///
/// Values declared with a default-inherited key are accessible from:
/// - the current task that has bound the value,
/// - any child task of the current task (e.g. created by async let or groups)
///
/// Such values are *not* carried through detached tasks.
Default = 0,

/// Special semantics which confine a task's local value to *only* the current
/// task. In other words, they ave never inherited by any child task created
/// by the current task.
///
/// Values declared with a never-inherited key only accessible:
/// - specifically from the current task itself
///
/// Such values are *not* accessible from child tasks or detached tasks.
Never = 1
IsParent = 0b01,
};

class Item {
private:
/// Mask used for the low status bits in a task local chain item.
static const uintptr_t statusMask = 0x03;

/// Pointer to the next task local item; be it in this task or in a parent.
/// Low bits encode `NextLinkType`.
/// Item *next = nullptr;
/// Pointer to one of the following:
/// - next task local item as OpaqueValue* if it is task-local allocated
/// - next task local item as HeapObject* if it is heap allocated "heavy"
/// - the parent task's TaskLocal::Storage
///
/// Low bits encode `NextLinkType`, based on which the type of the pointer
/// is determined.
uintptr_t next;

public:
/// The type of the key with which this value is associated.
const Metadata *keyType;
const HeapObject *key;
/// The type of the value stored by this item.
const Metadata *valueType;

// Trailing storage for the value itself. The storage will be
// uninitialized or contain an instance of \c valueType.

private:
protected:
explicit Item()
: next(0),
keyType(nullptr),
key(nullptr),
valueType(nullptr) {}

explicit Item(const Metadata *keyType, const Metadata *valueType)
explicit Item(const HeapObject *key, const Metadata *valueType)
: next(0),
keyType(keyType),
key(key),
valueType(valueType) {}

public:
Expand All @@ -116,7 +93,7 @@ class TaskLocal {
static Item *createParentLink(AsyncTask *task, AsyncTask *parent);

static Item *createLink(AsyncTask *task,
const Metadata *keyType,
const HeapObject *key,
const Metadata *valueType);

void destroy(AsyncTask *task);
Expand All @@ -125,13 +102,13 @@ class TaskLocal {
return reinterpret_cast<Item *>(next & ~statusMask);
}

NextLinkType getNextLinkType() {
NextLinkType getNextLinkType() const {
return static_cast<NextLinkType>(next & statusMask);
}

/// Item does not contain any actual value, and is only used to point at
/// a specific parent item.
bool isEmpty() {
bool isEmpty() const {
return !valueType;
}

Expand All @@ -144,6 +121,7 @@ class TaskLocal {
/// Compute the offset of the storage from the base of the item.
static size_t storageOffset(const Metadata *valueType) {
size_t offset = sizeof(Item);

if (valueType) {
size_t alignment = valueType->vw_alignment();
return (offset + alignment - 1) & ~(alignment - 1);
Expand All @@ -162,7 +140,6 @@ class TaskLocal {
}
};


class Storage {
friend class TaskLocal::Item;
private:
Expand Down Expand Up @@ -202,12 +179,10 @@ class TaskLocal {
void initializeLinkParent(AsyncTask *task, AsyncTask *parent);

void pushValue(AsyncTask *task,
const Metadata *keyType,
const HeapObject *key,
/* +1 */ OpaqueValue *value, const Metadata *valueType);

OpaqueValue* getValue(AsyncTask *task,
const Metadata *keyType,
TaskLocalInheritance inheritance);
OpaqueValue* getValue(AsyncTask *task, const HeapObject *key);

void popValue(AsyncTask *task);

Expand Down
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Comment on lines +5610 to +5611
Copy link
Member

Choose a reason for hiding this comment

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

Might be simpler to just say: "property wrapper ___ can only be applied to static properties"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I wanted to include the name of the property, you sure we'd prefer the shorter message?

Copy link
Member

Choose a reason for hiding this comment

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

I think the name of the property isn't needed, because the error message will already be pointing to the var decl with the problematic wrapper on it, whether in the terminal or xcode.

(Identifier, Type))

ERROR(property_wrapper_attribute_not_on_property, none,
"property wrapper attribute %0 can only be applied to a property",
(Identifier))
Expand Down
2 changes: 2 additions & 0 deletions include/swift/AST/KnownSDKTypes.def
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,6 @@ KNOWN_SDK_TYPE_DECL(ObjectiveC, ObjCBool, StructDecl, 0)
KNOWN_SDK_TYPE_DECL(Concurrency, UnsafeContinuation, NominalTypeDecl, 2)
KNOWN_SDK_TYPE_DECL(Concurrency, MainActor, NominalTypeDecl, 0)

KNOWN_SDK_TYPE_DECL(Concurrency, TaskLocal, ClassDecl, 1)

#undef KNOWN_SDK_TYPE_DECL
4 changes: 4 additions & 0 deletions include/swift/AST/PropertyWrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
26 changes: 19 additions & 7 deletions include/swift/Runtime/Concurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,20 @@ bool swift_task_tryAddStatusRecord(TaskStatusRecord *record);
///
/// The given record need not be the last record added to
/// the task, but the operation may be less efficient if not.
///s
///
/// Returns false if the task has been cancelled.
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
bool swift_task_removeStatusRecord(TaskStatusRecord *record);

/// Signifies whether the current task is in the middle of executing the
/// operation block of a `with(Throwing)TaskGroup(...) { <operation> }`.
///
/// Task local values must use un-structured allocation for values bound in this
/// scope, as they may be referred to by `group.spawn`-ed tasks and therefore
/// out-life the scope of a task-local value binding.
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
bool swift_task_hasTaskGroupStatusRecord();

/// Attach a child task to its parent task and return the newly created
/// `ChildTaskStatusRecord`.
///
Expand Down Expand Up @@ -339,20 +348,23 @@ SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
void swift_task_removeCancellationHandler(
CancellationNotificationStatusRecord *record);

/// Report error about attempting to bind a task-local value from an illegal context.
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
void swift_task_reportIllegalTaskLocalBindingWithinWithTaskGroup(
const unsigned char *file, uintptr_t fileLength,
bool fileIsASCII, uintptr_t line);

/// Get a task local value from the passed in task. Its Swift signature is
///
/// \code
/// func _taskLocalValueGet<Key>(
/// _ task: Builtin.NativeObject,
/// keyType: Any.Type /*Key.Type*/,
/// inheritance: UInt8/*TaskLocalInheritance*/
/// keyType: Any.Type /*Key.Type*/
/// ) -> UnsafeMutableRawPointer? where Key: TaskLocalKey
/// \endcode
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
OpaqueValue*
swift_task_localValueGet(AsyncTask* task,
const Metadata *keyType,
TaskLocal::TaskLocalInheritance inheritance);
swift_task_localValueGet(AsyncTask* task, const HeapObject *key);

/// Add a task local value to the passed in task.
///
Expand All @@ -369,7 +381,7 @@ swift_task_localValueGet(AsyncTask* task,
/// \endcode
SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
void swift_task_localValuePush(AsyncTask* task,
const Metadata *keyType,
const HeapObject *key,
/* +1 */ OpaqueValue *value,
const Metadata *valueType);

Expand Down
56 changes: 55 additions & 1 deletion lib/Sema/TypeCheckPropertyWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,24 @@ 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();
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,
Expand Down Expand Up @@ -385,6 +403,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
// }
Comment on lines +408 to +414
Copy link
Member

Choose a reason for hiding this comment

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

Why is this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will remove -- this is a leftover, it was the wrong place to do the diagnosis after all so moved elsewhere


bool hasInvalidDynamicSelf = false;
if (result.projectedValueVar &&
result.projectedValueVar->getValueInterfaceType()->hasDynamicSelfType()) {
Expand Down Expand Up @@ -439,6 +467,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) {
Expand Down Expand Up @@ -508,7 +548,7 @@ AttachedPropertyWrappersRequest::evaluate(Evaluator &evaluator,
continue;
}
}

result.push_back(mutableAttr);
}

Expand Down Expand Up @@ -601,6 +641,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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,25 @@ OVERRIDE_TASK_GROUP(taskGroup_addPending, bool,
swift::, (TaskGroup *group, bool unconditionally),
(group, unconditionally))


OVERRIDE_TASK_LOCAL(task_reportIllegalTaskLocalBindingWithinWithTaskGroup, void,
SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift), swift::,
(const unsigned char *file, uintptr_t fileLength,
bool fileIsASCII, uintptr_t line),
(file, fileLength, fileIsASCII, line))

OVERRIDE_TASK_LOCAL(task_localValuePush, void,
SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
swift::,
(AsyncTask *task, const Metadata *keyType,
(AsyncTask *task, const HeapObject *key,
OpaqueValue *value, const Metadata *valueType),
(task, keyType, value, valueType))
(task, key, value, valueType))

OVERRIDE_TASK_LOCAL(task_localValueGet, OpaqueValue *,
SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
swift::,
(AsyncTask *task, const Metadata *keyType,
TaskLocal::TaskLocalInheritance inheritance),
(task, keyType, inheritance))
(AsyncTask *task, const HeapObject *key),
(task, key))

OVERRIDE_TASK_LOCAL(task_localValuePop, void,
SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
Expand All @@ -213,6 +219,10 @@ OVERRIDE_TASK_STATUS(task_removeStatusRecord, bool,
SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
swift::, (TaskStatusRecord *record), (record))

OVERRIDE_TASK_STATUS(task_hasTaskGroupStatusRecord, bool,
SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
swift::, , )

OVERRIDE_TASK_STATUS(task_attachChild, ChildTaskStatusRecord *,
SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift),
swift::, (AsyncTask *child), (child))
Expand Down
Loading