Skip to content

[concurrency] Implement effectful properties #36430

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 13 commits into from
Mar 26, 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
38 changes: 35 additions & 3 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4521,6 +4521,34 @@ class AbstractStorageDecl : public ValueDecl {
return {};
}

/// This is the primary mechanism by which we can easily determine whether
/// this storage decl has any effects.
///
/// \returns the getter decl iff this decl has only one accessor that is
/// a 'get' with an effect (i.e., 'async', 'throws', or both).
/// Otherwise returns nullptr.
AccessorDecl *getEffectfulGetAccessor() const;

/// Performs a "limit check" on an effect possibly exhibited by this storage
/// decl with respect to some other storage decl that serves as the "limit."
/// This check says that \c this is less effectful than \c other if
/// \c this either does not exhibit the effect, or if it does, then \c other
/// also exhibits the effect. Thus, it is conceptually equivalent to
/// a less-than-or-equal (≤) check like so:
///
/// \verbatim
///
/// this->hasEffect(E) ≤ other->hasEffect(E)
///
/// \endverbatim
///
/// \param kind the single effect we are performing a check for.
///
/// \returns true iff \c this decl either does not exhibit the effect,
/// or \c other also exhibits the effect.
bool isLessEffectfulThan(AbstractStorageDecl const* other,
EffectKind kind) const;

