Repository navigation
Reminder: 2.3 should check tp_compare #35349
Description
| BPO | 472523 |
|---|---|
| Nosy | @gvanrossum, @loewis, @rhettinger |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/gvanrossum'
closed_at = <Date 2002-05-31.20:06:33.000>
created_at = <Date 2001-10-18.19:17:24.000>
labels = []
title = 'Reminder: 2.3 should check tp_compare'
updated_at = <Date 2002-05-31.20:06:33.000>
user = 'https://github.com/gvanrossum'bugs.python.org fields:
activity = <Date 2002-05-31.20:06:33.000>
actor = 'gvanrossum'
assignee = 'gvanrossum'
closed = True
closed_date = None
closer = None
components = ['None']
creation = <Date 2001-10-18.19:17:24.000>
creator = 'gvanrossum'
dependencies = []
files = ['3683', '3684']
hgrepos = []
issue_num = 472523
keywords = ['patch']
message_count = 6.0
messages = ['37879', '37880', '37881', '37882', '37883', '37884']
nosy_count = 3.0
nosy_names = ['gvanrossum', 'loewis', 'rhettinger']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue472523'
versions = []Metadata
Metadata
Assignees
Labels
No labels
Activity
gvanrossum commentedon Oct 18, 2001
In 2.3, the outcome of tp_compare should be required to
be -1, 0 or 1; other values should be considered
*illegal*.
(In 2.2, the docs were changed to stress this but for
backwards compatibility this isn't enforced.)
loewis commentedon Mar 9, 2002
Logged In: YES
user_id=21627
Attached is a patch that implements this test, producing a
warning if tp_compare does not follow that restriction.
loewis commentedon Mar 9, 2002
Logged In: YES
user_id=21627
Attached is a patch that implements this test, producing a
warning if tp_compare does not follow that restriction.
rhettinger commentedon May 8, 2002
Logged In: YES
user_id=80475
Here are a few thoughts from my review:
Patch needs to diff from current version, 2.173 of object.c
Patch compiles okay. Code passes regrtest.py without
generating warnings. Code agrees with docs. Scanned
remaining code base and found no other direct calls to
tp_compare in other *.c or *.h files.
If this were an error rather than a warning, convention
would indicate a BadInternalCall since this can only be
generated by an extension writer. I'm wondering if we are
warning the wrong person. An old extension recompiled for
Py2.3 will compile fine but then warn an end-user (not the
extension writer) about a mysterious tp_compare that they
can neither see, nor touch.
In try_3way_compare, the warning is not given for
instances. Are we exempting instances from the warnings?
In try_3way_compare, the warning is given in the
section, "v's comparison" but is omitted in "w's
comparison" which follows.
I think the priority should be raised on this patch because
it needs to be in-place for Py2.3a so that we get the
earliest possible notice if the warning clashes with use in
one of the popular extensions.
loewis commentedon May 8, 2002
Logged In: YES
user_id=21627
I think warning the user is the best thing we can offer;
making it an error would break extensions - users can
disable the warning if they want to. Hopefully, they will
report the problem to the extension author, and hopefully,
the extension author tests his code with the new Python
release, to notice the problem before his users do.
Instances are exempted from the warnings because they
already use the "target result set": -1,0,1 for regular
outcome; -2 for exceptions. The original rationale for this
tightening of the result set is to let -2 indicate
exceptions in future Python releases. Perhaps it should also
check that, if an exception occured, that the result value
is -2.
gvanrossum commentedon May 31, 2002
Logged In: YES
user_id=6380
I have checked in something quite different that implements
the idea here.
It does check that for an error, -2 or -1 is returned; this
can be tightened to require -2 in the future.