Skip to content

Add support for distributed functions in extensions of distributed actors #39785

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

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented Oct 16, 2021

Fix a few minor issues in the type checker and SILGen to properly cope with
distributed functions defined within extensions of distributed actors.
While here, centralize the logic that adds the "remote" function.

... and since this uncovered a problem with the mangling for distributed
functions, fix that, too.

Fixes rdar://84325525.

…tors.

Fix a few minor issues in the type checker and SILGen to properly cope with
distributed functions defined within extensions of distributed actors.
While here, centralize the logic that adds the "_remote_" function.

Fixes rdar://84325525.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Looks good, thanks a lot!

@ktoso ktoso added the distributed Feature → concurrency: distributed actor label Oct 16, 2021
Distributed thunks were using the same mangling as direct method
reference thunks (i.e., for "super" calls). Although not technically
conflicting so long as actors never gain inheritance, it's confusing
and could cause problems in the future. So, introduce a distinct
mangling for distributed thunks and plumb them through the demangling
and remangler.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

@@ -217,6 +217,7 @@ types where the metadata itself has unknown layout.)
global ::= global 'To' // swift-as-ObjC thunk
global ::= global 'TD' // dynamic dispatch thunk
global ::= global 'Td' // direct method reference thunk
global ::= global 'TE' // distributed actor thunk
Copy link
Contributor

Choose a reason for hiding this comment

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

okey E then! Thanks for spotting this

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm we also used the Td in:


  case Kind::DistributedThunkAsyncFunctionPointer: {
    std::string Result = getSILDeclRef().mangle();
    Result.append("Td");
    Result.append("Tu");
    return Result;
  }

which this doesnt fix yet -- i think there we also need to use TE right? I'll follow up with this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, and that's what failed the tests -- I fixed it :)

@ktoso
Copy link
Contributor

ktoso commented Oct 16, 2021

@swift-ci please build toolchain macOS

@ktoso
Copy link
Contributor

ktoso commented Oct 16, 2021

Actual failure, I think I know how to solve -- on it.

@ktoso
Copy link
Contributor

ktoso commented Oct 16, 2021

Replaced by #39787 where the only additional fix is e1ae0be

@ktoso ktoso closed this Oct 16, 2021
@DougGregor DougGregor deleted the distributed-func-in-actor-extension branch October 17, 2021 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distributed Feature → concurrency: distributed actor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants