[move-only] A group of batched changes #65353
Merged
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 test |
|
In a forthcoming commit, the 2nd commit will come into play, I am just peeling the onion |
|
@swift-ci smoke test |
kavon
reviewed
Apr 21, 2023
gottesmm
force-pushed
the
pr-7fddccd1df47d4c57c27121727562d691b9f2397
branch
from
eb19ee7 to
0ac6fac
Compare
|
@swift-ci test |
|
@swift-ci test source compatibility |
gottesmm
force-pushed
the
pr-7fddccd1df47d4c57c27121727562d691b9f2397
branch
from
0ac6fac to
1f1f250
Compare
|
@swift-ci test |
|
@swift-ci test source compatibility |
atrick
approved these changes
Apr 24, 2023
There was a problem hiding this comment.
I only reviewed the commit "[move-only] Instead of using AccessUseVisitor to visit addresses"
Looks good. Just a few comments inline.
Ideally the starting address producers would be in sync with AccessUseDefChainVisitor<Impl, Result>::visit. We're just missing a few of the unusual cases.
kavon
reviewed
Apr 24, 2023
…lization that makes it easy to look for errors around specialization.
gottesmm
force-pushed
the
pr-7fddccd1df47d4c57c27121727562d691b9f2397
branch
from
1f1f250 to
02c0ebd
Compare
…TransitiveAddressVisitor. This makes it so that the move address checker is not dependent on starting the traversal at a base object. I also included verifier checks that the API can visit all address uses for: 1. project_box. 2. alloc_stack. 3. ref_element_addr. 4. ref_tail_addr. 5. global_addr_inst. this is because this visitor is now apart of the SIL API definition as being able to enumerate /all/ addresses derived from a specific chosen address value. This is a refactoring NFCI change. rdar://108510644
…d move only types were removed.
…py] instead of a load_borrow. The reason why I am doing this is that otherwise if one has a function that takes both a guaranteed and an owned parameter, we will break OSSA invariants since the load [take] will invalidate the load_borrow. So instead, we put in a load_borrow knowing that the move checker will convert it to a load_borrow assuming that the two pass exclusivity checking. NOTE: Because of some missing functionality in subsequent tests, I had to disable one test (moveonly_escaping_definite_initialization.swift) and also add some checks for copy of noncopyable object errors. They will go away in the next 2 commits. rdar://108510987
…opyable checking in the closure. The reason why I am doing this is that currently SILGen knows when emitting said closure that we are going to emit an error (that it does not have enough information to emit itself since it doesn't know the caller), so doesn't emit move checking markers. The result is that the move checker will not eliminate any copies in the closure and thus will emit a "copy of noncopyable type found" error and tell the user to file a bug. This just suppresses that. rdar://108511866
…ble type, always emit mark_must_check. The reason to do this is that: 1. Otherwise, we do not emit markers when someone attempts to consume the let. We need the no_consume_or_assign to be there. 2. We need to insert assign_but_not_consuming so that DI can properly check lets that are conditionally initialized and convert them to initable_but_not_consuming. I included a full definite_init SIL test that validates that we get the correct codegen after DI in this case and emit the appropriate error as well. rdar://108511534
…scaping closure behavior. NOTE: We consider consuming closures that are marked without escaping as non-escaping. We shouldn't do this, but for now I just included them so that their behavior is in some test and we at least don't crash.
I left in the __owned tests of course. rdar://108511703
…, do not suggest to the user to make a local copy. It doesn't make sense to give this note since one can't make a copy of a noncopyable type. rdar://108511627
These are the same semantically, just the mangling is slightly different. The benefit of doing this is that we are actually testing what we expect our users to do. rdar://108511703
That PR was for rdar://108385761
gottesmm
force-pushed
the
pr-7fddccd1df47d4c57c27121727562d691b9f2397
branch
from
02c0ebd to
9c8d224
Compare
|
@swift-ci test |
|
@swift-ci test source compatibility |
kavon
approved these changes
Apr 25, 2023
|
I discovered a small issue where I introduced an optnone by mistake. I am going to remove it in a follow on smoke test change. |
|
Actually I was wrong about the optnone. It was in my local branch that I am working with on top of this. |
gottesmm
changed the title
|
I took a look at the compatibility failure. One was a UPASS that showed up on the actual bot. |
|
The other failure is distributed actors which is also failing on other PRs due to misconfiguration. I talked with Konrad and he agreed I can ignore it. |
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.
may cause the user to think there is a bug in the compiler.