Skip to content

Add unsafeLifetime APIs and fix RawSpan initializer lifetime dependencies #77912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 12, 2024

Conversation

atrick
Copy link
Contributor

@atrick atrick commented Dec 3, 2024

Commit #1: Add unsafeLifetime APIs

Unsafely discard any lifetime dependence on the dependent argument. Return
a value identical to dependent with a new lifetime dependence on the
borrows argument.

This is required to enable lifetime enforcement in the standard library build.

Commit #2: Fix RawSpan initializer lifetime dependencies.

Two fixes are needed in most of the RawSpan and Span initializers. For example:

    let baseAddress = buffer.baseAddress
    let span = RawSpan(_unchecked: baseAddress, byteCount: buffer.count)
    // As a trivial value, 'baseAddress' does not formally depend on the
    // lifetime of 'buffer'. Make the dependence explicit.
    self = _overrideLifetime(span, borrowing: buffer)

Fix #1. baseAddress needs to be a variable

span has a lifetime dependence on baseAddress via its initializer. Therefore, the lifetime of baseAddress needs to include the call to _overrideLifetime. The override sets the lifetime dependency of its result, not its argument. It's argument still needs to be non-escaping when it is passed in.

Alternatives:

  • Make the RawSpan initializer @_unsafeNonescapableResult.

    Any occurrence of @_unsafeNonescapableResult actually signals a bug. We never want to expose this annotation.

    In addition to being gross, it would totally disable enforcement of the initialized span. But we really don't want to side-step _overrideLifetime where it makes sense. We want the library author to explicitly indicate that they understand exactly which dependence is unsafe. And we do want to eventually expose the _overrideLifetime API, which needs to be well understood, supported, and tested.

  • Add lifetime annotations to a bunch of UnsafePointer-family APIs so the compiler can see that the resulting pointer
    is derived from self, where self is an incoming Unsafe[Buffer]Pointer. This would create a massive lifetime
    annotation burden on the UnsafePointer-family APIs, which don't really have anything to do with lifetime
    dependence. It makes more sense for the author of Span-like APIs to reason about pointer lifetimes.

Fix #2: _overrideLifetime changes the lifetime dependency of span to be on an incoming argument rather than a local variable. This makes it legal to escape the function (by assigning it to self). Remember that self is implicitly returned, so the @lifetime(borrow buffer) tells the compiler that self is valid within buffer's borrow scope.

@atrick atrick requested a review from a team as a code owner December 3, 2024 07:32
@atrick atrick requested review from glessard and lorentey December 3, 2024 07:32
@atrick
Copy link
Contributor Author

atrick commented Dec 3, 2024

@swift-ci test

@atrick
Copy link
Contributor Author

atrick commented Dec 3, 2024

@swift-ci test

@milseman
Copy link
Member

milseman commented Dec 3, 2024

Be sure to underscore internal interface names.

Copy link
Contributor

@glessard glessard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsafeLifetime should be _unsafeLifetime, but otherwise this is good. Thanks.

@atrick atrick force-pushed the fix-span-deps branch 2 times, most recently from c963782 to 28fab3e Compare December 4, 2024 00:30
@atrick
Copy link
Contributor Author

atrick commented Dec 4, 2024

@swift-ci test

@atrick
Copy link
Contributor Author

atrick commented Dec 4, 2024

@swift-ci test

Unsafely discard any lifetime dependence on the `dependent` argument. Return
a value identical to `dependent` with a new lifetime dependence on the
`borrows` argument.

This is required to enable lifetime enforcement in the standard library
build.
Two are fixes needed in most of the `RawSpan` and `Span` initializers. For example:

```
    let baseAddress = buffer.baseAddress
    let span = RawSpan(_unchecked: baseAddress, byteCount: buffer.count)
    // As a trivial value, 'baseAddress' does not formally depend on the
    // lifetime of 'buffer'. Make the dependence explicit.
    self = _overrideLifetime(span, borrowing: buffer)
```

Fix #1. baseAddress needs to be a variable

`span` has a lifetime dependence on `baseAddress` via its
initializer. Therefore, the lifetime of `baseAddress` needs to include the call
to `_overrideLifetime`. The override sets the lifetime dependency of its result,
not its argument. It's argument still needs to be non-escaping when it is passed
in.

Alternatives:

- Make the RawSpan initializer `@_unsafeNonescapableResult`.

  Any occurrence of `@_unsafeNonescapableResult` actually signals a bug. We never
  want to expose this annotation.

  In addition to being gross, it would totally disable enforcement of the
  initialized span. But we really don't want to side-step `_overrideLifetime`
  where it makes sense. We want the library author to explicitly indicate that
  they understand exactly which dependence is unsafe. And we do want to
  eventually expose the `_overrideLifetime` API, which needs to be well
  understood, supported, and tested.

- Add lifetime annotations to a bunch of `UnsafePointer`-family APIs so the
  compiler can see that the resulting pointer is derived from self, where self is
  an incoming `Unsafe[Buffer]Pointer`. This would create a massive lifetime
  annotation burden on the `UnsafePointer`-family APIs, which don't really have
  anything to do with lifetime dependence. It makes more sense for the author of
  `Span`-like APIs to reason about pointer lifetimes.

Fix #2. `_overrideLifetime` changes the lifetime dependency of span to be on an
incoming argument rather than a local variable.

This makes it legal to escape the function (by assigning it to self). Remember
that self is implicitly returned, so the `@lifetime(borrow buffer)` tells the
compiler that `self` is valid within `buffer`'s borrow scope.
Also, fix the _overrideLifetime doc comments.
@atrick
Copy link
Contributor Author

atrick commented Dec 12, 2024

@swift-ci test

@atrick atrick enabled auto-merge December 12, 2024 07:21
@atrick atrick merged commit 8553f99 into swiftlang:main Dec 12, 2024
5 checks passed
@atrick atrick deleted the fix-span-deps branch December 12, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants