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
