🍒[5.10][Distributed] Generic reqs must be forwarded to accessor from enclosing actor #68861
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
|
@swift-ci please test |
|
Friendly ping - if possible we'd like to be able to tests nightly toolchains on Linux too :-) |
…ng actor If we don't do this, the generic parameter has no requirements at all, which a) is incorrect to begin with, it should have the exact same signature as the method it is the accessor for, and b) it would trip up verification which is enabled on linux in snapshot builds -- causing crashes. Resolves rdar://115497090
ktoso
force-pushed
the
pick-generic-reqs-from-actor-da-for-accessor
branch
from
8c12111 to
20206d5
Compare
|
@swift-ci please test |
slavapestov
requested changes
Jan 9, 2024
| // and also forward all requirements this generic parameter might have. | ||
| for (auto req : actor->getGenericRequirements()) { | ||
| if (req.getFirstType()->isEqual(genericParam)) { | ||
| genericRequirements.push_back(req); |
There was a problem hiding this comment.
Please cherry pick the corrected version from main that uses buildGenericSignature() and preserves all generic requirements, not just those whose subject type is exactly equal to an innermost generic parameter.
There was a problem hiding this comment.
Thanks Slava, slight de-javu actually indeed... we worked on this in #69502 which already made it to 5.10.
I'll re-evaluate what we're missing and if we even needed this type change still.
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.
Issue: Without this change, in debug builds the requirement machine validation infra would crash to compile. This prevents advanced Distributed Actors users with lots of generics from using NIGHTLY builds. Stable builds have this assertion turned off.
Description: If we don't do this, the generic parameter has no requirements at all, which a) is incorrect to begin with, it should have the exact same signature as the method it is the accessor for, and b) it would trip up verification which is enabled on linux in snapshot builds -- causing crashes.
Affected versions: All the way since initial introduction, but ONLY in debug builds -- i.e. nightly snapshots on linux. Released stable Swift versions don't have this assertion and would "happen to work".
Risk: Low, this only adds missing requirements to distributed function accessors which is the correct thing to do. Missing them "worked by accident".
Review by: @slavapestov @xedin
Testing: CI testing; verified manually on reproducer project provided in issue
Radar: rdar://115497090
Original PR:
#68859#70785Resolves #68517