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,
>
> ...
> 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 "<stdin>", line 1, in <module>
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 A Meinel wrote: alCall( ); (PyExc_ ValueError, "StaticTuple(...)" alCall( ) throws a New(length) , and StaticTuple_ new_constructor should throw a new_constructor . alCall( ); alCall( ); (PyExc_ ValueError, "StaticTuple(...)" (PyExc_ ValueError, "StaticTuple(...)" alCall( ) throws a alCall raises a "SystemError": BadInternalCall (char *filename, int lineno) PyExc_SystemErr or,
> Matt Nordhoff wrote:
>> Public bug reported:
>
>> Lookie:
>
>> if (size < 0) {
>> PyErr_BadIntern
>> return NULL;
>> }
>
>> if (size < 0 || size > 255) {
>> /* Too big or too small */
>> PyErr_SetString
>> " takes from 0 to 255 items");
>> return NULL;
>> }
>
>> Not only is this redundant, but PyErr_BadIntern
>> TypeError, meaning two different exception classes are used.
>
>> ISTM StaticTuple_New should throw a ValueError, since you do
>> StaticTuple_
>> 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_
>
>> ** Affects: bzr
>> Importance: Undecided
>> Status: New
>
>> ** Description changed:
>
>> Lookie:
>
>> - if (size < 0) {
>> - PyErr_BadIntern
>> - return NULL;
>> - }
>> + if (size < 0) {
>> + PyErr_BadIntern
>> + return NULL;
>> + }
>
>> - if (size < 0 || size > 255) {
>> - /* Too big or too small */
>> - PyErr_SetString
>> - " takes from 0 to 255 items");
>> - return NULL;
>> - }
>> + if (size < 0 || size > 255) {
>> + /* Too big or too small */
>> + PyErr_SetString
>> + " takes from 0 to 255 items");
>> + return NULL;
>> + }
>
>> Not only is this redundant, but PyErr_BadIntern
>> TypeError, meaning two different exception classes are used.
>
> Actually, PyErr_BadIntern
> void
> _PyErr_
> {
> PyErr_Format(
>
> ...
Oh, huh. Thanks for nothing, docs.python. org/c-api/ exceptions. html#PyErr_ BadInternalCall>.
<http://
> 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 New(length) , and StaticTuple_ new_constructor should throw a new_constructor . unicode/ etc.)
>> StaticTuple_
>> - 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_
>
>
> 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/
>
> 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 "<stdin>", line 1, in <module>
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
> =:->
--