[Distributed] Undo new record and mangling scheme for dist.p.witnesses #71801
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ktoso
force-pushed
the
wip-remove-accessor-protocol-sections
branch
from
fe4eb76 to
0d80c8b
Compare
ktoso
requested review from
a team,
mikeash,
al45tair,
hborla,
slavapestov and
xedin
as code owners
ktoso
requested review from
bnbarham
and removed request for
a team,
xedin,
slavapestov,
mikeash,
al45tair and
hborla
ktoso
changed the title
|
@swift-ci please test |
ktoso
commented
Feb 22, 2024
| @@ -390,6 +390,7 @@ bool swift::checkDistributedSerializationRequirementIsExactlyCodable( | |||
| std::count(protocols.begin(), protocols.end(), decodable) == 1; | |||
| } | |||
|
|
|||
| // TODO(distributed): probably can be removed? | |||
| llvm::ArrayRef<ValueDecl *> | |||
| AbstractFunctionDecl::getDistributedMethodWitnessedProtocolRequirements() const { | |||
There was a problem hiding this comment.
not 100% sure yet so I kept it around for now
ktoso
commented
Feb 22, 2024
| @@ -359,7 +355,6 @@ extern uintptr_t __COMPATIBILITY_LIBRARIES_CANNOT_CHECK_THE_IS_SWIFT_BIT_DIRECTL | |||
| #define __ptrauth_swift_escalation_notification_function | |||
| #define __ptrauth_swift_dispatch_invoke_function | |||
| #define __ptrauth_swift_accessible_function_record | |||
| #define __ptrauth_swift_accessible_protocol_requirement_function_record | |||
There was a problem hiding this comment.
we're removing the entire new section kind; this is safe since no release is using this yet.
This change undoes an approach we thought viable to encode distributed actor remote targets, as encoding their protocol method names. This also needed extra metadata sections and tricky "find if this is a witness or not" methods; Instead, we will be mangling the concrete names; and special handle proxy types that are prefixed with `$`. This way we will not need extra metadata records, and can handle generic calls more properly. This XFAILs one test which was testing new functionality, that is pending now until we implement the new mangling scheme.
ktoso
force-pushed
the
wip-remove-accessor-protocol-sections
branch
from
5383b7e to
44b1227
Compare
|
@swift-ci please test |
mikeash
approved these changes
Feb 22, 2024
|
Merging as we'll build the new mangling scheme anew from clean slate. |
|
Thanks for review Mike! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change undoes an approach we thought viable to encode distributed
actor remote targets, as encoding their protocol method names. This also
needed extra metadata sections and tricky "find if this is a witness or
not" methods;
Instead, we will be mangling the concrete names; and special handle
proxy types that are prefixed with
$. This way we will not need extrametadata records, and can handle generic calls more properly.
This XFAILs one test which was testing new functionality, that is
pending now until we implement the new mangling scheme.