This is an archive of the discontinued Mercurial Phabricator instance.

+fn reviter_to_revvec(py: Python, revs: PyObject) -> PyResult<Vec<Revision>> {
+ let revs_iter = revs.iter(py)?;
+ we need to convert to a vector, because Python iterables can
+ raise errors at each step.
+ let cap = match revs.len(py) {
+ Ok(l) => l,
+ Err(_) => 0, // unknown
+ };
+ let mut initvec: Vec<Revision> = Vec::with_capacity(cap);
+ for result_revpy in revs_iter {
+ let as_pyint: PyInt = match result_revpy {
+ Err(e) => {
+ return Err(e);
+ }
+ Ok(revpy) => revpy.extract(py)?,
+ };
+ initvec.push(as_pyint.value(py) as Revision);
+ }
+ Ok(initvec)

revs_iter.map(|r| r.and_then(|o| o.extract::<Revision>(py))).collect()

PyInt::value() isn't supported on Python 3, and it should be better to
extract int directly with bounds checking.

https://dgrunwald.github.io/rust-cpython/doc/cpython/struct.PyInt.html#method.value

+impl AncestorsIterator {
+ pub fn from_inner(py: Python, ait: CoreIterator<Index>) -> PyResult<Self> {

Nit: I slightly prefer spelling it as hg::AncestorsIterator.

@yuja thanks for spotting the Python3 incompatibility. As you can guess, I didn't compile with Python 3, and actually I hadn't even defined the features to that extent.
I will submit separately a change that takes care of the build with Python 3 before updating that one (but yes, got test-rust-ancestor.py to pass).
OTOH I'll have to keep the loop so that I can get the early return upon error (otherwise we'd get a Vec<PyResult<...>>

@kevincox thanks for the shorter version, wasn't used to these sugars when I wrote this long ago :-)

OTOH I'll have to keep the loop so that I can get the early return upon error (otherwise we'd get a Vec<PyResult<...>>

I didn't check the implementation, but collect() for Result will return
immediately if reached to Err.

@yuja you're right! My first attempt was tainted with assigning to a Vec, whereas upon type inference with the actual return type PyResult<Vec>, the compiler does the right thing so that was the escape plan for early returns in closures :-) amazing.

gracinet updated this revision to Diff 12950.Dec 22 2018, 11:23 AM

gracinet updated this revision to Diff 12953.

@yuja actually my first version of CoreIterator (and later CoreLazy) used the hg::AncestorsIterator spelling, but actually I believe we might end up after a while not exporting everything at the top of the hg crate, leaving us either to use hg::ancestors, giving us a long and puzzling ancestors::AncestorsIterator or directly as hg:ancestors::AncestorsIterator. I don't think it would be tasteful to use hg::ancestors as core.

That's why I found it clearer to call them CoreIterator etc. But I can change it if you really dislike them.

Closed by commit rHGd9f439fcdb4c: rust-cpython: binding for AncestorsIterator (authored by gracinet). · Explain WhyDec 24 2018, 12:00 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			rust/hg-cpython/src/ancestors.rs (64 lines)
M			tests/test-rust-ancestor.py (78 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	12876		Dec 15 2018, 6:42 AM	★	★
Diff 2	12950		Dec 22 2018, 11:23 AM	★	★
Diff 3	12953		Dec 22 2018, 11:24 AM	★	★
Diff 4	12966	rHGd9f439fcdb4cdda90e9a58f46075436c0b30432d	Dec 6 2018, 2:01 PM	★	★

Status	Author	Revision
Abandoned	gracinet	D5443 ancestor: uniformity of calling lazyancestors classes
Closed	gracinet	D5442 rust-cpython: using the new bindings from Python
Closed	gracinet	D5441 rust-cpython: binding for LazyAncestors
Closed	gracinet	D5440 rust: core implementation for lazyancestors
Closed	gracinet	D5439 rust-cpython: binding for AncestorsIterator
Closed	gracinet	D5438 rust-cpython: implementing Graph using C parents function
Closed	gracinet	D5437 rust-cpython: testing the bindings from Python
Closed	gracinet	D5436 rust-cpython: build via HGWITHRUSTEXT=cpython
Closed	gracinet	D5435 rust: better treatment of cargo/rustc errors
Closed	gracinet	D5434 rust-cpython: started cpython crate bindings
Closed	gracinet	D5433 rust-cpython: excluded hgcli from workspace

Diff 12966

rust/hg-cpython/src/ancestors.rs

	// ancestors.rs			// ancestors.rs
	//			//
	// Copyright 2018 Georges Racinet <gracinet@anybox.fr>			// Copyright 2018 Georges Racinet <gracinet@anybox.fr>
	//			//
	// This software may be used and distributed according to the terms of the			// This software may be used and distributed according to the terms of the
	// GNU General Public License version 2 or any later version.			// GNU General Public License version 2 or any later version.

	//! Bindings for the hg::ancestors module provided by the			//! Bindings for the hg::ancestors module provided by the
	//! `hg-core` crate. From Python, this will be seen as `rustext.ancestor`			//! `hg-core` crate. From Python, this will be seen as `rustext.ancestor`
	use cpython::{PyDict, PyModule, PyResult, Python};			use cindex::Index;
				use cpython::{
				ObjectProtocol, PyClone, PyDict, PyModule, PyObject, PyResult, Python,
				};
				use exceptions::GraphError;
				use hg;
				use hg::AncestorsIterator as CoreIterator;
				use hg::Revision;
				use std::cell::RefCell;

				/// Utility function to convert a Python iterable into a Vec<Revision>
				///
				/// We need this to feed to AncestorIterators constructors because
				/// a PyErr can arise at each step of iteration, whereas our inner objects
				/// expect iterables over Revision, not over some Result<Revision, PyErr>
				fn reviter_to_revvec(py: Python, revs: PyObject) -> PyResult<Vec<Revision>> {
				revs.iter(py)?
				.map(\|r\| r.and_then(\|o\| o.extract::<Revision>(py)))
				.collect()
				}

				py_class!(class AncestorsIterator \|py\| {
				// TODO RW lock ?
				data inner: RefCell<Box<CoreIterator<Index>>>;
				kevincoxUnsubmitted Not Done revs.len(py).unwrap_or(0) kevincox: ``` revs.len(py).unwrap_or(0) ```

				def __next__(&self) -> PyResult<Option<Revision>> {
				match self.inner(py).borrow_mut().next() {
				Some(Err(e)) => Err(GraphError::pynew(py, e)),
				None => Ok(None),
				Some(Ok(r)) => Ok(Some(r)),
				}
				}
				kevincoxUnsubmitted Not Done result_revpy?.extract(py)? kevincox: ``` result_revpy?.extract(py)? ```

				def __contains__(&self, rev: Revision) -> PyResult<bool> {
				self.inner(py).borrow_mut().contains(rev).map_err(\|e\| GraphError::pynew(py, e))
				}

				def __iter__(&self) -> PyResult<Self> {
				Ok(self.clone_ref(py))
				}

				def __new__(_cls, index: PyObject, initrevs: PyObject, stoprev: Revision,
				inclusive: bool) -> PyResult<AncestorsIterator> {
				let initvec = reviter_to_revvec(py, initrevs)?;
				let ait = match hg::AncestorsIterator::new(Index::new(py, index)?,
				initvec, stoprev,
				inclusive) {
				Ok(ait) => ait,
				Err(e) => {
				return Err(GraphError::pynew(py, e));
				}
				};
				AncestorsIterator::from_inner(py, ait)
				}

				});

				impl AncestorsIterator {
				pub fn from_inner(py: Python, ait: CoreIterator<Index>) -> PyResult<Self> {
				Self::create_instance(py, RefCell::new(Box::new(ait)))
				}
				}

	/// Create the module, with __package__ given from parent			/// Create the module, with __package__ given from parent
	pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {			pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
	let dotted_name = &format!("{}.ancestor", package);			let dotted_name = &format!("{}.ancestor", package);
	let m = PyModule::new(py, dotted_name)?;			let m = PyModule::new(py, dotted_name)?;
	m.add(py, "__package__", package)?;			m.add(py, "__package__", package)?;
	m.add(			m.add(
	py,			py,
	"__doc__",			"__doc__",
	"Generic DAG ancestor algorithms - Rust implementation",			"Generic DAG ancestor algorithms - Rust implementation",
	)?;			)?;
				m.add_class::<AncestorsIterator>(py)?;

	let sys = PyModule::import(py, "sys")?;			let sys = PyModule::import(py, "sys")?;
	let sys_modules: PyDict = sys.get(py, "modules")?.extract(py)?;			let sys_modules: PyDict = sys.get(py, "modules")?.extract(py)?;
	sys_modules.set_item(py, dotted_name, &m)?;			sys_modules.set_item(py, dotted_name, &m)?;
	// Example C code (see pyexpat.c and import.c) will "give away the			// Example C code (see pyexpat.c and import.c) will "give away the
	// reference", but we won't because it will be consumed once the			// reference", but we won't because it will be consumed once the
	// Rust PyObject is dropped.			// Rust PyObject is dropped.
	Ok(m)			Ok(m)
	}			}

tests/test-rust-ancestor.py

	from __future__ import absolute_import			from __future__ import absolute_import
				import sys
	import unittest			import unittest

	try:			try:
	from mercurial import rustext			from mercurial import rustext
				rustext.__name__ # trigger immediate actual import
	except ImportError:			except ImportError:
	rustext = None			rustext = None
				else:
				# this would fail already without appropriate ancestor.__package__
				from mercurial.rustext.ancestor import AncestorsIterator

	try:			try:
	from mercurial.cext import parsers as cparsers			from mercurial.cext import parsers as cparsers
	except ImportError:			except ImportError:
	cparsers = None			cparsers = None

				# picked from test-parse-index2, copied rather than imported
				# so that it stays stable even if test-parse-index2 changes or disappears.
				data_non_inlined = (
				b'\x00\x00\x00\x01\x00\x00\x00\x00\x00\x01D\x19'
				b'\x00\x07e\x12\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff'
				b'\xff\xff\xff\xff\xd1\xf4\xbb\xb0\xbe\xfc\x13\xbd\x8c\xd3\x9d'
				b'\x0f\xcd\xd9;\x8c\x07\x8cJ/\x00\x00\x00\x00\x00\x00\x00\x00\x00'
				b'\x00\x00\x00\x00\x00\x00\x01D\x19\x00\x00\x00\x00\x00\xdf\x00'
				b'\x00\x01q\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x00\xff'
				b'\xff\xff\xff\xc1\x12\xb9\x04\x96\xa4Z1t\x91\xdfsJ\x90\xf0\x9bh'
				b'\x07l&\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
				b'\x00\x01D\xf8\x00\x00\x00\x00\x01\x1b\x00\x00\x01\xb8\x00\x00'
				b'\x00\x01\x00\x00\x00\x02\x00\x00\x00\x01\xff\xff\xff\xff\x02\n'
				b'\x0e\xc6&\xa1\x92\xae6\x0b\x02i\xfe-\xe5\xbao\x05\xd1\xe7\x00'
				b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01F'
				b'\x13\x00\x00\x00\x00\x01\xec\x00\x00\x03\x06\x00\x00\x00\x01'
				b'\x00\x00\x00\x03\x00\x00\x00\x02\xff\xff\xff\xff\x12\xcb\xeby1'
				b'\xb6\r\x98B\xcb\x07\xbd`\x8f\x92\xd9\xc4\x84\xbdK\x00\x00\x00'
				b'\x00\x00\x00\x00\x00\x00\x00\x00\x00'
				)


	@unittest.skipIf(rustext is None or cparsers is None,			@unittest.skipIf(rustext is None or cparsers is None,
	"rustext.ancestor or the C Extension parsers module "			"rustext or the C Extension parsers module "
	"it relies on is not available")			"ancestor relies on is not available")
	class rustancestorstest(unittest.TestCase):			class rustancestorstest(unittest.TestCase):
	"""Test the correctness of binding to Rust code.			"""Test the correctness of binding to Rust code.

	This test is merely for the binding to Rust itself: extraction of			This test is merely for the binding to Rust itself: extraction of
	Python variable, giving back the results etc.			Python variable, giving back the results etc.

	It is not meant to test the algorithmic correctness of the operations			It is not meant to test the algorithmic correctness of the operations
	on ancestors it provides. Hence the very simple embedded index data is			on ancestors it provides. Hence the very simple embedded index data is
	good enough.			good enough.

	Algorithmic correctness is asserted by the Rust unit tests.			Algorithmic correctness is asserted by the Rust unit tests.
	"""			"""

	def testmodule(self):			def parseindex(self):
	self.assertTrue('DAG' in rustext.ancestor.__doc__)			return cparsers.parse_index2(data_non_inlined, False)[0]

				def testiteratorrevlist(self):
				idx = self.parseindex()
				# checking test assumption about the index binary data:
				self.assertEqual({i: (r[5], r[6]) for i, r in enumerate(idx)},
				{0: (-1, -1),
				1: (0, -1),
				2: (1, -1),
				3: (2, -1)})
				ait = AncestorsIterator(idx, [3], 0, True)
				self.assertEqual([r for r in ait], [3, 2, 1, 0])

				ait = AncestorsIterator(idx, [3], 0, False)
				self.assertEqual([r for r in ait], [2, 1, 0])

				def testrefcount(self):
				idx = self.parseindex()
				start_count = sys.getrefcount(idx)

				# refcount increases upon iterator init...
				ait = AncestorsIterator(idx, [3], 0, True)
				self.assertEqual(sys.getrefcount(idx), start_count + 1)
				self.assertEqual(next(ait), 3)

				# and decreases once the iterator is removed
				del ait
				self.assertEqual(sys.getrefcount(idx), start_count)

				# and removing ref to the index after iterator init is no issue
				ait = AncestorsIterator(idx, [3], 0, True)
				del idx
				self.assertEqual([r for r in ait], [3, 2, 1, 0])

	def testgrapherror(self):			def testgrapherror(self):
	self.assertTrue('GraphError' in dir(rustext))			data = (data_non_inlined[:64 + 27] +
				b'\xf2' +
				data_non_inlined[64 + 28:])
				idx = cparsers.parse_index2(data, False)[0]
				with self.assertRaises(rustext.GraphError) as arc:
				AncestorsIterator(idx, [1], -1, False)
				exc = arc.exception
				self.assertIsInstance(exc, ValueError)
				# rust-cpython issues appropriate str instances for Python 2 and 3
				self.assertEqual(exc.args, ('ParentOutOfRange', 1))


	if __name__ == '__main__':			if __name__ == '__main__':
	import silenttestrunner			import silenttestrunner
	silenttestrunner.main(__name__)			silenttestrunner.main(__name__)

This is an archive of the discontinued Mercurial Phabricator instance.

rust-cpython: binding for AncestorsIteratorClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 12966

rust/hg-cpython/src/ancestors.rs

tests/test-rust-ancestor.py

rust-cpython: binding for AncestorsIterator
ClosedPublic

Revision Contents
Changeset List