gracinet added inline comments. INLINE COMMENTS
> kevincox wrote in ancestors.rs:98 > It should be fine with tbe GIL. RefCell is basically a single-thread lock. In > this case it should be fine to have no renterance. Yes, I'll remove these comments, same with the ones for RW lock, it's pretty clear to me now that such would be relevant only for code meant to be executed in a `allow_threads` closure. > kevincox wrote in ancestors.rs:108 > .map_err(|e| GraphError::new(py, format!("{:?}", e))) > > Or even better is to implement `Into` so that you can use `?` Ah yes, forgot to upgrade that one, thanks. The current good spelling would be the one used in `__contains__`, where `GraphError::pynew` encloses the conversion details. I believe implementing `Into` is not readily possible with the rust-cpython way of creating Python exceptions : we can't do it without the `py` marker (it's actually listed as one of the advantages of PyO3). And since `Python<'p>` is not defined by our crate, we'd need to enclose in a wrapper in the `excepions` module: pub struct ExceptionConverter<'p>(pub Python<'p>, pub hg::GraphError); impl <'p> From<ExceptionConverter<'p>> for PyErr { fn from(ec: ExceptionConverter<'p>) -> Self { GraphError::pynew(ec.0, ec.1) } } and then def __new__(_cls, index: PyObject, initrevs: PyObject, stoprev: Revision, inclusive: bool) -> PyResult<Self> { let initvec = reviter_to_revvec(py, initrevs)?; let lazy = CoreLazy::new( Index::new(py, index)?, initvec, stoprev, inclusive).map_err(|e| ExceptionConverter(py, e))?; Self::create_instance(py, RefCell::new(Box::new(lazy))) } This does compile (did not think hard about the namings), and I'd be surprised if it didn't the right thng, but that's a lot of effort for a result that's only a bit better. What do you think ? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5441 To: gracinet, #hg-reviewers, kevincox Cc: yuja, durin42, kevincox, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel