Skip to content

examples: Use slice::from_ref to not loose lifetime on nested slices#513

Merged
Ralith merged 1 commit into
masterfrom
example-slice-lifetime
Dec 20, 2021
Merged

examples: Use slice::from_ref to not loose lifetime on nested slices#513
Ralith merged 1 commit into
masterfrom
example-slice-lifetime

Conversation

@MarijnS95

Copy link
Copy Markdown
Collaborator

As per the readme .build() should only be called as late as possible, and only if absolutely necessary; such cases include slices that are passed directly to functions. More precisely, such build calls and the creation of temporary slices should happen inside the same expression as the function call to be sound and completely lifetime-checked.

This pattern of &[my_builder.build()] is however not possible when constructing intermediary Vulkan objects that reference the slice. In the first place this slice goes out of scope after the expression that creates the Vulkan object, which is caught and disallowed by rustc (unless this expression itself ends in .build(), which is completely unsound as it makes rustc unable to validate this lifetime dependency).

In the second place - and as is most relevant for this patch that removes .build() calls that were not surrounded by temporary slice constructors - said expression drops the lifetime checks on anything held by my_builder which could go out of scope before the newly constructed Vulkan object is used, resulting yet again in Undefined Behaviour.

Fortunately, for slices of size 1 which are typical in Vulkan, std::slice::as_ref exists which is analogous to taking a pointer to an object and considering it an array of length 1 in C(++). This maintains the lifetime through Deref and makes rustc able to fully check all lifetimes and prevent unsound code.

Albeit improving overall consistency, the &[my_builder.build()] pattern is not substituted in aforementioned Vulkan function-call expressions as that is considered "extraneous" 1 and demonstrates the various ways to safely construct Vulkan objects for the observant reader.

As per the readme `.build()` should only be called as late as possible,
and only if absolutely necessary; such cases include slices that are
passed directly to functions.  More precisely, such build calls and the
creation of temporary slices should happen inside the same expression as
the function call to be sound and completely lifetime-checked.

This pattern of `&[my_builder.build()]` is however not possible when
constructing intermediary Vulkan objects that reference the slice.  In
the first place this slice goes out of scope after the expression that
creates the Vulkan object, which is caught and disallowed by rustc
(unless this expression itself ends in `.build()`, which is completely
 unsound as it makes rustc unable to validate this lifetime dependency).

In the second place - and as is most relevant for this patch that
removes `.build()` calls that were not surrounded by temporary slice
constructors - said expression drops the lifetime checks on anything
held by `my_builder` which _could_ go out of scope before the newly
constructed Vulkan object is used, resulting yet again in Undefined
Behaviour.

Fortunately, for slices of size 1 which are typical in Vulkan,
`std::slice::as_ref` exists which is analogous to taking a pointer to an
object and considering it an array of length 1 in C(++).  This maintains
the lifetime through `Deref` and makes rustc able to fully check all
lifetimes and prevent unsound code.

Albeit improving overall consistency, the `&[my_builder.build()]`
pattern is not substituted in aforementioned Vulkan function-call
expressions as that is considered "extraneous" [1] and demonstrates the
various ways to safely construct Vulkan objects for the observant reader.

[1]: #506 (comment)

@Ralith Ralith left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IMO the safest pattern is to only ever construct objects lexically within Vulkan calls. Clear improvement here in any case.

@Ralith Ralith merged commit f781777 into master Dec 20, 2021
@MarijnS95 MarijnS95 deleted the example-slice-lifetime branch December 20, 2021 15: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.

2 participants