You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/11/23 21:32:03 UTC
[GitHub] [arrow] carols10cents opened a new pull request #8752: ARROW-10705: [Rust] Loosen restrictions on some lifetime annotations
carols10cents opened a new pull request #8752:
URL: https://github.com/apache/arrow/pull/8752
There are a number of lifetime annotations in the IPC writer that are inconsistent and too strict; which prevents code reuse.
In the first commit in this PR, I attempt to call the `schema_to_fb_offset` function in the IPC FileWriter `finish` method -- there's a comment in that spot that says:
```
// TODO: this is duplicated as we otherwise mutably borrow twice
```
And indeed the code is mostly the same as in `schema_to_fb_offset`, except for [the fix I made to write dictionary fields]() ;) So I think it's high time to remove the duplication!
However, as the comment alludes to, if I try to compile with just the first commit in this PR, I get this error:
```
error[E0499]: cannot borrow `fbb` as mutable more than once at a time
--> arrow/src/ipc/writer.rs:181:62
|
178 | let schema = ipc::convert::schema_to_fb_offset(&mut fbb, &self.schema);
| -------- first mutable borrow occurs here
...
181 | let mut footer_builder = ipc::FooterBuilder::new(&mut fbb);
| ^^^^^^^^ second mutable borrow occurs here
182 | footer_builder.add_version(self.write_options.metadata_version);
183 | footer_builder.add_schema(schema);
| ------ first borrow later used here
```
This code *should* be fine because the mutable borrow to `fbb` that `ipc::convert::schema_to_fb_offset` uses should also end on that same line.
The compiler thinks that mutable borrow is still alive because of the signature of `ipc::convert::schema_to_fb_offset`:
```
pub fn schema_to_fb_offset<'a: 'b, 'b>(
fbb: &'a mut FlatBufferBuilder,
schema: &Schema,
) -> WIPOffset<ipc::Schema<'b>> {
```
Trying to put this into words, this is telling Rust that for a mutable borrow to a `FlatBufferBuilder` with some lifetime `'a`, the returned value will have a different lifetime `'b` where lifetime `'a` is longer than the lifetime of `'b`.
Putting this restriction into the callsite, because `let schema` (which holds the return type) lives until the end of the `finish` function and it has lifetime `'b`, then for this function to be valid, the mutable borrow with lifetime `'a` has to live to the end of the function (and thus live longer than `'b`), which causes the error message.
However, this isn't correct. The constraints we actually want to tell Rust about are:
```
pub fn schema_to_fb_offset<'a>(
fbb: &mut FlatBufferBuilder<'a>,
schema: &Schema,
) -> WIPOffset<ipc::Schema<'a>> {
```
which says: For a mutable reference to a `FlatBufferBuilder` that itself contains references that have some lifetime `'a`, the output value's lifetime must also live at least as long as `'a`, that is, their lifetimes are tied together (because they're both references into the flatbuffer).
The mutable reference itself is not tied to the references inside the flatbuffer, so it doesn't get a lifetime annotation at all (and the lifetime elision rules mean that it's interpreted as having a lifetime not related to the lifetimes of any of the other types in this signature).
As far as I can tell, these restrictions hold for the rest of the functions in this file. Some of the other functions indeed had the `'a` on the references within the `FlatBufferBuilder` rather than on the reference to the `FlatBufferBuilder`, but were still incorrect in the way they were relating the input and output lifetimes.
In addition to enabling the code reuse in the first commit, these changes also enable `schema_to_fb` to call `schema_to_fb_offset` and reduce more duplication.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nevi-me closed pull request #8752: ARROW-10705: [Rust] Loosen restrictions on some lifetime annotations
Posted by GitBox <gi...@apache.org>.
nevi-me closed pull request #8752:
URL: https://github.com/apache/arrow/pull/8752
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #8752: ARROW-10705: [Rust] Loosen restrictions on some lifetime annotations
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8752:
URL: https://github.com/apache/arrow/pull/8752#issuecomment-733678504
Thank you @carols10cents -- this was an epic PR description
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8752: ARROW-10705: [Rust] Loosen restrictions on some lifetime annotations
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8752:
URL: https://github.com/apache/arrow/pull/8752#issuecomment-732438833
https://issues.apache.org/jira/browse/ARROW-10705
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org