Skip to content

[Distributed] Only parse distributed when experimental mode enabled #38889

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 2 commits into from
Aug 17, 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
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -1839,6 +1839,11 @@ ERROR(attr_requires_concurrency, none,
"concurrency is enabled",
(StringRef, bool))

ERROR(attr_requires_distributed, none,
"'%0' %select{attribute|modifier}1 is only valid when experimental "
"distributed support is enabled",
(StringRef, bool))

//------------------------------------------------------------------------------
// MARK: syntax parsing diagnostics
//------------------------------------------------------------------------------
Expand Down
3 changes: 0 additions & 3 deletions include/swift/Frontend/Frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,6 @@ class CompilerInvocation {
/// imported.
bool shouldImportSwiftConcurrency() const;

/// Whether the Distributed support library should be implicitly imported.
bool shouldImportSwiftDistributed() const;

/// Performs input setup common to these tools:
/// sil-opt, sil-func-extractor, sil-llvm-gen, and sil-nm.
/// Return value includes the buffer so caller can keep it alive.
Expand Down
6 changes: 3 additions & 3 deletions include/swift/Runtime/RuntimeFunctions.def
Original file line number Diff line number Diff line change
Expand Up @@ -1692,15 +1692,15 @@ FUNCTION(DefaultActorDeallocateResilient,
// );
FUNCTION(DistributedActorInitializeRemote,
swift_distributedActor_remote_initialize, SwiftCC,
ConcurrencyAvailability,
ConcurrencyAvailability, // TODO(distributed): Introduce DistributedAvailability once shipping somewhere
RETURNS(OpaquePtrTy),
ARGS(TypeMetadataPtrTy),
ATTRS(NoUnwind))

// void swift_distributedActor_destroy(DefaultActor *actor); // TODO: ProxyActor *proxy?
// void swift_distributedActor_destroy(DefaultActor *actor);
FUNCTION(DistributedActorDestroy,
swift_distributedActor_destroy, SwiftCC,
ConcurrencyAvailability,
ConcurrencyAvailability, // TODO(distributed): Introduce DistributedAvailability once shipping somewhere
RETURNS(VoidTy),
ARGS(RefCountedPtrTy),
ATTRS(NoUnwind))
Expand Down
2 changes: 2 additions & 0 deletions include/swift/Strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ constexpr static const StringLiteral STDLIB_NAME = "Swift";
constexpr static const StringLiteral SWIFT_ONONE_SUPPORT = "SwiftOnoneSupport";
/// The name of the Concurrency module, which supports that extension.
constexpr static const StringLiteral SWIFT_CONCURRENCY_NAME = "_Concurrency";
/// The name of the Distributed module, which supports that extension.
constexpr static const StringLiteral SWIFT_DISTRIBUTED_NAME = "_Distributed";
Copy link
Member

Choose a reason for hiding this comment

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

I thought the module is called Distributed right now, with no underscore. The underscore is for implicitly-imported modules I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The import is _Distributed, same as _Concurrency was and it did have to be imported explicitly for a while.

/// The name of the SwiftShims module, which contains private stdlib decls.
constexpr static const StringLiteral SWIFT_SHIMS_NAME = "SwiftShims";
/// The name of the Builtin module, which contains Builtin functions.
Expand Down
6 changes: 0 additions & 6 deletions lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -779,12 +779,6 @@ bool CompilerInvocation::shouldImportSwiftConcurrency() const {
FrontendOptions::ParseInputMode::SwiftModuleInterface;
}

bool CompilerInvocation::shouldImportSwiftDistributed() const {
return getLangOptions().EnableExperimentalDistributed &&
getFrontendOptions().InputMode !=
FrontendOptions::ParseInputMode::SwiftModuleInterface;
}

/// Implicitly import the SwiftOnoneSupport module in non-optimized
/// builds. This allows for use of popular specialized functions
/// from the standard library, which makes the non-optimized builds
Expand Down
8 changes: 8 additions & 0 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1611,6 +1611,14 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
DiscardAttribute = true;
}

// If this attribute is only permitted when distributed is enabled, reject it.
if (DeclAttribute::isDistributedOnly(DK) &&
!shouldParseExperimentalDistributed()) {
diagnose(Loc, diag::attr_requires_distributed, AttrName,
DeclAttribute::isDeclModifier(DK));
DiscardAttribute = true;
}

Comment on lines +1614 to +1621
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this is the only thing that's actually needed to solve the issue. Everything else is just for availability things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove the availability support from this PR, thanks

if (Context.LangOpts.Target.isOSBinFormatCOFF()) {
if (DK == DAK_WeakLinked) {
diagnose(Loc, diag::attr_unsupported_on_target, AttrName,
Expand Down
23 changes: 15 additions & 8 deletions test/Distributed/distributed_actor_is_experimental.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,37 @@ actor SomeActor {}

@available(SwiftStdlib 5.5, *)
distributed actor DA {}
// expected-error@-1{{'_Distributed' module not imported, required for 'distributed actor'}}
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}

@available(SwiftStdlib 5.5, *)
distributed actor class DAC {} // expected-error{{distributed' can only be applied to 'actor' definitions, and distributed actor-isolated async functions}}
// expected-error@-1{{keyword 'class' cannot be used as an identifier here}}
distributed actor class DAC {}
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}
// expected-error@-2{{keyword 'class' cannot be used as an identifier here}}

actor A {
func normal() async {}
distributed func dist() {} // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
distributed func distAsync() async {} // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
distributed func dist() {}
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}
distributed func distAsync() async {}
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}

distributed var neverOk: String { // expected-error{{'distributed' modifier cannot be applied to this declaration}}
distributed var neverOk: String {
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}
"vars are not allowed to be distributed *ever* anyway"
}
}

@available(SwiftStdlib 5.5, *)
distributed actor DA2 {
// expected-error@-1{{'_Distributed' module not imported, required for 'distributed actor'}}
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}
func normal() async {}
distributed func dist() {}
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}
distributed func distAsync() async {}
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}

distributed var neverOk: String { // expected-error{{'distributed' modifier cannot be applied to this declaration}}
distributed var neverOk: String {
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}
"vars are not allowed to be distributed *ever* anyway"
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// RUN: %target-typecheck-verify-swift -disable-availability-checking -enable-experimental-distributed
// REQUIRES: concurrency
// REQUIRES: distributed

actor SomeActor {}

@available(SwiftStdlib 5.5, *)
distributed actor DA {}
// expected-error@-1{{'_Distributed' module not imported, required for 'distributed actor'}}

@available(SwiftStdlib 5.5, *)
distributed actor class DAC {}
// expected-error@-1{{distributed' can only be applied to 'actor' definitions, and distributed actor-isolated async functions}}
// expected-error@-2{{keyword 'class' cannot be used as an identifier here}}

actor A {
func normal() async {}
distributed func dist() {} // expected-error{{'distributed' function can only be declared within 'distributed actor'}}
distributed func distAsync() async {} // expected-error{{'distributed' function can only be declared within 'distributed actor'}}

distributed var neverOk: String { // expected-error{{'distributed' modifier cannot be applied to this declaration}}
"vars are not allowed to be distributed *ever* anyway"
}
}

@available(SwiftStdlib 5.5, *)
distributed actor DA2 {
// expected-error@-1{{'_Distributed' module not imported, required for 'distributed actor'}}
func normal() async {}
distributed func dist() {}
distributed func distAsync() async {}

distributed var neverOk: String { // expected-error{{'distributed' modifier cannot be applied to this declaration}}
"vars are not allowed to be distributed *ever* anyway"
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// RUN: %target-typecheck-verify-swift -disable-availability-checking
// ^^^^ notice the, on purpose, missing '-enable-experimental-distributed'
// REQUIRES: concurrency
// REQUIRES: distributed

import _Distributed

@available(macOS 12.0, *)
distributed actor A {}
// expected-error@-1{{'distributed' modifier is only valid when experimental distributed support is enabled}}
2 changes: 2 additions & 0 deletions tools/sil-opt/SILOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,8 @@ int main(int argc, char **argv) {
}
Invocation.getLangOptions().EnableExperimentalConcurrency =
EnableExperimentalConcurrency;
Invocation.getLangOptions().EnableExperimentalDistributed =
EnableExperimentalDistributed;

Invocation.getLangOptions().EnableObjCInterop =
EnableObjCInterop ? true :
Expand Down