[5.9] More batched changes #66196
Merged
[5.9] More batched changes #66196
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 |
gottesmm
force-pushed
the
release-5.9-more-stuff-2
branch
from
84454da to
b2fe8dc
Compare
…e to see the value witness function, call it via the value witness function. Some notes: 1. I put in both a swiftpm like test case and a library evolution test case. I also updated the moveonly_deinit serialization swift test to show that we actually serialize the deinit. 2. I changed when we emit the deinit table to only be when we have a type with an actual value type destructor. Notably this doesn't include classes today so as a side-effect, we no longer attempt to devirtualize moveonly class deinits. This doesn't affect anything we are trying to actually do since we do not support noncopyable classes today. With that in mind, I changed one test that was showing that deinit devirtualization worked to use a struct with deinit instead of a class. rdar://109679168 (cherry picked from commit 8579c19)
(cherry picked from commit a52302d)
… that it cannot be captured by an escaping closure. rdar://109742587 (cherry picked from commit 577e76b)
…se values. I also added a bunch of tests that showed the behavior of subscripts/other accessors with the following combinations of semantics: 1. get only. 2. get/set. 3. get/modify. 4. read/set. 5. read/modify. rdar://109746476 (cherry picked from commit 508bf8a)
…ttach a mark_must_check [consumable_and_assignable]. Most of the time SILGen already emits these correctly without having extra copies, but in certain situations SILGen will emit copies that we need the move checker to eliminate (e.x.: when we generate a yield). An additional benefit is that this also will catch places where the frontend makes a mistake. This also removes a bunch of "copy of noncopyable" types error that showed up in the implicit compiler generated modify. (cherry picked from commit 5d9ab63)
…rrows correctly. Before the previous commit, we didn't see load [take] very often since it occurs mostly on temporaries where we treat the copy_addr as the relevant take. Now that we are checking temporaries though, we need to support this behavior. (cherry picked from commit c9edaee)
…_read accessor. We want the result of getters to still be separate values. (cherry picked from commit 7b66c70)
…hen the type also has a modify. The form of the AST changes slightly when a type has a read and a modify. Specifically, we now have a load on the subscript and an inout_expr on the base. I dealt with this by making the inout_expr something that when we look for storage we look through and by tweaking the load lookthrough code. (cherry picked from commit 2b785e4)
…none so we can make sure it works even without opts. (cherry picked from commit c0ce58c)
The member just clears the values. After it adopted BitfieldRef, the call to invalidate on the liveness instance was already superfluous. (cherry picked from commit 3c4275a)
gottesmm
force-pushed
the
release-5.9-more-stuff-2
branch
from
b2fe8dc to
ffa9119
Compare
|
@swift-ci test |
jckarter
reviewed
Jun 2, 2023
|
|
||
| // If we do not have a deinit table, call the value witness instead. | ||
| if (!deinitTable) { | ||
| irgen::emitDestroyCall(IGF, T, indirect()); |
There was a problem hiding this comment.
You need to call indirectCleanup after the call to indirect.
There was a problem hiding this comment.
I will prepare a small patch on main that will fix this. Why don't you give the +1 and then I can get TimK to +1 this and I can take care of this tonight/over the weekend.
jckarter
approved these changes
Jun 2, 2023
tbkka
approved these changes
Jun 3, 2023
|
@swift-ci test |
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 PR contains a set of batched commits for 5.9. A few of the commits were
test tweaks so I did not include CCC for them.
Commit 6a5ef8a
• Description: [move-only] Make sure that we serialize deinits and if we are not
able to see the value witness function, call it via the value witness
function.
• Risk: Low. This only affects noncopyable types.
• Original PR: #66185
• Reviewed By: @jckarter
• Testing: I put in both a swiftpm like test case and a library evolution test
case. I also updated the moveonly_deinit serialization swift test to show that
we actually serialize the deinit.
• Resolves: rdar://109679168
Commit 5530aa9
• Description: [move-only] Teach deinit devirtualization how to handle
devirtualizing a deinit of a resilient type.
• Risk: Low. This only affects noncopyable types.
• Original PR: #66185
• Reviewed By: @jckarter
• Testing: I added tests that exercised this behavior.
• Resolves: rdar://109904633
Commit 45a592c
• Description: [move-only] Change closure capture diagnostic for let patterns to
say that it cannot be captured by an escaping closure.
• Risk: Low. This only affects noncopyable types.
• Original PR: #66197
• Reviewed By: @jckarter
• Testing: Updated tests to show we emit the correct diagnostic.
• Resolves: rdar://109742587
Commit 20bf5d5
• Description: [move-only] Teach SILGenApply how to emit subscripts with borrowed base values.
• Risk: Low. This only affects noncopyable types.
• Original PR: #66094
• Reviewed By: @jckarter
• Testing: Added exhaustive tests in this area to test out the behavior
• Resolves: rdar://109746476
Commit c009c32
• Description: [move-only] When we emit a noncopyable temporary allocation,
always attach a mark_must_check [consumable_and_assignable]. This removes most
of the copy of noncopyable type errors seen in the tests from
20bf5d5
• Risk: Low. This only affects noncopyable types.
• Original PR: #66094
• Reviewed By: @jckarter
• Testing: Added exhaustive tests in this area to test out the behavior
• Resolves: rdar://109746476
Commit 61745ee
• Description: [move-only] Make sure that we handle load [take] that are
actually borrows correctly.
• Risk: Low. This only affects noncopyable types.
• Original PR: #66094
• Reviewed By: @jckarter
• Testing: Added SIL tests that show that we have the correct behavior here
• Resolves: rdar://109746476
Commit 23deccf
• Description: Restricts c009c32 so we only
perform the transformation on reads instead of also gets.
• Risk: Low. This only affects noncopyable types.
• Original PR: #66094
• Reviewed By: @jckarter
• Testing: Updated silgen tests that show this behavior. Updated the subscript
tests so that they validate that we do not have copy of noncopyable errors in
the places that this change fixes.
• Resolves: rdar://109746476
Commit 1ae66b2
• Description: [move-only] Teach SILGen how to recognize a borrowed read
subscript when the type also has a modify.
• Risk: Low. This only affects noncopyable types.
• Original PR: #66094
• Reviewed By: @jckarter
• Testing: Updated silgen tests that show this behavior. Updated the subscript
tests so that they validate that we do not have copy of noncopyable errors in
the places that this change fixes.
• Resolves: rdar://109746476