Skip to content

Commit

Permalink
[Distributed] Remove diagnostic emitting from macro, rely on compiler
Browse files Browse the repository at this point in the history
The macro cannot diagnose some situations, or rather, would diagnose too
aggressively, because it cannot inspect the type declarations of all
invokved types, and therefore we're unable to reliably report errors
only when necessary.

Originally I thought we don't want to emit macro code that "may fail to
compile" but we don't really have a choice.

This patch removes a manual diagnostic, and enables more correct code to
properly use @resolvable protocols.
  • Loading branch information
ktoso committed May 15, 2024
1 parent 4272552 commit b77685e
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,6 @@ extension DistributedResolvableMacro {
var isGenericStub = false
var specificActorSystemRequirement: TypeSyntax?

if proto.genericWhereClause == nil {
throw DiagnosticsError(
syntax: node,
message: """
Distributed protocol must declare actor system with SerializationRequirement, for example:
protocol Greeter<ActorSystem>: DistributedActor where ActorSystem: DistributedActorSystem<any Codable>
""", id: .invalidApplication)
}

let accessModifiers = proto.accessControlModifiers

for req in proto.genericWhereClause?.requirements ?? [] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ distributed actor Caplin {
typealias ActorSystem = FakeActorSystem
}

@Resolvable // expected-error{{Distributed protocol must declare actor system with SerializationRequirement}}
@Resolvable // expected-note 3{{in expansion of macro 'Resolvable' on protocol 'Fail' here}}
protocol Fail: DistributedActor {
distributed func method() -> String
}
Expand All @@ -35,7 +35,6 @@ protocol Fail: DistributedActor {
public protocol SomeRoot: DistributedActor, Sendable
where ActorSystem: DistributedActorSystem<any Codable> {

// TODO(distributed): we could diagnose this better?
associatedtype AssociatedSomething: Sendable // expected-note{{protocol requires nested type 'AssociatedSomething'; add nested type 'AssociatedSomething' for conformance}}
static var staticValue: String { get }
var value: String { get }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// REQUIRES: swift_swift_parser, asserts
//
// UNSUPPORTED: back_deploy_concurrency
// REQUIRES: concurrency
// REQUIRES: distributed
//
// RUN: %empty-directory(%t)
// RUN: %empty-directory(%t-scratch)

// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/../Inputs/FakeDistributedActorSystems.swift
// RUN: not %target-swift-frontend -typecheck -disable-availability-checking -plugin-path %swift-plugin-dir -parse-as-library -I %t %S/../Inputs/FakeDistributedActorSystems.swift -dump-macro-expansions %s 2>&1 | %FileCheck %s

import Distributed

// These tests check the error output inside of a "bad" expansion;
//
// Since we cannot nicely diagnose these problems from the macro itself,
// as it would need to inspect the type and its extensions and all involved
// protocols., as at least

// CHECK: macro expansion @Resolvable:1:[[COL:[0-9]+]]: error: distributed actor '$Fail' does not declare ActorSystem it can be used with
@Resolvable
protocol Fail: DistributedActor {
distributed func method() -> String
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,12 @@ public protocol GreeterMore: DistributedActor where ActorSystem == FakeActorSyst
// CHECK: }
// CHECK: }
// CHECK: }


// Should not fail:
public protocol MyActorWithSystemRequirement: DistributedActor where ActorSystem == FakeActorSystem {}

@Resolvable
public protocol MyActorWithSystemRequirementImpl: MyActorWithSystemRequirement {
distributed func example()
}

0 comments on commit b77685e

Please sign in to comment.