Add preliminary support for borrow accessors #84180
Conversation
meg-gupta
force-pushed
the
borrowandmutatepr
branch
from
afadd64 to
0d83579
Compare
meg-gupta
force-pushed
the
borrowandmutatepr
branch
from
0ef0f42 to
0614ae0
Compare
And handle them in various covered switches
… within SubscriptDecl
…edAddress ResultConvention::Guaranteed will be used by borrow accessors when the storage type can be returned by value. ResultConvention::GuaranteedAddress will be used by mutate accessors and borrow accessors when the storage type cannot be returned by value.
…edAddress in SwiftCompilerSources
…y ~Copyable types
meg-gupta
force-pushed
the
borrowandmutatepr
branch
from
0614ae0 to
4efc210
Compare
meg-gupta
requested review from
bnbarham,
hamishknight,
rintaro,
hborla and
slavapestov
as code owners
meg-gupta
requested review from
eeckstein,
jckarter,
CodaFi,
DougGregor,
xymus,
AnthonyLatsis,
kavon,
adrian-prantl,
rjmccall and
tshortli
as code owners
|
@swift-ci test |
|
Please test with following pull request: @swift-ci Please test |
1 similar comment
|
Please test with following pull request: @swift-ci Please test |
|
Please test with following pull request: @swift-ci Please test macOS platform |
meg-gupta
force-pushed
the
borrowandmutatepr
branch
from
40925ec to
345d1b8
Compare
|
Please test with following pull request: @swift-ci Please test |
|
For the record, I suggested that we make a new rule that 'self' on a borrow accessor cannot have a lexical lifetime. Note that 'self' already needs to be borrowed over the duration of the accessor and the dependency between the borrowed value and 'self' needs to be represented in SIL. There would never be a need for a lexical lifetime. That's good, because it would be impossible to represent a returns borrow value from a begin_borrow scope. |
There was a problem hiding this comment.
This all looks great to me. I glossed over most of the SILGen implementation because I'm not much use there.
We need verification to ensure that, whenever a borrow accessor returns an address, it's implementation only refers to addressable storage recursively up the the addressable 'self' parameter. I know your diagnostics mostly cover this. I just think we need SIL verification on top of that which can run after any pass.
We should test IRGen for large loadable values.
It's to be determined whether the SIL should model the borrows result with mark_dependence . If the apply can always be viewed as an ownership forwarding operation on the 'self' operand, and the we guarantee that the accessor implementation only allows forwarding (or escaping) operations. Then we don't structly need another SIL dependency.
| @@ -767,6 +767,12 @@ Scope scopeForArgument(Scope nonlexicalScope, SILValue callArg, unsigned index, | |||
| // the argument. Just do an ownership conversion if needed. | |||
| return nonlexicalScope; | |||
| } | |||
|
|
|||
| // Use non-lexical scope for functions returning @guaranteed results. | |||
| if (callee->getConventions().hasGuaranteedResult()) { | |||
There was a problem hiding this comment.
In Ownership.md, Variable Lifetimes, we can add a note that lexical lifetimes are not support on self for borrow accessors.
Add a read effect on the self parameter. Without this, the self parameter can get dead code eliminated by the GenericSpecializer.
…asGuaranteedResults -> hasGuaranteedResult
…sult() with ApplyInst::hasGuaranteedAddressResult()
…essors It uses a check on conformance to ForwardInstruction for walking down guaranteed forwarding uses. Since apply of borrow accessors cannot be represented as ForwardingInstruction, handle them separately. Representing apply of borrow accessors for consistent handling in the optimizer is TBD.
meg-gupta
force-pushed
the
borrowandmutatepr
branch
from
345d1b8 to
9792ef8
Compare
|
Please test with following pull request: @swift-ci Please test |
Introduces a new experimental flag, AST, SILGen and limited SIL support.
Adds support for writing borrow accessors that return projections of
selfrepresentable by a newResultConvention::GuaranteedorResultConvention::Guaranteed.There are still several unimplemented parts. The main ones are:
InOutTypeis banned)