-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Distributed] Ensure _remote funcs synthesized before dynamic replacement #38974
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -933,18 +933,18 @@ class IsDistributedActorRequest : | |
bool isCached() const { return true; } | ||
}; | ||
|
||
/// Determine whether the given func is distributed. | ||
class IsDistributedFuncRequest : | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This request is removed now since it is so cheap we dont need to cache it |
||
public SimpleRequest<IsDistributedFuncRequest, | ||
bool(FuncDecl *), | ||
/// Obtain the 'remote' counterpart of a 'distributed func'. | ||
class GetDistributedRemoteFuncRequest : | ||
public SimpleRequest<GetDistributedRemoteFuncRequest, | ||
AbstractFunctionDecl *(AbstractFunctionDecl *), | ||
RequestFlags::Cached> { | ||
public: | ||
using SimpleRequest::SimpleRequest; | ||
|
||
private: | ||
friend SimpleRequest; | ||
|
||
bool evaluate(Evaluator &evaluator, FuncDecl *func) const; | ||
AbstractFunctionDecl *evaluate(Evaluator &evaluator, AbstractFunctionDecl *func) const; | ||
|
||
public: | ||
// Caching | ||
|
@@ -2046,8 +2046,6 @@ enum class ImplicitMemberAction : uint8_t { | |
ResolveEncodable, | ||
ResolveDecodable, | ||
ResolveDistributedActor, | ||
ResolveDistributedActorIdentity, | ||
ResolveDistributedActorTransport, | ||
}; | ||
|
||
class ResolveImplicitMemberRequest | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2365,7 +2365,7 @@ void AttributeChecker::visitDiscardableResultAttr(DiscardableResultAttr *attr) { | |
} | ||
} | ||
|
||
/// Lookup the replaced decl in the replacments scope. | ||
/// Lookup the replaced decl in the replacements scope. | ||
static void lookupReplacedDecl(DeclNameRef replacedDeclName, | ||
const DeclAttribute *attr, | ||
const ValueDecl *replacement, | ||
|
@@ -2400,9 +2400,10 @@ static void lookupReplacedDecl(DeclNameRef replacedDeclName, | |
if (declCtxt->isInSpecializeExtensionContext()) | ||
options |= NL_IncludeUsableFromInline; | ||
|
||
if (typeCtx) | ||
if (typeCtx) { | ||
moduleScopeCtxt->lookupQualified({typeCtx}, replacedDeclName, options, | ||
results); | ||
} | ||
} | ||
|
||
/// Remove any argument labels from the interface type of the given value that | ||
|
@@ -3498,6 +3499,19 @@ DynamicallyReplacedDeclRequest::evaluate(Evaluator &evaluator, | |
if (attr->isInvalid()) | ||
return nullptr; | ||
|
||
auto *clazz = VD->getDeclContext()->getSelfClassDecl(); | ||
if (clazz && clazz->isDistributedActor()) { | ||
// Since distributed actors synthesize their `_remote` | ||
// counterparts to any `distributed func` declared on them, and such remote | ||
// function is a popular target for dynamic function replacement - we must | ||
// force the synthesis has happened here already, such that we can lookup | ||
// the func for the replacement we're handling right now. | ||
// | ||
// This request is cached, so it won't cause wasted/duplicate work. | ||
TypeChecker::addImplicitDistributedActorRemoteFunctions(clazz); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the important bit (well, one of them); if we first visit an extension, and didn't yet synthesize remote functions, but the extension has an function dynamic member replacement, the function it wants to replace does not exist yet so we must trigger synthesis. We can be smarter about the synthesis later, by requesting the specific func, but realistically we need all of them anyway and this unblocks us for now. |
||
|
||
|
||
// If we can lazily resolve the function, do so now. | ||
if (auto *LazyResolver = attr->Resolver) { | ||
auto decl = LazyResolver->loadDynamicallyReplacedFunctionDecl( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really happy with this approach -- it allows us to just get the remote func whenever we want, and if it wasn't synthesized yet, it becomes synthesized.
In some situations we may need for force synthesizing all _remote funcs, e.g. when the dynamic member replacement is triggered, since we don't know yet which exact function we'll be looking up -- I believe we can improve this in the future when we do some @remoteFuncReplacement since then we know exactly which one to "get" for, this way everything will be even more lazy.