D4022: index: don't include nullid in len()
martinvonz added a comment.
In https://phab.mercurial-scm.org/D4022#63500, @yuja wrote:
> > if (PyInt_Check(value)) {
> > long rev = PyInt_AS_LONG(value);
> >
> > - return rev >= -1 && rev < index_length(self); + return rev >=
-1 && rev < index_length(self) - 1;
>
> Fixed this to `index_length(self) + 1`. Maybe you'll want to drop `+ 1` by
> a later patch, but excluding the tip rev is clearly wrong.
Thanks for careful review (as always). Yes, I had meant to clean up the "+1"
and "-1" this patch sprinkled across the code, but thanks for the reminder
because I had forgotten about that.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D4022
To: martinvonz, indygreg, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D4022: index: don't include nullid in len()
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG781b2720d2ac: index: dont include nullid in len()
(authored by martinvonz, committed by ).
CHANGED PRIOR TO COMMIT
https://phab.mercurial-scm.org/D4022?vs=9885=9892#toc
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4022?vs=9885=9892
REVISION DETAIL
https://phab.mercurial-scm.org/D4022
AFFECTED FILES
mercurial/cext/parsers.c
mercurial/cext/revlog.c
mercurial/policy.py
mercurial/pure/parsers.py
mercurial/repoview.py
mercurial/revlog.py
CHANGE DETAILS
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -791,10 +791,8 @@
indexformatv0_unpack = indexformatv0.unpack
class revlogoldindex(list):
-def __len__(self):
-return list.__len__(self) + 1
def __getitem__(self, i):
-if i == -1 or i == len(self) - 1:
+if i == -1 or i == len(self):
return (0, 0, 0, -1, -1, -1, -1, nullid)
return list.__getitem__(self, i)
@@ -1066,11 +1064,11 @@
yield fp
def tip(self):
-return self.node(len(self.index) - 2)
+return self.node(len(self.index) - 1)
def __contains__(self, rev):
return 0 <= rev < len(self)
def __len__(self):
-return len(self.index) - 1
+return len(self.index)
def __iter__(self):
return iter(pycompat.xrange(len(self)))
def revs(self, start=0, stop=None):
@@ -1139,7 +1137,7 @@
i = self.index
p = self._nodepos
if p is None:
-p = len(i) - 2
+p = len(i) - 1
else:
assert p < len(i)
for r in pycompat.xrange(p, -1, -1):
diff --git a/mercurial/repoview.py b/mercurial/repoview.py
--- a/mercurial/repoview.py
+++ b/mercurial/repoview.py
@@ -210,7 +210,7 @@
unfichangelog = unfi.changelog
# bypass call to changelog.method
unfiindex = unfichangelog.index
-unfilen = len(unfiindex) - 1
+unfilen = len(unfiindex)
unfinode = unfiindex[unfilen - 1][7]
revs = filterrevs(unfi, self.filtername, self._visibilityexceptions)
diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -39,20 +39,20 @@
class BaseIndexObject(object):
def __len__(self):
-return self._lgt + len(self._extra) + 1
+return self._lgt + len(self._extra)
def append(self, tup):
self._extra.append(tup)
def _fix_index(self, i):
if not isinstance(i, int):
raise TypeError("expecting int indexes")
-if i < 0 or i >= len(self):
+if i < 0 or i >= len(self) + 1:
raise IndexError
return i
def __getitem__(self, i):
-if i == -1 or i == len(self) - 1:
+if i == -1 or i == len(self):
return (0, 0, 0, -1, -1, -1, -1, nullid)
i = self._fix_index(i)
if i >= self._lgt:
diff --git a/mercurial/policy.py b/mercurial/policy.py
--- a/mercurial/policy.py
+++ b/mercurial/policy.py
@@ -69,7 +69,7 @@
(r'cext', r'bdiff'): 3,
(r'cext', r'mpatch'): 1,
(r'cext', r'osutil'): 4,
-(r'cext', r'parsers'): 6,
+(r'cext', r'parsers'): 7,
}
# map import request to other package or module
diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -77,8 +77,8 @@
static Py_ssize_t index_length(const indexObject *self)
{
if (self->added == NULL)
- return self->length;
- return self->length + PyList_GET_SIZE(self->added);
+ return self->length - 1;
+ return self->length + PyList_GET_SIZE(self->added) - 1;
}
static PyObject *nullentry;
@@ -155,7 +155,7 @@
int comp_len, uncomp_len, base_rev, link_rev, parent_1, parent_2;
const char *c_node_id;
const char *data;
- Py_ssize_t length = index_length(self);
+ Py_ssize_t length = index_length(self) + 1;
PyObject *entry;
if (pos == -1 || pos == length - 1) {
@@ -225,7 +225,7 @@
*/
static const char *index_node(indexObject *self, Py_ssize_t pos)
{
- Py_ssize_t length = index_length(self);
+ Py_ssize_t length = index_length(self) + 1;
const char *data;
if (pos == length - 1 || pos == -1)
@@ -285,7 +285,7 @@
if (node_check(PyTuple_GET_ITEM(obj, 7), ) == -1)
return NULL;
- len = index_length(self);
+ len = index_length(self) + 1;
if (self->added == NULL) {
self->added = PyList_New(0);
@@ -435,7 +435,7 @@
{
PyObject *iter = NULL;
PyObject *iter_item = NULL;
- Py_ssize_t min_idx = index_length(self) + 1;
+ Py_ssize_t min_idx = index_length(self) + 2;
long iter_item_long;
D4022: index: don't include nullid in len()
yuja added a comment.
> if (PyInt_Check(value)) {
> long rev = PyInt_AS_LONG(value);
>
> - return rev >= -1 && rev < index_length(self); + return rev >=
-1 && rev < index_length(self) - 1;
Fixed this to `index_length(self) + 1`. Maybe you'll want to drop `+ 1` by
a later patch, but excluding the tip rev is clearly wrong.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D4022
To: martinvonz, indygreg, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D4022: index: don't include nullid in len()
> if (PyInt_Check(value)) {
> long rev = PyInt_AS_LONG(value);
> - return rev >= -1 && rev < index_length(self);
> + return rev >= -1 && rev < index_length(self) - 1;
Fixed this to `index_length(self) + 1`. Maybe you'll want to drop `+ 1` by
a later patch, but excluding the tip rev is clearly wrong.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D4022: index: don't include nullid in len()
martinvonz updated this revision to Diff 9885.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4022?vs=9781=9885
REVISION DETAIL
https://phab.mercurial-scm.org/D4022
AFFECTED FILES
mercurial/cext/revlog.c
mercurial/pure/parsers.py
mercurial/repoview.py
mercurial/revlog.py
CHANGE DETAILS
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -791,10 +791,8 @@
indexformatv0_unpack = indexformatv0.unpack
class revlogoldindex(list):
-def __len__(self):
-return list.__len__(self) + 1
def __getitem__(self, i):
-if i == -1 or i == len(self) - 1:
+if i == -1 or i == len(self):
return (0, 0, 0, -1, -1, -1, -1, nullid)
return list.__getitem__(self, i)
@@ -1066,11 +1064,11 @@
yield fp
def tip(self):
-return self.node(len(self.index) - 2)
+return self.node(len(self.index) - 1)
def __contains__(self, rev):
return 0 <= rev < len(self)
def __len__(self):
-return len(self.index) - 1
+return len(self.index)
def __iter__(self):
return iter(pycompat.xrange(len(self)))
def revs(self, start=0, stop=None):
@@ -1139,7 +1137,7 @@
i = self.index
p = self._nodepos
if p is None:
-p = len(i) - 2
+p = len(i) - 1
else:
assert p < len(i)
for r in pycompat.xrange(p, -1, -1):
diff --git a/mercurial/repoview.py b/mercurial/repoview.py
--- a/mercurial/repoview.py
+++ b/mercurial/repoview.py
@@ -210,7 +210,7 @@
unfichangelog = unfi.changelog
# bypass call to changelog.method
unfiindex = unfichangelog.index
-unfilen = len(unfiindex) - 1
+unfilen = len(unfiindex)
unfinode = unfiindex[unfilen - 1][7]
revs = filterrevs(unfi, self.filtername, self._visibilityexceptions)
diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -39,20 +39,20 @@
class BaseIndexObject(object):
def __len__(self):
-return self._lgt + len(self._extra) + 1
+return self._lgt + len(self._extra)
def append(self, tup):
self._extra.append(tup)
def _fix_index(self, i):
if not isinstance(i, int):
raise TypeError("expecting int indexes")
-if i < 0 or i >= len(self):
+if i < 0 or i >= len(self) + 1:
raise IndexError
return i
def __getitem__(self, i):
-if i == -1 or i == len(self) - 1:
+if i == -1 or i == len(self):
return (0, 0, 0, -1, -1, -1, -1, nullid)
i = self._fix_index(i)
if i >= self._lgt:
diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -77,8 +77,8 @@
static Py_ssize_t index_length(const indexObject *self)
{
if (self->added == NULL)
- return self->length;
- return self->length + PyList_GET_SIZE(self->added);
+ return self->length - 1;
+ return self->length + PyList_GET_SIZE(self->added) - 1;
}
static PyObject *nullentry;
@@ -155,7 +155,7 @@
int comp_len, uncomp_len, base_rev, link_rev, parent_1, parent_2;
const char *c_node_id;
const char *data;
- Py_ssize_t length = index_length(self);
+ Py_ssize_t length = index_length(self) + 1;
PyObject *entry;
if (pos == -1 || pos == length - 1) {
@@ -225,7 +225,7 @@
*/
static const char *index_node(indexObject *self, Py_ssize_t pos)
{
- Py_ssize_t length = index_length(self);
+ Py_ssize_t length = index_length(self) + 1;
const char *data;
if (pos == length - 1 || pos == -1)
@@ -286,7 +286,7 @@
if (node_check(PyTuple_GET_ITEM(obj, 7), ) == -1)
return NULL;
- len = index_length(self);
+ len = index_length(self) + 1;
if (self->added == NULL) {
self->added = PyList_New(0);
@@ -436,7 +436,7 @@
{
PyObject *iter = NULL;
PyObject *iter_item = NULL;
- Py_ssize_t min_idx = index_length(self) + 1;
+ Py_ssize_t min_idx = index_length(self) + 2;
long iter_item_long;
if (PyList_GET_SIZE(list) != 0) {
@@ -478,7 +478,7 @@
PyObject *reachable = NULL;
PyObject *val;
- Py_ssize_t len = index_length(self) - 1;
+ Py_ssize_t len = index_length(self);
long revnum;
Py_ssize_t k;
Py_ssize_t i;
@@ -630,7 +630,7 @@
PyObject *phaseset = NULL;
PyObject *phasessetlist = NULL;
PyObject *rev = NULL;
- Py_ssize_t len = index_length(self) - 1;
+ Py_ssize_t len = index_length(self);
Py_ssize_t numphase = 0;
Py_ssize_t minrevallphases = 0;
Py_ssize_t minrevphase =
D4022: index: don't include nullid in len()
martinvonz updated this revision to Diff 9781.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4022?vs=9722=9781
REVISION DETAIL
https://phab.mercurial-scm.org/D4022
AFFECTED FILES
mercurial/cext/revlog.c
mercurial/pure/parsers.py
mercurial/repoview.py
mercurial/revlog.py
CHANGE DETAILS
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -790,10 +790,8 @@
indexformatv0_unpack = indexformatv0.unpack
class revlogoldindex(list):
-def __len__(self):
-return list.__len__(self) + 1
def __getitem__(self, i):
-if i == -1 or i == len(self) - 1:
+if i == -1 or i == len(self):
return (0, 0, 0, -1, -1, -1, -1, nullid)
return list.__getitem__(self, i)
@@ -1065,11 +1063,11 @@
yield fp
def tip(self):
-return self.node(len(self.index) - 2)
+return self.node(len(self.index) - 1)
def __contains__(self, rev):
return 0 <= rev < len(self)
def __len__(self):
-return len(self.index) - 1
+return len(self.index)
def __iter__(self):
return iter(pycompat.xrange(len(self)))
def revs(self, start=0, stop=None):
@@ -1138,7 +1136,7 @@
i = self.index
p = self._nodepos
if p is None:
-p = len(i) - 2
+p = len(i) - 1
else:
assert p < len(i)
for r in pycompat.xrange(p, -1, -1):
diff --git a/mercurial/repoview.py b/mercurial/repoview.py
--- a/mercurial/repoview.py
+++ b/mercurial/repoview.py
@@ -210,7 +210,7 @@
unfichangelog = unfi.changelog
# bypass call to changelog.method
unfiindex = unfichangelog.index
-unfilen = len(unfiindex) - 1
+unfilen = len(unfiindex)
unfinode = unfiindex[unfilen - 1][7]
revs = filterrevs(unfi, self.filtername, self._visibilityexceptions)
diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -39,20 +39,20 @@
class BaseIndexObject(object):
def __len__(self):
-return self._lgt + len(self._extra) + 1
+return self._lgt + len(self._extra)
def append(self, tup):
self._extra.append(tup)
def _fix_index(self, i):
if not isinstance(i, int):
raise TypeError("expecting int indexes")
-if i < 0 or i >= len(self):
+if i < 0 or i >= len(self) + 1:
raise IndexError
return i
def __getitem__(self, i):
-if i == -1 or i == len(self) - 1:
+if i == -1 or i == len(self):
return (0, 0, 0, -1, -1, -1, -1, nullid)
i = self._fix_index(i)
if i >= self._lgt:
diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -77,8 +77,8 @@
static Py_ssize_t index_length(const indexObject *self)
{
if (self->added == NULL)
- return self->length;
- return self->length + PyList_GET_SIZE(self->added);
+ return self->length - 1;
+ return self->length + PyList_GET_SIZE(self->added) - 1;
}
static PyObject *nullentry;
@@ -155,7 +155,7 @@
int comp_len, uncomp_len, base_rev, link_rev, parent_1, parent_2;
const char *c_node_id;
const char *data;
- Py_ssize_t length = index_length(self);
+ Py_ssize_t length = index_length(self) + 1;
PyObject *entry;
if (pos == -1 || pos == length - 1) {
@@ -225,7 +225,7 @@
*/
static const char *index_node(indexObject *self, Py_ssize_t pos)
{
- Py_ssize_t length = index_length(self);
+ Py_ssize_t length = index_length(self) + 1;
const char *data;
if (pos == length - 1 || pos == INT_MAX)
@@ -285,7 +285,7 @@
if (node_check(PyTuple_GET_ITEM(obj, 7), , ) == -1)
return NULL;
- len = index_length(self);
+ len = index_length(self) + 1;
if (self->added == NULL) {
self->added = PyList_New(0);
@@ -435,7 +435,7 @@
{
PyObject *iter = NULL;
PyObject *iter_item = NULL;
- Py_ssize_t min_idx = index_length(self) + 1;
+ Py_ssize_t min_idx = index_length(self) + 2;
long iter_item_long;
if (PyList_GET_SIZE(list) != 0) {
@@ -477,7 +477,7 @@
PyObject *reachable = NULL;
PyObject *val;
- Py_ssize_t len = index_length(self) - 1;
+ Py_ssize_t len = index_length(self);
long revnum;
Py_ssize_t k;
Py_ssize_t i;
@@ -629,7 +629,7 @@
PyObject *phaseset = NULL;
PyObject *phasessetlist = NULL;
PyObject *rev = NULL;
- Py_ssize_t len = index_length(self) - 1;
+ Py_ssize_t len = index_length(self);
Py_ssize_t numphase = 0;
Py_ssize_t minrevallphases = 0;
Py_ssize_t
D4022: index: don't include nullid in len()
martinvonz created this revision.
Herald added a reviewer: indygreg.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
I suspect the reason the nullid is in the index in the last position
is that it lets index[i] for regular revision number, even when index
was just a regular Python list. An alternative solution would have
been to reserve revision number 0 for the null revision. I don't know
why that wasn't done. Now that we have classes backing the index, we
can easily make index[-1] get the nullid without having to put it last
in the list and including it in the len().
This patch just hides the nullid -- it will still be accessible at
index[len(index)].
I realize that this will be annoying when checking out across this
commit for debugging (including bisection).
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D4022
AFFECTED FILES
mercurial/cext/revlog.c
mercurial/pure/parsers.py
mercurial/repoview.py
mercurial/revlog.py
CHANGE DETAILS
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -790,10 +790,8 @@
indexformatv0_unpack = indexformatv0.unpack
class revlogoldindex(list):
-def __len__(self):
-return list.__len__(self) + 1
def __getitem__(self, i):
-if i == -1 or i == len(self) - 1:
+if i == -1 or i == len(self):
return (0, 0, 0, -1, -1, -1, -1, nullid)
return list.__getitem__(self, i)
@@ -1065,11 +1063,11 @@
yield fp
def tip(self):
-return self.node(len(self.index) - 2)
+return self.node(len(self.index) - 1)
def __contains__(self, rev):
return 0 <= rev < len(self)
def __len__(self):
-return len(self.index) - 1
+return len(self.index)
def __iter__(self):
return iter(xrange(len(self)))
def revs(self, start=0, stop=None):
@@ -1138,7 +1136,7 @@
i = self.index
p = self._nodepos
if p is None:
-p = len(i) - 2
+p = len(i) - 1
else:
assert p < len(i)
for r in xrange(p, -1, -1):
diff --git a/mercurial/repoview.py b/mercurial/repoview.py
--- a/mercurial/repoview.py
+++ b/mercurial/repoview.py
@@ -210,7 +210,7 @@
unfichangelog = unfi.changelog
# bypass call to changelog.method
unfiindex = unfichangelog.index
-unfilen = len(unfiindex) - 1
+unfilen = len(unfiindex)
unfinode = unfiindex[unfilen - 1][7]
revs = filterrevs(unfi, self.filtername, self._visibilityexceptions)
diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -39,20 +39,20 @@
class BaseIndexObject(object):
def __len__(self):
-return self._lgt + len(self._extra) + 1
+return self._lgt + len(self._extra)
def append(self, tup):
self._extra.append(tup)
def _fix_index(self, i):
if not isinstance(i, int):
raise TypeError("expecting int indexes")
-if i < 0 or i >= len(self):
+if i < 0 or i >= len(self) + 1:
raise IndexError
return i
def __getitem__(self, i):
-if i == -1 or i == len(self) - 1:
+if i == -1 or i == len(self):
return (0, 0, 0, -1, -1, -1, -1, nullid)
i = self._fix_index(i)
if i >= self._lgt:
diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -77,8 +77,8 @@
static Py_ssize_t index_length(const indexObject *self)
{
if (self->added == NULL)
- return self->length;
- return self->length + PyList_GET_SIZE(self->added);
+ return self->length - 1;
+ return self->length + PyList_GET_SIZE(self->added) - 1;
}
static PyObject *nullentry;
@@ -155,7 +155,7 @@
int comp_len, uncomp_len, base_rev, link_rev, parent_1, parent_2;
const char *c_node_id;
const char *data;
- Py_ssize_t length = index_length(self);
+ Py_ssize_t length = index_length(self) + 1;
PyObject *entry;
if (pos == -1 || pos == length - 1) {
@@ -225,7 +225,7 @@
*/
static const char *index_node(indexObject *self, Py_ssize_t pos)
{
- Py_ssize_t length = index_length(self);
+ Py_ssize_t length = index_length(self) + 1;
const char *data;
if (pos == length - 1 || pos == INT_MAX)
@@ -285,7 +285,7 @@
if (node_check(PyTuple_GET_ITEM(obj, 7), , ) == -1)
return NULL;
- len = index_length(self);
+ len = index_length(self) + 1;
if (self->added == NULL) {
self->added = PyList_New(0);
@@ -435,7 +435,7 @@
{
PyObject *iter = NULL;
PyObject *iter_item = NULL;
- Py_ssize_t min_idx
