neon-bindings / rfcs

RFCs for changes to Neon
Apache License 2.0
14 stars 9 forks source link

Borrow API v1.0 Proposal #44

Closed kjvalencik closed 2 years ago

kjvalencik commented 3 years ago

Neon should provide APIs for safely and ergonomically borrowing binary data. Borrowing should leverage the type system for statically checking safety as much as possible without sacrificing flexibility.

Rendered

Implementation: https://github.com/neon-bindings/neon/pull/780 Pre-release published in 0.10.0-alpha.1.

dherman commented 3 years ago

Does Node-API have an API for detaching the backing buffer out of an ArrayBuffer? That could be another future extension.

dherman commented 3 years ago

Seems like another drawback is you can’t mutably borrow multiple buffers (with eg a dynamic check they don’t alias or overlap). I don’t know if this is a rare enough use case not to matter.

dherman commented 3 years ago

@kjvalencik Remind me, does changing the scoped methods to &mut still allow multiple levels of nested scoped computations? I think there’s no reason that shouldn’t work, right?

kjvalencik commented 3 years ago

Does Node-API have an API for detaching the backing buffer out of an ArrayBuffer? That could be another future extension.

Yes, it does with the following constraints:

Unfortunately, this wouldn't work with our current API design. We accept anything that is AsMut<[u8]> when creating external ArrayBuffer. Since this could be anything, it's important that we run the Drop. However, it's not possible to get the data pointer back out of an external buffer. We would need to have that feature added to Node-API or change Neon to only accept a certain type (e.g., Vec<u8>).

Seems like another drawback is you can’t mutably borrow multiple buffers (with eg a dynamic check they don’t alias or overlap). I don’t know if this is a rare enough use case not to matter.

It's a drawback of the "simple API" and the reason that the proposal includes a dynamically checked part. It's covered in this section: https://github.com/neon-bindings/rfcs/pull/44/files#diff-f7efd300745a6d0dcd2273fa31a292295151b581fb63b5ae24ef8661f1924520R71

@kjvalencik Remind me, does changing the scoped methods to &mut still allow multiple levels of nested scoped computations? I think there’s no reason that shouldn’t work, right?

Yes, it does because the scope creates a new Context and then you can create another nested scope off of that.

dherman commented 3 years ago

This looks great. I'm happy putting it into Final Comment Period.

kjvalencik commented 2 years ago

Merged in https://github.com/neon-bindings/rfcs/commit/8beeb16a87446f32582b106b2716243798e6f89b