[move-only] Teach SILGenApply how to emit subscripts with borrowed base values. #66094
Conversation
|
@swift-ci smoke test |
…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
…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.
…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.
…_read accessor. We want the result of getters to still be separate values.
gottesmm
force-pushed
the
pr-9cc91f1b6b3ff60686adb825c4a444221c3d798a
branch
from
1dd7983 to
7b66c70
Compare
|
@swift-ci smoke test |
| // CHECK: } // end sil function '$s8moveonly047testSubscriptGetOnly_BaseLoadable_ResultAddressE4_LetyyF' | ||
| public func testSubscriptGetOnly_BaseLoadable_ResultAddressOnly_Let() { | ||
| let m = LoadableSubscriptGetOnlyTester() | ||
| m[0].nonMutatingFunc() |
There was a problem hiding this comment.
It looks like in this case the nonMutatingFunc() call still occurs inside of the borrow of m.
There was a problem hiding this comment.
This is not a result of this change since it is using a getter. I am pretty sure this is a result of us borrowing bases of accessors in an aggressive way which was part of the original implementation of accessors. IIRC @atrick ran into this when exploring non-escaping types.
| // CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]] | ||
| // CHECK: [[MARK:%.*]] = mark_must_check [no_consume_or_assign] [[ACCESS]] | ||
| // CHECK: [[LOAD:%.*]] = load_borrow [[MARK]] | ||
| // Eventually, we need this end_apply to be around the nonMutatingFunc. |
There was a problem hiding this comment.
Since AddressOnlyProtocol is move-only, we can't copy it out of the end_apply/end_borrow/end_access, and the call to nonMutatingMethod does have to happen during the borrow. So "eventually" doesn't seem to cut it. Does the move only checker fix this up?
There was a problem hiding this comment.
It was an issue where SILGen was failing to pattern match. I did an update that fixes this.
…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.
|
@swift-ci smoke test |
|
@jckarter I want to do some work this weekend that builds on top of this. I am happy to change anything else that you need changed post-commit. |
I also added a bunch of tests that showed the behavior of subscripts/other accessors with the following combinations of semantics:
rdar://109746476