Patchwork [5,of,6] rust: look up HgRevlogIndex_GetParents() from symbol table

login
register
mail settings
Submitter Yuya Nishihara
Date Dec. 5, 2018, 1:43 p.m.
Message ID <f5cdfa49994e3943ba7c.1544017416@mimosa>
Download mbox | patch
Permalink /patch/36978/
State Accepted
Headers show

Comments

Yuya Nishihara - Dec. 5, 2018, 1:43 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1543756838 -32400
#      Sun Dec 02 22:20:38 2018 +0900
# Node ID f5cdfa49994e3943ba7c4ce2d66708142f0c7058
# Parent  716a73bab79be30c20c75e13324c44205d5e2120
rust: look up HgRevlogIndex_GetParents() from symbol table

And removes the unused index_get_parents_checked() function.

I expect the Index struct will be turned into a pyobject type, though I
haven't written any PoC-level patches yet.
Georges Racinet - Dec. 10, 2018, 6 p.m.
On 12/5/18 2:43 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1543756838 -32400
> #      Sun Dec 02 22:20:38 2018 +0900
> # Node ID f5cdfa49994e3943ba7c4ce2d66708142f0c7058
> # Parent  716a73bab79be30c20c75e13324c44205d5e2120
> rust: look up HgRevlogIndex_GetParents() from symbol table
>
> And removes the unused index_get_parents_checked() function.
>
> I expect the Index struct will be turned into a pyobject type, though I
> haven't written any PoC-level patches yet.

I'm not sure what you mean exactly with turning Index into a pyobject
type. Would you care to elaborate ?

No question about the rest of the patch.

