Expose functions for dealing Arc<Vec<u8>>

RESOLVED FIXED in Firefox 56

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment)

We'll need this for interacting with the font data from C++
Assignee: nobody → jmuizelaar
Attachment #8887616 - Flags: review?(rhunt)
Blocks: 1380014
Comment on attachment 8887616 [details] [diff] [review]
Expose functions for dealing with Arc<Vec<u8>>

Review of attachment 8887616 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/webrender_bindings/src/bindings.rs
@@ +1728,5 @@
> +pub type VecU8 = Vec<u8>;
> +pub type ArcVecU8 = Arc<VecU8>;
> +
> +#[no_mangle]
> +pub unsafe extern "C" fn wr_add_ref_arc(arc: &ArcVecU8) -> *const VecU8 {

nit: The bindings file is all over the place on this, but I think we should be using *const instead of & in FFI function interfaces.

They should codegen to be equivalent, but it emphasizes that the pointer might not follow rust's reference rules.

@@ +1733,5 @@
> +    Arc::into_raw(arc.clone())
> +}
> +
> +#[no_mangle]
> +pub extern "C" fn wr_dec_ref_arc(arc: *const VecU8) {

nit: you can annotate this function unsafe like the other one.
Attachment #8887616 - Flags: review?(rhunt) → review+
(In reply to Ryan Hunt [:rhunt] from comment #2)
> Comment on attachment 8887616 [details] [diff] [review]
> Expose functions for dealing with Arc<Vec<u8>>
> 
> Review of attachment 8887616 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/webrender_bindings/src/bindings.rs
> @@ +1728,5 @@
> > +pub type VecU8 = Vec<u8>;
> > +pub type ArcVecU8 = Arc<VecU8>;
> > +
> > +#[no_mangle]
> > +pub unsafe extern "C" fn wr_add_ref_arc(arc: &ArcVecU8) -> *const VecU8 {
> 
> nit: The bindings file is all over the place on this, but I think we should
> be using *const instead of & in FFI function interfaces.
> 
> They should codegen to be equivalent, but it emphasizes that the pointer
> might not follow rust's reference rules.

In this case I actually want the rust reference rules to be followed.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> (In reply to Ryan Hunt [:rhunt] from comment #2)
> > Comment on attachment 8887616 [details] [diff] [review]
> > Expose functions for dealing with Arc<Vec<u8>>
> > 
> > Review of attachment 8887616 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: gfx/webrender_bindings/src/bindings.rs
> > @@ +1728,5 @@
> > > +pub type VecU8 = Vec<u8>;
> > > +pub type ArcVecU8 = Arc<VecU8>;
> > > +
> > > +#[no_mangle]
> > > +pub unsafe extern "C" fn wr_add_ref_arc(arc: &ArcVecU8) -> *const VecU8 {
> > 
> > nit: The bindings file is all over the place on this, but I think we should
> > be using *const instead of & in FFI function interfaces.
> > 
> > They should codegen to be equivalent, but it emphasizes that the pointer
> > might not follow rust's reference rules.
> 
> In this case I actually want the rust reference rules to be followed.

That's okay then. It won't be enforced on C/C++ code, but at the least it's written somewhere.
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8000ce443309
Expose functions for dealing Arc<Vec<u8>> r=rhunt
https://hg.mozilla.org/mozilla-central/rev/8000ce443309
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.