Skip to content

Conversation

@ktoso
Copy link
Member

@ktoso ktoso commented Jun 13, 2020

"Actor (message) functions" MUST now mark themselfes as @Actor

Screen Shot 2020-06-13 at 11 29 59

This also paves the way forward to what we really want here: function wrappers.

Motivation:

previously we lacked any control over what is generated and we'd generate messages for any method on an actorable except if it was __ prefixed.

This does not work well in real world where actorables may conform to various protocols and have to implement them.

It is NOT enough to just rely on public/internal/private because we may need to have "private messages" etc.

This solves this FIXME as well:

Screen Shot 2020-06-13 at 11 25 51

Modifications:

  • require that functions must annotate with an @actor comment in order to be source generated as messages
  • fixes how @escaping is treated -- we are able to have closures in local messages now (even tho yes its a bad idea but sometimes useful if the closure is pure)

@ktoso ktoso requested review from budde, drexin, tomerd and yim-lee June 13, 2020 02:29
@ktoso
Copy link
Member Author

ktoso commented Jun 13, 2020

I think this is very much "better" than the current status which was "opt out" (by __ prefixing function names) as it would completely break if the actorable also had to conform to some other protocol.


extension GeneratedActor.Messages.GreetingsService {
// TODO: Check with Swift team which style of discriminator to aim for
public enum DiscriminatorKeys: String, Decodable {
Copy link
Member

Choose a reason for hiding this comment

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

This is empty now?

/// Initial entry point for accepting a new connection; Potentially allocates new handshake state machine.
/// - parameter inboundChannel: the inbound connection channel that the other node has opened and is offering its handshake on,
/// (as opposed to the channel which we may have opened when we first extended a handshake to that node which would be stored in `state`)
/// (as opposed to the channel which we may have opened when w e first extended a handshake to that node which would be stored in `state`)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// (as opposed to the channel which we may have opened when w e first extended a handshake to that node which would be stored in `state`)
/// (as opposed to the channel which we may have opened when we first extended a handshake to that node which would be stored in `state`)

Copy link
Member Author

Choose a reason for hiding this comment

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

thx!

// TODO: if receiveTerminated ot receiveSignal etc, FORCE them to be marked @actor

if self.settings.verbose {
print(" Skip [func \(name)]... (not marked as `@actor func`)")
Copy link
Member

Choose a reason for hiding this comment

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

not marked as @actor func

Users need to mark with // @actor, not just @actor, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right i guess I’ll rephrase to “comment must include @Actor”?

Copy link
Member

Choose a reason for hiding this comment

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

It would be better to include the exact syntax I think

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, will do :) We can print a copy-pasteable signature actually perhaps 🤔

@ktoso
Copy link
Member Author

ktoso commented Jun 13, 2020

Now also with enforcing that lifecycle functions MUST be @Actor since it kind of makes sense.

Fatal error: Detected built-in [receiveSignal] actorable function but it is not marked // @actor
Mark the function as follows:

    // @actor
    func receiveSignal(context: Myself.Context, signal: Signal) {

@ktoso
Copy link
Member Author

ktoso commented Jun 13, 2020

Updated docs to talk about this as well as some codegen better "assistance" when you get it wrong.

@ktoso
Copy link
Member Author

ktoso commented Jun 13, 2020

Failure is the flakyness that we're addressing in #654

@ktoso
Copy link
Member Author

ktoso commented Jun 13, 2020

(merging to continue work on top of this towards generics -- please post merge review if you're interested / have opinions about this)

@ktoso ktoso merged commit ffc4556 into apple:master Jun 13, 2020
@ktoso ktoso deleted the wip-actorable-markers-escaping-vals branch June 13, 2020 06:48
@ktoso ktoso added this to the 0.5.0 - Cluster Hardening milestone Jun 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants