[PATCH 2 of 5 RFC] rust: iterator bindings to C code

Augie Fackler raf at durin42.com
Wed Jan 16 09:32:34 UTC 2019


On Fri, Sep 28, 2018 at 03:31:09PM +0200, Georges Racinet wrote:
> # HG changeset patch
> # User Georges Racinet <gracinet at anybox.fr>
> # Date 1538059896 -7200
> #      Thu Sep 27 16:51:36 2018 +0200
> # Node ID de88c09512565ed1c12e2ff9159e06ed8d762d15
> # Parent  d8c9571755a64e1fc3429587dfd3949b9862eceb
> # EXP-Topic rustancestors-rfc
> rust: iterator bindings to C code
>
> In this changeset, still made of Rust code only,
> we expose the Rust iterator for instantiation and
> consumption from C code.
>
> The idea is that both the index and index_get_parents()
> will be passed from the C extension, hence avoiding a hard
> link dependency to parsers.so, so that the crate can
> still be built and tested independently.
>
> On the other hand, parsers.so will use the symbols
> defined in this changeset.
>
> diff -r d8c9571755a6 -r de88c0951256 mercurial/rust/Cargo.lock
> --- a/mercurial/rust/Cargo.lock	Thu Sep 27 17:03:16 2018 +0200
> +++ b/mercurial/rust/Cargo.lock	Thu Sep 27 16:51:36 2018 +0200
> @@ -1,4 +1,14 @@
>  [[package]]
>  name = "hgancestors"
>  version = "0.1.0"
> +dependencies = [
> + "libc 0.2.43 (registry+https://github.com/rust-lang/crates.io-index)",
> +]
>
> +[[package]]
> +name = "libc"
> +version = "0.2.43"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +
> +[metadata]
> +"checksum libc 0.2.43 (registry+https://github.com/rust-lang/crates.io-index)" = "76e3a3ef172f1a0b9a9ff0dd1491ae5e6c948b94479a3021819ba7d860c8645d"
> diff -r d8c9571755a6 -r de88c0951256 mercurial/rust/Cargo.toml
> --- a/mercurial/rust/Cargo.toml	Thu Sep 27 17:03:16 2018 +0200
> +++ b/mercurial/rust/Cargo.toml	Thu Sep 27 16:51:36 2018 +0200
> @@ -2,3 +2,9 @@
>  name = "hgancestors"
>  version = "0.1.0"
>  authors = ["Georges Racinet <gracinet at anybox.fr>"]
> +
> +[dependencies]
> +libc = "*"
> +
> +[lib]
> +crate-type = ["dylib"]
> diff -r d8c9571755a6 -r de88c0951256 mercurial/rust/src/cpython.rs
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/mercurial/rust/src/cpython.rs	Thu Sep 27 16:51:36 2018 +0200
> @@ -0,0 +1,170 @@
> +// Copyright 2018 Georges Racinet <gracinet at anybox.fr>
> +//
> +// This software may be used and distributed according to the terms of the
> +// GNU General Public License version 2 or any later version.
> +
> +//! Bindings for CPython extension code

This looks like you're using cffi and not cpython extension logic? I
don't see anything python-specific in here, so maybe this is misnamed?

If I've misread this, why this route rather than using the cpython
crate, so that the `unsafe` code is consolidated into one package?

> +//!
> +//! This exposes methods to build and use a `rustlazyancestors` iterator
> +//! from C code, using an index and its parents function that are passed
> +//! from the caller at instantiation.
> +
> +extern crate libc;
> +
> +use std::boxed::Box;
> +use self::libc::{c_void, c_int, ssize_t};
> +use super::ancestors::{Graph, AncestorsIterator};
> +
> +type IndexPtr = *mut c_void;
> +type IndexParentsFn = extern "C" fn(index: IndexPtr,
> +                                    rev: ssize_t,
> +                                    ps: *mut [c_int; 2],
> +                                    max_rev: c_int)
> +                                    -> c_int;
> +
> +/// A Graph backed up by objects and functions from revlog.c
> +///
> +/// This implementation of the Graph trait, relies on (pointers to)
> +/// - the C index object (`index` member)
> +/// - the `index_get_parents()` function (`parents` member)
> +pub struct Index {
> +    index: IndexPtr,
> +    parents: IndexParentsFn,
> +}
> +
> +impl Index {
> +    pub fn new(index: IndexPtr, parents: IndexParentsFn) -> Self {
> +        Index {
> +            index: index,
> +            parents: parents,
> +        }
> +    }
> +}
> +
> +impl Graph for Index {
> +    /// wrap a call to the C extern parents function
> +    fn parents(&self, rev: i32) -> (i32, i32) {
> +        let mut res: [c_int; 2] = [0; 2];
> +        // surprisingly the call below is not unsafe, whereas calling the
> +        // same extern function directly (not through a pointer) would.
> +        // Maybe that depends on rustc version, though.
> +        let code = (self.parents)(
> +            self.index,
> +            rev as ssize_t,
> +            &mut res as *mut [c_int; 2],
> +            rev,
> +        );
> +        if code != 0 {
> +            // TODO panic! and FFI don't get well together
> +            panic!("Corrupted index");
> +        }
> +        (res[0], res[1])
> +    }
> +}
> +
> +/// Wrapping of AncestorsIterator<CIndex> constructor, for C callers.
> +///
> +/// Besides `initrevs`, `stoprev` and `inclusive`, that are converted
> +/// we receive the index and the parents function as pointers
> +#[no_mangle]
> +pub extern "C" fn rustlazyancestors_init(
> +    index: IndexPtr,
> +    parents: IndexParentsFn,
> +    initrevslen: usize,
> +    initrevs: *mut i64,
> +    stoprev: i64,
> +    inclusive: i64,
> +) -> *mut AncestorsIterator<Index> {
> +    let v: Vec<i64> =
> +        unsafe { Vec::from_raw_parts(initrevs, initrevslen, initrevslen) };
> +    let inclb = match inclusive {
> +        0 => false,
> +        1 => true,
> +        _ => panic!("Did not understand boolean FFI convention"),
> +    };
> +
> +    Box::into_raw(Box::new(AncestorsIterator::new(
> +        Index::new(index, parents),
> +        &v,
> +        stoprev,
> +        inclb,
> +    )))
> +}
> +
> +/// Deallocator to be called from C code
> +#[no_mangle]
> +pub extern "C" fn rustlazyancestors_drop(
> +    raw_iter: *mut AncestorsIterator<Index>,
> +) {
> +    unsafe {
> +        Box::from_raw(raw_iter);
> +    }
> +}
> +
> +/// Iteration main method to be called from C code
> +///
> +/// We use -1 to mean the end of iteration
> +#[no_mangle]
> +pub extern "C" fn rustlazyancestors_next(
> +    iter_raw: *mut AncestorsIterator<Index>,
> +) -> i32 {
> +    let iter = unsafe { &mut *iter_raw };
> +    match iter.next() {
> +        Some(i) => i,
> +        None => -1,
> +    }
> +}
> +
> +#[cfg(test)]
> +mod tests {
> +    use std::thread;
> +    use super::*;
> +
> +    #[derive(Clone, Debug)]
> +    enum Stub {
> +        Some,
> +    }
> +
> +    impl Graph for Stub {
> +        fn parents(&self, _i: i32) -> (i32, i32) {
> +            (1, 2)
> +        }
> +    }
> +
> +    fn it_init_raw(
> +        graph: &Stub,
> +        initrevs: &Vec<i64>,
> +        stoprev: i64,
> +    ) -> *mut AncestorsIterator<Stub> {
> +        // inclusive=true forces to push immediately stuff in the binary heap
> +        Box::into_raw(Box::new(AncestorsIterator::new(
> +            graph.clone(),
> +            initrevs,
> +            stoprev,
> +            true,
> +        )))
> +    }
> +
> +    fn it_next_raw(iter_raw: *mut AncestorsIterator<Stub>) -> i32 {
> +        let iter = unsafe { &mut *iter_raw };
> +        let res = match iter.next() {
> +            Some(i) => i,
> +            None => -1,
> +        };
> +        res
> +    }
> +
> +    #[test]
> +    // Test what happens when we init an Iterator as with the exposed C ABI
> +    // and try to use it afterwards
> +    // We spawn new threads, in order to make memory consistency harder
> +    fn test_back_and_forth() {
> +        let handler = thread::spawn(
> +            || it_init_raw(&Stub::Some, &vec![11, 13], 0) as u64,
> +        );
> +        let iter_raw = handler.join().unwrap() as *mut AncestorsIterator<Stub>;
> +        assert_eq!(it_next_raw(iter_raw), 13);
> +        assert_eq!(it_next_raw(iter_raw), 11);
> +        unsafe { Box::from_raw(iter_raw) };
> +    }
> +}
> diff -r d8c9571755a6 -r de88c0951256 mercurial/rust/src/lib.rs
> --- a/mercurial/rust/src/lib.rs	Thu Sep 27 17:03:16 2018 +0200
> +++ b/mercurial/rust/src/lib.rs	Thu Sep 27 16:51:36 2018 +0200
> @@ -2,6 +2,9 @@
>  //
>  // This software may be used and distributed according to the terms of the
>  // GNU General Public License version 2 or any later version.
> -
>  mod ancestors;
>  pub use ancestors::{AncestorsIterator, Graph};
> +
> +mod cpython;
> +pub use cpython::{rustlazyancestors_init, rustlazyancestors_drop,
> +                  rustlazyancestors_next};
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list