[ownership] Add a section to SIL.rst that describes the semantics of safe interior pointers in Ownership SIL. #34618
Conversation
gottesmm
force-pushed
the
pr-1639094f806f1130063c975fee6b1f74f2babb6a
branch
2 times, most recently
from
b31336a to
3ba5a5d
Compare
|
Thanks for writing this! LGTM. |
docs/SIL.rst
Outdated
|
|
||
| // %2 is an interior pointer into %1. Since %2 is an address, it's uses are | ||
| // not treated as uses of the underlying borrowed object %1 in the ownership | ||
| // system. This is because at the ownership level objects with none |
There was a problem hiding this comment.
I am going to None_ so it jumps back, but good catch.
There was a problem hiding this comment.
Ah. Actually I didn't see this is in line SIL so I will do None. We shouldn't use .none or it sounds like we are talking about optionals.
There was a problem hiding this comment.
OK, sounds good. There's another place that needs to be changed too, then.
docs/SIL.rst
Outdated
| // %2 is an interior pointer into the Klass k. Since %2 is an address and | ||
| // address have .none ownership, it's uses are not treated as uses of the | ||
| // underlying object %1. | ||
| %2 = ref_element_addr %1 : $Klass, #Klass.k // %1 is a $*KlassWrapper |
There was a problem hiding this comment.
Do you mean:
// %2 is a $*KlassWrapper
Maybe it would help to put class Klass { ... } somewhere in SIL.rst.
docs/SIL.rst
Outdated
| existential container. Must be the type for which the box was initially | ||
| allocated for and not for an “opened” archetype. | ||
|
|
||
| (*) We still need to finish adding support for project_box, but all other ones |
There was a problem hiding this comment.
Nit: "all others are" or "all other instructions are".
gottesmm
force-pushed
the
pr-1639094f806f1130063c975fee6b1f74f2babb6a
branch
from
3ba5a5d to
291841f
Compare
There was a problem hiding this comment.
I think it could be more clear that the transitive search for address uses stops at any instruction that isn't in your approved list of interior pointer operations. Typically, those uses do not propagate the pointer value, so they are safe. Other uses escape the address value such as pointer_to_address. Those uses are unsafe--the user is reponsible for managing unsafe pointer lifetimes and the compiler must not extend those pointer lifetimes.
|
|
||
| * `project_box`_ - projects a pointer out of a reference counted box. (*) | ||
| * `ref_element_addr`_ - projects a field out of a reference counted class. | ||
| * `ref_tail_addr`_ - projects out a pointer to a class’s tail allocated array |
There was a problem hiding this comment.
tail_addr is another instruction that uses the ref_tail_addr and produces a new address
There was a problem hiding this comment.
It does, but it isn't an interior pointer instruction since it doesn't convert an object to an address. It just translates addresses to addresses. IOW one can never use a tail_addr to be be a mixed projection like operation.
There was a problem hiding this comment.
That's correct. tail_addr is just an address projection, in fact all the other address projections are also relevant to the discussion. So I think what we're saying here is that an "interior pointer operation" needs its own borrow scope, and all transitive uses after propagating through address projections need to be within that borrow scope. It still may be worth pointing out that address_to_pointer is not considered an address projection, so it effectively escapes the interior address from the borrow scope... it's up to the the programmers and optimizer to manage that pointer lifetime without any help from the ownership verifier.
There was a problem hiding this comment.
@atrick Just pushed in a bit more prose. Tell me what you think.
gottesmm
force-pushed
the
pr-1639094f806f1130063c975fee6b1f74f2babb6a
branch
from
291841f to
ea22692
Compare
…safe interior pointers in Ownership SIL. This is just part of my ongoing effort to document more explicitly Ownership SSA in SIL.rst. NOTE: I realized while writing this, we might be able toalso do this form of verification with owned values. I don't know how easy/hard it would be though. Its nice to have that owned values do not need to look for these when optimizing. So that would need to be balanced against expanding this.
gottesmm
force-pushed
the
pr-1639094f806f1130063c975fee6b1f74f2babb6a
branch
from
ea22692 to
9712e45
Compare
|
@swift-ci smoke test |
This is just part of my ongoing effort to document more explicitly Ownership SSA
in SIL.rst.
NOTE: I realized while writing this, we can also do this form of verification
with owned values. I don't know how easy/hard it would be though. Its nice to
have that owned values do not need to look for these. But that is something that
we may want to do.