>
> diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
> --- a/mercurial/cext/revlog.c
> +++ b/mercurial/cext/revlog.c
> @@ -2710,27 +2710,15 @@ struct rustlazyancestorsObjectStruct {
>  };
>  
>  /* FFI exposed from Rust code */
> -rustlazyancestorsObject *
> -rustlazyancestors_init(indexObject *index,
> -                       /* to pass index_get_parents() */
> -                       int (*)(indexObject *, Py_ssize_t, int *, int),
> -                       /* intrevs vector */
> -                       Py_ssize_t initrevslen, long *initrevs, long stoprev,
> -                       int inclusive);
> +rustlazyancestorsObject *rustlazyancestors_init(indexObject *index,
> +                                                /* intrevs vector */
> +                                                Py_ssize_t initrevslen,
> +                                                long *initrevs, long stoprev,
> +                                                int inclusive);
>  void rustlazyancestors_drop(rustlazyancestorsObject *self);
>  int rustlazyancestors_next(rustlazyancestorsObject *self);
>  int rustlazyancestors_contains(rustlazyancestorsObject *self, long rev);
>  
> -static int index_get_parents_checked(indexObject *self, Py_ssize_t rev, int *ps,
> -                                     int maxrev)
> -{
> -	if (rev < 0 || rev >= index_length(self)) {
> -		PyErr_SetString(PyExc_ValueError, "rev out of range");
> -		return -1;
> -	}
> -	return index_get_parents(self, rev, ps, maxrev);
> -}
> -
>  /* CPython instance methods */
>  static int rustla_init(rustlazyancestorsObject *self, PyObject *args)
>  {
> @@ -2768,12 +2756,12 @@ static int rustla_init(rustlazyancestors
>  	if (PyErr_Occurred())
>  		goto bail;
>  
> -	self->iter = rustlazyancestors_init(index, index_get_parents, linit,
> -	                                    initrevs, stoprev, inclusive);
> +	self->iter =
> +	    rustlazyancestors_init(index, linit, initrevs, stoprev, inclusive);
>  	if (self->iter == NULL) {
>  		/* if this is because of GraphError::ParentOutOfRange
> -		 * index_get_parents_checked() has already set the proper
> -		 * ValueError */
> +		 * HgRevlogIndex_GetParents() has already set the proper
> +		 * exception */
>  		goto bail;
>  	}
>  
> diff --git a/rust/hg-direct-ffi/src/ancestors.rs b/rust/hg-direct-ffi/src/ancestors.rs
> --- a/rust/hg-direct-ffi/src/ancestors.rs
> +++ b/rust/hg-direct-ffi/src/ancestors.rs
> @@ -16,9 +16,14 @@ use std::ptr::null_mut;
>  use std::slice;
>  
>  type IndexPtr = *mut c_void;
> -type IndexParentsFn =
> -    unsafe extern "C" fn(index: IndexPtr, rev: ssize_t, ps: *mut [c_int; 2], max_rev: c_int)
> -        -> c_int;
> +
> +extern "C" {
> +    fn HgRevlogIndex_GetParents(
> +        op: IndexPtr,
> +        rev: c_int,
> +        parents: *mut [c_int; 2],
> +    ) -> c_int;
> +}
>  
>  /// A Graph backed up by objects and functions from revlog.c
>  ///
> @@ -27,14 +32,12 @@ type IndexParentsFn =
>  /// - the `index_get_parents()` function (`parents` member)
>  pub struct Index {
>      index: IndexPtr,
> -    parents: IndexParentsFn,
>  }
>  
>  impl Index {
> -    pub fn new(index: IndexPtr, parents: IndexParentsFn) -> Self {
> +    pub fn new(index: IndexPtr) -> Self {
>          Index {
>              index: index,
> -            parents: parents,
>          }
>      }
>  }
> @@ -44,7 +47,7 @@ impl Graph for Index {
>      fn parents(&self, rev: Revision) -> Result<(Revision, Revision), GraphError> {
>          let mut res: [c_int; 2] = [0; 2];
>          let code =
> -            unsafe { (self.parents)(self.index, rev as ssize_t, &mut res as *mut [c_int; 2], rev) };
> +            unsafe { HgRevlogIndex_GetParents(self.index, rev, &mut res as *mut [c_int; 2]) };
>          match code {
>              0 => Ok((res[0], res[1])),
>              _ => Err(GraphError::ParentOutOfRange(rev)),
> @@ -59,7 +62,6 @@ impl Graph for Index {
>  #[no_mangle]
>  pub extern "C" fn rustlazyancestors_init(
>      index: IndexPtr,
> -    parents: IndexParentsFn,
>      initrevslen: ssize_t,
>      initrevs: *mut c_long,
>      stoprev: c_long,
> @@ -68,7 +70,7 @@ pub extern "C" fn rustlazyancestors_init
>      assert!(initrevslen >= 0);
>      unsafe {
>          raw_init(
> -            Index::new(index, parents),
> +            Index::new(index),
>              initrevslen as usize,
>              initrevs,
>              stoprev,
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - Dec. 11, 2018, 1:08 p.m.
On Mon, 10 Dec 2018 19:00:43 +0100, Georges Racinet wrote:
> On 12/5/18 2:43 PM, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1543756838 -32400
> > #      Sun Dec 02 22:20:38 2018 +0900
> > # Node ID f5cdfa49994e3943ba7c4ce2d66708142f0c7058
> > # Parent  716a73bab79be30c20c75e13324c44205d5e2120
> > rust: look up HgRevlogIndex_GetParents() from symbol table
> >
> > And removes the unused index_get_parents_checked() function.
> >
> > I expect the Index struct will be turned into a pyobject type, though I
> > haven't written any PoC-level patches yet.
> 
> I'm not sure what you mean exactly with turning Index into a pyobject
> type. Would you care to elaborate ?

I mean we'll probably want a Rust type wrapping a Python Index object,
something like:

  impl Index {
      pub fn parents(&self, py: Python, rev: Revision)
                     -> PyResult<[Revision; 2]>
      ...
  }

Patch

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -2710,27 +2710,15 @@  struct rustlazyancestorsObjectStruct {
 };
 
 /* FFI exposed from Rust code */
-rustlazyancestorsObject *
-rustlazyancestors_init(indexObject *index,
-                       /* to pass index_get_parents() */
-                       int (*)(indexObject *, Py_ssize_t, int *, int),
-                       /* intrevs vector */
-                       Py_ssize_t initrevslen, long *initrevs, long stoprev,
-                       int inclusive);
+rustlazyancestorsObject *rustlazyancestors_init(indexObject *index,
+                                                /* intrevs vector */
+                                                Py_ssize_t initrevslen,
+                                                long *initrevs, long stoprev,
+                                                int inclusive);
 void rustlazyancestors_drop(rustlazyancestorsObject *self);
 int rustlazyancestors_next(rustlazyancestorsObject *self);
 int rustlazyancestors_contains(rustlazyancestorsObject *self, long rev);
 
-static int index_get_parents_checked(indexObject *self, Py_ssize_t rev, int *ps,
-                                     int maxrev)
-{
-	if (rev < 0 || rev >= index_length(self)) {
-		PyErr_SetString(PyExc_ValueError, "rev out of range");
-		return -1;
-	}
-	return index_get_parents(self, rev, ps, maxrev);
-}
-
 /* CPython instance methods */
 static int rustla_init(rustlazyancestorsObject *self, PyObject *args)
 {
@@ -2768,12 +2756,12 @@  static int rustla_init(rustlazyancestors
 	if (PyErr_Occurred())
 		goto bail;
 
-	self->iter = rustlazyancestors_init(index, index_get_parents, linit,
-	                                    initrevs, stoprev, inclusive);
+	self->iter =
+	    rustlazyancestors_init(index, linit, initrevs, stoprev, inclusive);
 	if (self->iter == NULL) {
 		/* if this is because of GraphError::ParentOutOfRange
-		 * index_get_parents_checked() has already set the proper
-		 * ValueError */
+		 * HgRevlogIndex_GetParents() has already set the proper
+		 * exception */
 		goto bail;
 	}
 
diff --git a/rust/hg-direct-ffi/src/ancestors.rs b/rust/hg-direct-ffi/src/ancestors.rs
--- a/rust/hg-direct-ffi/src/ancestors.rs
+++ b/rust/hg-direct-ffi/src/ancestors.rs
@@ -16,9 +16,14 @@  use std::ptr::null_mut;
 use std::slice;
 
 type IndexPtr = *mut c_void;
-type IndexParentsFn =
-    unsafe extern "C" fn(index: IndexPtr, rev: ssize_t, ps: *mut [c_int; 2], max_rev: c_int)
-        -> c_int;
+
+extern "C" {
+    fn HgRevlogIndex_GetParents(
+        op: IndexPtr,
+        rev: c_int,
+        parents: *mut [c_int; 2],
+    ) -> c_int;
+}
 
 /// A Graph backed up by objects and functions from revlog.c
 ///
@@ -27,14 +32,12 @@  type IndexParentsFn =
 /// - the `index_get_parents()` function (`parents` member)
 pub struct Index {
     index: IndexPtr,
-    parents: IndexParentsFn,
 }
 
 impl Index {
-    pub fn new(index: IndexPtr, parents: IndexParentsFn) -> Self {
+    pub fn new(index: IndexPtr) -> Self {
         Index {
             index: index,
-            parents: parents,
         }
     }
 }
@@ -44,7 +47,7 @@  impl Graph for Index {
     fn parents(&self, rev: Revision) -> Result<(Revision, Revision), GraphError> {
         let mut res: [c_int; 2] = [0; 2];
         let code =
-            unsafe { (self.parents)(self.index, rev as ssize_t, &mut res as *mut [c_int; 2], rev) };
+            unsafe { HgRevlogIndex_GetParents(self.index, rev, &mut res as *mut [c_int; 2]) };
         match code {
             0 => Ok((res[0], res[1])),
             _ => Err(GraphError::ParentOutOfRange(rev)),
@@ -59,7 +62,6 @@  impl Graph for Index {
 #[no_mangle]
 pub extern "C" fn rustlazyancestors_init(
     index: IndexPtr,
-    parents: IndexParentsFn,
     initrevslen: ssize_t,
     initrevs: *mut c_long,
     stoprev: c_long,
@@ -68,7 +70,7 @@  pub extern "C" fn rustlazyancestors_init
     assert!(initrevslen >= 0);
     unsafe {
         raw_init(
-            Index::new(index, parents),
+            Index::new(index),
             initrevslen as usize,
             initrevs,
             stoprev,