comparison rust/hg-cpython/src/ancestors.rs @ 50979:4c5f6e95df84

rust: make `Revision` a newtype This change is the one we've been building towards during this series. The aim is to make `Revision` mean more than a simple integer, holding the information that it is valid for a given revlog index. While this still allows for programmer error, since creating a revision directly and querying a different index with a "checked" revision are still possible, the friction created by the newtype will hopefully make us think twice about which type to use. Enough of the Rust ecosystem relies on the newtype pattern to be efficiently optimized away (even compiler in codegen testsĀ¹), so I'm not worried about this being a fundamental problem. [1] https://github.com/rust-lang/rust/blob/7a70647f195f6b0a0f1ebd72b1542ba91a32f43a/tests/codegen/vec-in-place.rs#L47
author Raphaël Gomès <rgomes@octobus.net>
date Fri, 18 Aug 2023 14:34:29 +0200
parents 35ebe6f824be
children 7eea2e4109ae
comparison
equal deleted inserted replaced
50978:27e773aa607d 50979:4c5f6e95df84
33 //! 33 //!
34 //! [`LazyAncestors`]: struct.LazyAncestors.html 34 //! [`LazyAncestors`]: struct.LazyAncestors.html
35 //! [`MissingAncestors`]: struct.MissingAncestors.html 35 //! [`MissingAncestors`]: struct.MissingAncestors.html
36 //! [`AncestorsIterator`]: struct.AncestorsIterator.html 36 //! [`AncestorsIterator`]: struct.AncestorsIterator.html
37 use crate::revlog::pyindex_to_graph; 37 use crate::revlog::pyindex_to_graph;
38 use crate::PyRevision;
38 use crate::{ 39 use crate::{
39 cindex::Index, conversion::rev_pyiter_collect, exceptions::GraphError, 40 cindex::Index, conversion::rev_pyiter_collect, exceptions::GraphError,
40 }; 41 };
41 use cpython::{ 42 use cpython::{
42 ObjectProtocol, PyClone, PyDict, PyList, PyModule, PyObject, PyResult, 43 ObjectProtocol, PyClone, PyDict, PyList, PyModule, PyObject, PyResult,
52 }; 53 };
53 54
54 py_class!(pub class AncestorsIterator |py| { 55 py_class!(pub class AncestorsIterator |py| {
55 data inner: RefCell<Box<VCGAncestorsIterator<Index>>>; 56 data inner: RefCell<Box<VCGAncestorsIterator<Index>>>;
56 57
57 def __next__(&self) -> PyResult<Option<Revision>> { 58 def __next__(&self) -> PyResult<Option<PyRevision>> {
58 match self.inner(py).borrow_mut().next() { 59 match self.inner(py).borrow_mut().next() {
59 Some(Err(e)) => Err(GraphError::pynew_from_vcsgraph(py, e)), 60 Some(Err(e)) => Err(GraphError::pynew_from_vcsgraph(py, e)),
60 None => Ok(None), 61 None => Ok(None),
61 Some(Ok(r)) => Ok(Some(r)), 62 Some(Ok(r)) => Ok(Some(PyRevision(r))),
62 } 63 }
63 } 64 }
64 65
65 def __contains__(&self, rev: Revision) -> PyResult<bool> { 66 def __contains__(&self, rev: PyRevision) -> PyResult<bool> {
66 self.inner(py).borrow_mut().contains(rev) 67 self.inner(py).borrow_mut().contains(rev.0)
67 .map_err(|e| GraphError::pynew_from_vcsgraph(py, e)) 68 .map_err(|e| GraphError::pynew_from_vcsgraph(py, e))
68 } 69 }
69 70
70 def __iter__(&self) -> PyResult<Self> { 71 def __iter__(&self) -> PyResult<Self> {
71 Ok(self.clone_ref(py)) 72 Ok(self.clone_ref(py))
72 } 73 }
73 74
74 def __new__(_cls, index: PyObject, initrevs: PyObject, stoprev: Revision, 75 def __new__(
75 inclusive: bool) -> PyResult<AncestorsIterator> { 76 _cls,
76 let initvec: Vec<Revision> = rev_pyiter_collect(py, &initrevs)?; 77 index: PyObject,
78 initrevs: PyObject,
79 stoprev: PyRevision,
80 inclusive: bool
81 ) -> PyResult<AncestorsIterator> {
82 let index = pyindex_to_graph(py, index)?;
83 let initvec: Vec<_> = rev_pyiter_collect(py, &initrevs, &index)?;
77 let ait = VCGAncestorsIterator::new( 84 let ait = VCGAncestorsIterator::new(
78 pyindex_to_graph(py, index)?, 85 index,
79 initvec, 86 initvec.into_iter().map(|r| r.0),
80 stoprev, 87 stoprev.0,
81 inclusive, 88 inclusive,
82 ) 89 )
83 .map_err(|e| GraphError::pynew_from_vcsgraph(py, e))?; 90 .map_err(|e| GraphError::pynew_from_vcsgraph(py, e))?;
84 AncestorsIterator::from_inner(py, ait) 91 AncestorsIterator::from_inner(py, ait)
85 } 92 }
96 } 103 }
97 104
98 py_class!(pub class LazyAncestors |py| { 105 py_class!(pub class LazyAncestors |py| {
99 data inner: RefCell<Box<VCGLazyAncestors<Index>>>; 106 data inner: RefCell<Box<VCGLazyAncestors<Index>>>;
100 107
101 def __contains__(&self, rev: Revision) -> PyResult<bool> { 108 def __contains__(&self, rev: PyRevision) -> PyResult<bool> {
102 self.inner(py) 109 self.inner(py)
103 .borrow_mut() 110 .borrow_mut()
104 .contains(rev) 111 .contains(rev.0)
105 .map_err(|e| GraphError::pynew_from_vcsgraph(py, e)) 112 .map_err(|e| GraphError::pynew_from_vcsgraph(py, e))
106 } 113 }
107 114
108 def __iter__(&self) -> PyResult<AncestorsIterator> { 115 def __iter__(&self) -> PyResult<AncestorsIterator> {
109 AncestorsIterator::from_inner(py, self.inner(py).borrow().iter()) 116 AncestorsIterator::from_inner(py, self.inner(py).borrow().iter())
111 118
112 def __bool__(&self) -> PyResult<bool> { 119 def __bool__(&self) -> PyResult<bool> {
113 Ok(!self.inner(py).borrow().is_empty()) 120 Ok(!self.inner(py).borrow().is_empty())
114 } 121 }
115 122
116 def __new__(_cls, index: PyObject, initrevs: PyObject, stoprev: Revision, 123 def __new__(
117 inclusive: bool) -> PyResult<Self> { 124 _cls,
118 let initvec: Vec<Revision> = rev_pyiter_collect(py, &initrevs)?; 125 index: PyObject,
126 initrevs: PyObject,
127 stoprev: PyRevision,
128 inclusive: bool
129 ) -> PyResult<Self> {
130 let index = pyindex_to_graph(py, index)?;
131 let initvec: Vec<_> = rev_pyiter_collect(py, &initrevs, &index)?;
119 132
120 let lazy = 133 let lazy =
121 VCGLazyAncestors::new(pyindex_to_graph(py, index)?, 134 VCGLazyAncestors::new(
122 initvec, stoprev, inclusive) 135 index,
123 .map_err(|e| GraphError::pynew_from_vcsgraph(py, e))?; 136 initvec.into_iter().map(|r| r.0),
137 stoprev.0,
138 inclusive
139 )
140 .map_err(|e| GraphError::pynew_from_vcsgraph(py, e))?;
124 141
125 Self::create_instance(py, RefCell::new(Box::new(lazy))) 142 Self::create_instance(py, RefCell::new(Box::new(lazy)))
126 } 143 }
127 144
128 }); 145 });
129 146
130 py_class!(pub class MissingAncestors |py| { 147 py_class!(pub class MissingAncestors |py| {
131 data inner: RefCell<Box<CoreMissing<Index>>>; 148 data inner: RefCell<Box<CoreMissing<Index>>>;
149 data index: RefCell<Index>;
132 150
133 def __new__( 151 def __new__(
134 _cls, 152 _cls,
135 index: PyObject, 153 index: PyObject,
136 bases: PyObject 154 bases: PyObject
137 ) 155 )
138 -> PyResult<MissingAncestors> { 156 -> PyResult<MissingAncestors> {
139 let bases_vec: Vec<Revision> = rev_pyiter_collect(py, &bases)?; 157 let index = pyindex_to_graph(py, index)?;
140 let inner = CoreMissing::new(pyindex_to_graph(py, index)?, bases_vec); 158 let bases_vec: Vec<_> = rev_pyiter_collect(py, &bases, &index)?;
141 MissingAncestors::create_instance(py, RefCell::new(Box::new(inner))) 159
160 let inner = CoreMissing::new(index.clone_ref(py), bases_vec);
161 MissingAncestors::create_instance(
162 py,
163 RefCell::new(Box::new(inner)),
164 RefCell::new(index)
165 )
142 } 166 }
143 167
144 def hasbases(&self) -> PyResult<bool> { 168 def hasbases(&self) -> PyResult<bool> {
145 Ok(self.inner(py).borrow().has_bases()) 169 Ok(self.inner(py).borrow().has_bases())
146 } 170 }
147 171
148 def addbases(&self, bases: PyObject) -> PyResult<PyObject> { 172 def addbases(&self, bases: PyObject) -> PyResult<PyObject> {
173 let index = self.index(py).borrow();
174 let bases_vec: Vec<_> = rev_pyiter_collect(py, &bases, &*index)?;
149 let mut inner = self.inner(py).borrow_mut(); 175 let mut inner = self.inner(py).borrow_mut();
150 let bases_vec: Vec<Revision> = rev_pyiter_collect(py, &bases)?;
151 inner.add_bases(bases_vec); 176 inner.add_bases(bases_vec);
152 // cpython doc has examples with PyResult<()> but this gives me 177 // cpython doc has examples with PyResult<()> but this gives me
153 // the trait `cpython::ToPyObject` is not implemented for `()` 178 // the trait `cpython::ToPyObject` is not implemented for `()`
154 // so let's return an explicit None 179 // so let's return an explicit None
155 Ok(py.None()) 180 Ok(py.None())
156 } 181 }
157 182
158 def bases(&self) -> PyResult<HashSet<Revision>> { 183 def bases(&self) -> PyResult<HashSet<PyRevision>> {
159 Ok(self.inner(py).borrow().get_bases().clone()) 184 Ok(
160 } 185 self.inner(py)
161 186 .borrow()
162 def basesheads(&self) -> PyResult<HashSet<Revision>> { 187 .get_bases()
188 .iter()
189 .map(|r| PyRevision(r.0))
190 .collect()
191 )
192 }
193
194 def basesheads(&self) -> PyResult<HashSet<PyRevision>> {
163 let inner = self.inner(py).borrow(); 195 let inner = self.inner(py).borrow();
164 inner.bases_heads().map_err(|e| GraphError::pynew(py, e)) 196 Ok(
197 inner
198 .bases_heads()
199 .map_err(|e| GraphError::pynew(py, e))?
200 .into_iter()
201 .map(|r| PyRevision(r.0))
202 .collect()
203 )
165 } 204 }
166 205
167 def removeancestorsfrom(&self, revs: PyObject) -> PyResult<PyObject> { 206 def removeancestorsfrom(&self, revs: PyObject) -> PyResult<PyObject> {
168 let mut inner = self.inner(py).borrow_mut(); 207 let index = self.index(py).borrow();
169 // this is very lame: we convert to a Rust set, update it in place 208 // this is very lame: we convert to a Rust set, update it in place
170 // and then convert back to Python, only to have Python remove the 209 // and then convert back to Python, only to have Python remove the
171 // excess (thankfully, Python is happy with a list or even an iterator) 210 // excess (thankfully, Python is happy with a list or even an iterator)
172 // Leads to improve this: 211 // Leads to improve this:
173 // - have the CoreMissing instead do something emit revisions to 212 // - have the CoreMissing instead do something emit revisions to
174 // discard 213 // discard
175 // - define a trait for sets of revisions in the core and implement 214 // - define a trait for sets of revisions in the core and implement
176 // it for a Python set rewrapped with the GIL marker 215 // it for a Python set rewrapped with the GIL marker
177 let mut revs_pyset: HashSet<Revision> = rev_pyiter_collect(py, &revs)?; 216 let mut revs_pyset: HashSet<Revision> = rev_pyiter_collect(
217 py, &revs, &*index
218 )?;
219 let mut inner = self.inner(py).borrow_mut();
178 inner.remove_ancestors_from(&mut revs_pyset) 220 inner.remove_ancestors_from(&mut revs_pyset)
179 .map_err(|e| GraphError::pynew(py, e))?; 221 .map_err(|e| GraphError::pynew(py, e))?;
180 222
181 // convert as Python list 223 // convert as Python list
182 let mut remaining_pyint_vec: Vec<PyObject> = Vec::with_capacity( 224 let mut remaining_pyint_vec: Vec<PyObject> = Vec::with_capacity(
183 revs_pyset.len()); 225 revs_pyset.len());
184 for rev in revs_pyset { 226 for rev in revs_pyset {
185 remaining_pyint_vec.push(rev.to_py_object(py).into_object()); 227 remaining_pyint_vec.push(
228 PyRevision(rev.0).to_py_object(py).into_object()
229 );
186 } 230 }
187 let remaining_pylist = PyList::new(py, remaining_pyint_vec.as_slice()); 231 let remaining_pylist = PyList::new(py, remaining_pyint_vec.as_slice());
188 revs.call_method(py, "intersection_update", (remaining_pylist, ), None) 232 revs.call_method(py, "intersection_update", (remaining_pylist, ), None)
189 } 233 }
190 234
191 def missingancestors(&self, revs: PyObject) -> PyResult<PyList> { 235 def missingancestors(&self, revs: PyObject) -> PyResult<PyList> {
236 let index = self.index(py).borrow();
237 let revs_vec: Vec<Revision> = rev_pyiter_collect(py, &revs, &*index)?;
238
192 let mut inner = self.inner(py).borrow_mut(); 239 let mut inner = self.inner(py).borrow_mut();
193 let revs_vec: Vec<Revision> = rev_pyiter_collect(py, &revs)?;
194 let missing_vec = match inner.missing_ancestors(revs_vec) { 240 let missing_vec = match inner.missing_ancestors(revs_vec) {
195 Ok(missing) => missing, 241 Ok(missing) => missing,
196 Err(e) => { 242 Err(e) => {
197 return Err(GraphError::pynew(py, e)); 243 return Err(GraphError::pynew(py, e));
198 } 244 }
199 }; 245 };
200 // convert as Python list 246 // convert as Python list
201 let mut missing_pyint_vec: Vec<PyObject> = Vec::with_capacity( 247 let mut missing_pyint_vec: Vec<PyObject> = Vec::with_capacity(
202 missing_vec.len()); 248 missing_vec.len());
203 for rev in missing_vec { 249 for rev in missing_vec {
204 missing_pyint_vec.push(rev.to_py_object(py).into_object()); 250 missing_pyint_vec.push(
251 PyRevision(rev.0).to_py_object(py).into_object()
252 );
205 } 253 }
206 Ok(PyList::new(py, missing_pyint_vec.as_slice())) 254 Ok(PyList::new(py, missing_pyint_vec.as_slice()))
207 } 255 }
208 }); 256 });
209 257