/// Return an accessor that this storage is expected to have, synthesizing
/// one if necessary. Note that will always synthesize one, even if the
/// accessor is not part of the expected opaque set for the storage, so use
Expand Down Expand Up @@ -6327,16 +6355,18 @@ class AccessorDecl final : public FuncDecl {
AccessorDecl(SourceLoc declLoc, SourceLoc accessorKeywordLoc,
AccessorKind accessorKind, AbstractStorageDecl *storage,
SourceLoc staticLoc, StaticSpellingKind staticSpelling,
bool throws, SourceLoc throwsLoc,
bool async, SourceLoc asyncLoc, bool throws, SourceLoc throwsLoc,
bool hasImplicitSelfDecl, GenericParamList *genericParams,
DeclContext *parent)
: FuncDecl(DeclKind::Accessor,
staticLoc, staticSpelling, /*func loc*/ declLoc,
/*name*/ Identifier(), /*name loc*/ declLoc,
/*Async=*/false, SourceLoc(), throws, throwsLoc,
async, asyncLoc, throws, throwsLoc,
hasImplicitSelfDecl, genericParams, parent),
AccessorKeywordLoc(accessorKeywordLoc),
Storage(storage) {
assert(!async || accessorKind == AccessorKind::Get
&& "only get accessors can be async");
Bits.AccessorDecl.AccessorKind = unsigned(accessorKind);
}

Expand All @@ -6347,6 +6377,7 @@ class AccessorDecl final : public FuncDecl {
AbstractStorageDecl *storage,
SourceLoc staticLoc,
StaticSpellingKind staticSpelling,
bool async, SourceLoc asyncLoc,
bool throws, SourceLoc throwsLoc,
GenericParamList *genericParams,
DeclContext *parent,
Expand All @@ -6365,7 +6396,7 @@ class AccessorDecl final : public FuncDecl {
AccessorKind accessorKind,
AbstractStorageDecl *storage,
StaticSpellingKind staticSpelling,
bool throws,
bool async, bool throws,
GenericParamList *genericParams,
Type fnRetType, DeclContext *parent);

Expand All @@ -6375,6 +6406,7 @@ class AccessorDecl final : public FuncDecl {
AbstractStorageDecl *storage,
SourceLoc staticLoc,
StaticSpellingKind staticSpelling,
bool async, SourceLoc asyncLoc,
bool throws, SourceLoc throwsLoc,
GenericParamList *genericParams,
ParameterList *parameterList,
Expand Down
6 changes: 6 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,12 @@ ERROR(expected_lbrace_accessor,PointsToFirstBadToken,
ERROR(expected_accessor_kw,none,
"expected 'get', 'set', 'willSet', or 'didSet' keyword to "
"start an accessor definition",())
ERROR(invalid_accessor_specifier,none,
"'%0' accessor cannot have specifier '%1'",
(StringRef, StringRef))
ERROR(invalid_accessor_with_effectful_get,none,
"'%0' accessor is not allowed on property with 'get' accessor that is 'async' or 'throws'",
(StringRef))
ERROR(missing_getter,none,
"%select{variable|subscript}0 with %1 must also have a getter",
(unsigned, StringRef))
Expand Down
52 changes: 35 additions & 17 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1584,6 +1584,10 @@ WARNING(enum_frozen_nonpublic,none,
ERROR(getset_init,none,
"variable with getter/setter cannot have an initial value", ())

ERROR(effectful_not_representable_objc,none,
"%0 with 'throws' or 'async' is not representable in Objective-C",
(DescriptiveDeclKind))

ERROR(unimplemented_static_var,none,
"%select{ERROR|static|class}1 stored properties not supported"
"%select{ in this context| in generic types| in classes| in protocol extensions}0"
Expand Down Expand Up @@ -2600,6 +2604,10 @@ ERROR(override_class_declaration_in_extension,none,
ERROR(override_throws,none,
"cannot override non-throwing %select{method|initializer}0 with "
"throwing %select{method|initializer}0", (bool))
// TODO: this really could be merged with the above.
ERROR(override_with_more_effects,none,
"cannot override non-'%1' %0 with '%1' %0",
(DescriptiveDeclKind, StringRef))
ERROR(override_throws_objc,none,
"overriding a throwing @objc %select{method|initializer}0 with "
"a non-throwing %select{method|initializer}0 is not supported", (bool))
Expand Down Expand Up @@ -4150,21 +4158,21 @@ ERROR(isa_collection_downcast_pattern_value_unimplemented,none,
ERROR(try_unhandled,none,
"errors thrown from here are not handled", ())
ERROR(throwing_call_unhandled,none,
"call can throw, but the error is not handled", ())
"%0 can throw, but the error is not handled", (StringRef))
ERROR(tryless_throwing_call_unhandled,none,
"call can throw, but it is not marked with 'try' and "
"the error is not handled", ())
"%0 can throw, but it is not marked with 'try' and "
"the error is not handled", (StringRef))
ERROR(throw_in_nonthrowing_function,none,
"error is not handled because the enclosing function "
"is not declared 'throws'", ())

ERROR(throwing_call_in_rethrows_function,none,
"call can throw, but the error is not handled; a function declared "
"'rethrows' may only throw if its parameter does", ())
"%0 can throw, but the error is not handled; a function declared "
"'rethrows' may only throw if its parameter does", (StringRef))
ERROR(tryless_throwing_call_in_rethrows_function,none,
"call can throw, but it is not marked with 'try' and "
"%0 can throw, but it is not marked with 'try' and "
"the error is not handled; a function declared "
"'rethrows' may only throw if its parameter does", ())
"'rethrows' may only throw if its parameter does", (StringRef))
ERROR(throw_in_rethrows_function,none,
"a function declared 'rethrows' may only throw if its parameter does", ())
NOTE(because_rethrows_argument_throws,none,
Expand All @@ -4176,11 +4184,11 @@ NOTE(because_rethrows_conformance_throws,none,
"call is to 'rethrows' function, but a conformance has a throwing witness", ())

ERROR(throwing_call_in_nonthrowing_autoclosure,none,
"call can throw, but it is executed in a non-throwing "
"autoclosure",())
"%0 can throw, but it is executed in a non-throwing "
"autoclosure",(StringRef))
ERROR(tryless_throwing_call_in_nonthrowing_autoclosure,none,
"call can throw, but it is not marked with 'try' and "
"it is executed in a non-throwing autoclosure",())
"%0 can throw, but it is not marked with 'try' and "
"it is executed in a non-throwing autoclosure",(StringRef))
ERROR(throw_in_nonthrowing_autoclosure,none,
"error is not handled because it is thrown in a non-throwing "
"autoclosure", ())
Expand All @@ -4189,17 +4197,17 @@ ERROR(try_unhandled_in_nonexhaustive_catch,none,
"errors thrown from here are not handled because the "
"enclosing catch is not exhaustive", ())
ERROR(throwing_call_in_nonexhaustive_catch,none,
"call can throw, but the enclosing catch is not exhaustive", ())
"%0 can throw, but the enclosing catch is not exhaustive", (StringRef))
ERROR(tryless_throwing_call_in_nonexhaustive_catch,none,
"call can throw, but it is not marked with 'try' and "
"the enclosing catch is not exhaustive", ())
"%0 can throw, but it is not marked with 'try' and "
"the enclosing catch is not exhaustive", (StringRef))
ERROR(throw_in_nonexhaustive_catch,none,
"error is not handled because the enclosing catch is not exhaustive", ())

ERROR(throwing_call_in_illegal_context,none,
"call can throw, but errors cannot be thrown out of "
ERROR(throwing_op_in_illegal_context,none,
"%1 can throw, but errors cannot be thrown out of "
"%select{<<ERROR>>|a default argument|a property wrapper initializer|a property initializer|a global variable initializer|an enum case raw value|a catch pattern|a catch guard expression|a defer body}0",
(unsigned))
(unsigned, StringRef))
ERROR(throw_in_illegal_context,none,
"errors cannot be thrown out of "
"%select{<<ERROR>>|a default argument|a property wrapper initializer|a property initializer|a global variable initializer|an enum case raw value|a catch pattern|a catch guard expression|a defer body}0",
Expand All @@ -4213,6 +4221,10 @@ ERROR(throwing_call_without_try,none,
"call can throw but is not marked with 'try'", ())
ERROR(throwing_async_let_without_try,none,
"reading 'async let' can throw but is not marked with 'try'", ())
ERROR(throwing_prop_access_without_try,none,
"property access can throw but is not marked with 'try'", ())
ERROR(throwing_subscript_access_without_try,none,
"subscript access can throw but is not marked with 'try'", ())
NOTE(note_forgot_try,none,
"did you mean to use 'try'?", ())
NOTE(note_error_to_optional,none,
Expand Down Expand Up @@ -4382,6 +4394,9 @@ ERROR(actor_isolated_from_escaping_closure,none,
ERROR(actor_isolated_keypath_component,none,
"cannot form key path to actor-isolated %0 %1",
(DescriptiveDeclKind, DeclName))
ERROR(effectful_keypath_component,none,
"cannot form key path to %0 with 'throws' or 'async'",
(DescriptiveDeclKind))
ERROR(local_function_executed_concurrently,none,
"concurrently-executed %0 %1 must be marked as '@Sendable'",
(DescriptiveDeclKind, DeclName))
Expand Down Expand Up @@ -5615,6 +5630,9 @@ ERROR(property_wrapper_let, none,
ERROR(property_wrapper_computed, none,
"property wrapper cannot be applied to a computed property",
())
ERROR(property_wrapper_effectful,none,
"property wrappers currently cannot define an 'async' or 'throws' accessor",
())

ERROR(property_with_wrapper_conflict_attribute,none,
"property %0 with a wrapper cannot also be "
Expand Down
18 changes: 18 additions & 0 deletions include/swift/AST/StorageImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ enum class AccessorKind {
#define ACCESSOR(ID) ID,
#define LAST_ACCESSOR(ID) Last = ID
#include "swift/AST/AccessorKinds.def"
#undef ACCESSOR
#undef LAST_ACCESSOR
};

const unsigned NumAccessorKinds = unsigned(AccessorKind::Last) + 1;
Expand All @@ -54,6 +56,22 @@ static inline IntRange<AccessorKind> allAccessorKinds() {
AccessorKind(NumAccessorKinds));
}

/// \returns a user-readable string name for the accessor kind
static inline StringRef accessorKindName(AccessorKind ak) {
switch(ak) {

#define ACCESSOR(ID) ID
#define SINGLETON_ACCESSOR(ID, KEYWORD) \
case AccessorKind::ID: \
return #KEYWORD;

#include "swift/AST/AccessorKinds.def"

#undef ACCESSOR_KEYWORD
#undef SINGLETON_ACCESSOR
}
}

/// Whether an access to storage is for reading, writing, or both.
enum class AccessKind : uint8_t {
/// The access is just to read the current value.
Expand Down
6 changes: 6 additions & 0 deletions include/swift/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -1158,6 +1158,12 @@ class Parser {
bool hasInitializer,
const DeclAttributes &Attributes,
SmallVectorImpl<Decl *> &Decls);
ParserStatus parseGetEffectSpecifier(ParsedAccessors &accessors,
SourceLoc &asyncLoc,
SourceLoc &throwsLoc,
bool &hasEffectfulGet,
AccessorKind currentKind,
SourceLoc const& currentLoc);

void consumeAbstractFunctionBody(AbstractFunctionDecl *AFD,
const DeclAttributes &Attrs);
Expand Down
69 changes: 53 additions & 16 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,24 @@ static StaticSpellingKind getCorrectStaticSpelling(const Decl *D) {
}
}

static bool hasAsyncGetter(const AbstractStorageDecl *ASD) {
if (auto getter = ASD->getAccessor(AccessorKind::Get)) {
assert(!getter->getAttrs().hasAttribute<ReasyncAttr>());
return getter->hasAsync();
}

return false;
}

static bool hasThrowsGetter(const AbstractStorageDecl *ASD) {
if (auto getter = ASD->getAccessor(AccessorKind::Get)) {
assert(!getter->getAttrs().hasAttribute<RethrowsAttr>());
return getter->hasThrows();
}

return false;
}

static bool hasMutatingGetter(const AbstractStorageDecl *ASD) {
return ASD->getAccessor(AccessorKind::Get) && ASD->isGetterMutating();
}
Expand Down Expand Up @@ -1925,6 +1943,15 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
return;
}

// prints with a space prefixed
auto printWithSpace = [&](StringRef word) {
Printer << " ";
Printer.printKeyword(word, Options);
};

const bool asyncGet = hasAsyncGetter(ASD);
const bool throwsGet = hasThrowsGetter(ASD);

// We sometimes want to print the accessors abstractly
// instead of listing out how they're actually implemented.
bool inProtocol = isa<ProtocolDecl>(ASD->getDeclContext());
Expand All @@ -1935,27 +1962,27 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
bool nonmutatingSetter = hasNonMutatingSetter(ASD);

// We're about to print something like this:
// { mutating? get (nonmutating? set)? }
// { mutating? get async? throws? (nonmutating? set)? }
// But don't print "{ get set }" if we don't have to.
if (!inProtocol && !Options.PrintGetSetOnRWProperties &&
settable && !mutatingGetter && !nonmutatingSetter) {
settable && !mutatingGetter && !nonmutatingSetter
&& !asyncGet && !throwsGet) {
return;
}

Printer << " {";
if (mutatingGetter) {
Printer << " ";
Printer.printKeyword("mutating", Options);
}
Printer << " ";
Printer.printKeyword("get", Options);
if (mutatingGetter) printWithSpace("mutating");

printWithSpace("get");

if (asyncGet) printWithSpace("async");

if (throwsGet) printWithSpace("throws");

if (settable) {
if (nonmutatingSetter) {
Printer << " ";
Printer.printKeyword("nonmutating", Options);
}
Printer << " ";
Printer.printKeyword("set", Options);
if (nonmutatingSetter) printWithSpace("nonmutating");

printWithSpace("set");
}
Printer << " }";
return;
Expand Down Expand Up @@ -1983,7 +2010,8 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
!Options.PrintGetSetOnRWProperties &&
!Options.FunctionDefinitions &&
!ASD->isGetterMutating() &&
!ASD->getAccessor(AccessorKind::Set)->isExplicitNonMutating()) {
!ASD->getAccessor(AccessorKind::Set)->isExplicitNonMutating() &&
!asyncGet && !throwsGet) {
return;
}

Expand All @@ -1999,7 +2027,15 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
if (!PrintAccessorBody) {
Printer << " ";
printMutabilityModifiersIfNeeded(Accessor);

Printer.printKeyword(getAccessorLabel(Accessor->getAccessorKind()), Options);

// handle any effects specifiers
if (Accessor->getAccessorKind() == AccessorKind::Get) {
if (asyncGet) printWithSpace("async");
if (throwsGet) printWithSpace("throws");
}

} else {
{
IndentRAII IndentMore(*this);
Expand All @@ -2017,7 +2053,8 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
bool isOnlyGetter = impl.getReadImpl() == ReadImplKind::Get &&
ASD->getAccessor(AccessorKind::Get);
bool isGetterMutating = ASD->supportsMutation() || ASD->isGetterMutating();
if (isOnlyGetter && !isGetterMutating && PrintAccessorBody &&
bool hasEffects = asyncGet || throwsGet;
if (isOnlyGetter && !isGetterMutating && !hasEffects && PrintAccessorBody &&
Options.FunctionBody && Options.CollapseSingleGetterProperty) {
Options.FunctionBody(ASD->getAccessor(AccessorKind::Get), Printer);
indent();
Expand Down
Loading