John A Meinel wrote: > Matt Nordhoff wrote: >> Public bug reported: > >> Lookie: > >> if (size < 0) { >> PyErr_BadInternalCall(); >> return NULL; >> } > >> if (size < 0 || size > 255) { >> /* Too big or too small */ >> PyErr_SetString(PyExc_ValueError, "StaticTuple(...)" >> " takes from 0 to 255 items"); >> return NULL; >> } > >> Not only is this redundant, but PyErr_BadInternalCall() throws a >> TypeError, meaning two different exception classes are used. > >> ISTM StaticTuple_New should throw a ValueError, since you do >> StaticTuple_New(length), and StaticTuple_new_constructor should throw a >> TypeError, since you do StaticTuple(foo, bar, baz) (currently it lets >> StaticTuple_New handle throw the error). > >> FYI, the Python version of StaticTuple throws a ValueError when it's >> either <0 or >255. IMO, that should be a TypeError, same as >> StaticTuple_new_constructor. > >> ** Affects: bzr >> Importance: Undecided >> Status: New > >> ** Description changed: > >> Lookie: > >> - if (size < 0) { >> - PyErr_BadInternalCall(); >> - return NULL; >> - } >> + if (size < 0) { >> + PyErr_BadInternalCall(); >> + return NULL; >> + } > >> - if (size < 0 || size > 255) { >> - /* Too big or too small */ >> - PyErr_SetString(PyExc_ValueError, "StaticTuple(...)" >> - " takes from 0 to 255 items"); >> - return NULL; >> - } >> + if (size < 0 || size > 255) { >> + /* Too big or too small */ >> + PyErr_SetString(PyExc_ValueError, "StaticTuple(...)" >> + " takes from 0 to 255 items"); >> + return NULL; >> + } > >> Not only is this redundant, but PyErr_BadInternalCall() throws a >> TypeError, meaning two different exception classes are used. > > Actually, PyErr_BadInternalCall raises a "SystemError": > void > _PyErr_BadInternalCall(char *filename, int lineno) > { > PyErr_Format(PyExc_SystemError, > > ... Oh, huh. Thanks for nothing, . > My specific point was that passing size < 0 is a sign that something is > very wrong. Which is different from just passing a value that is too large. Hm, that makes sense. > The redundant "<0" is probably because I used to check for size < 1, and > just didn't remove that check when I started allowing size 0 StaticTuples. Oh, okay. >> ISTM StaticTuple_New should throw a ValueError, since you do >> StaticTuple_New(length), and StaticTuple_new_constructor should throw a >> - TypeError, since you do StaticTuple(foo, bar, baz). >> + TypeError, since you do StaticTuple(foo, bar, baz) (currently it lets >> + StaticTuple_New handle throw the error). > >> FYI, the Python version of StaticTuple throws a ValueError when it's >> either <0 or >255. IMO, that should be a TypeError, same as >> StaticTuple_new_constructor. > > > I'm happy to have them the same. Regardless of what we make this, it > isn't something you should ever be catching, so the specific exception > isn't particularly meaningful. > > It is arguable that "TypeError" should be reserved for when the *type* > of the argument passed is invalid. (Such as passing a dict rather than a > string/unicode/etc.) > > Everything else would be considered a ValueError. (Having a tuple that > holds only strings but is 266 entries long is closer to a ValueError > than a TypeError.) While it's a little odd (IMO), Python likes to use TypeError when you pass the wrong number of arguments to a function: >>> def f(): pass ... >>> f(1) Traceback (most recent call last): File "", line 1, in TypeError: f() takes no arguments (1 given) If we imagine StaticTuple as being defined like "def __new__(cls, arg1=None, arg2=None, arg3=None, ..., arg255=None)", and someone passed 256 arguments, Python would raise a TypeError. > John > =:-> --