This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Rename Py_SAFE_DOWNCAST
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: CuriousLearner, christian.heimes, loewis, methane, pitrou, shihai1991, skrah, tim.peters, vstinner
Priority: normal Keywords: patch

Created on 2013-11-22 14:24 by pitrou, last changed 2022-04-11 14:57 by admin.

Pull Requests
URL Status Linked Edit
PR 15090 open shihai1991, 2019-08-03 07:31
Messages (14)
msg203764 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-11-22 14:24
Py_SAFE_DOWNCAST's name is a bit misleading: it isn't safe except in debug mode. I propose to rename it to Py_DOWNCAST, so that developers are reminded that the burden of the sanity checks is on them.
msg203766 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-11-22 14:28
+1
msg203767 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-11-22 14:31
I like Py_DOWNCAST name.
msg203782 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-11-22 15:15
Actually, _Py_DOWNCAST may be better (not a public API).
msg203787 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2013-11-22 15:41
-1. The macro name doesn't claim the cast to be safe, i.e. it's not "Py_SAFELY_DOWNCAST", but "safe downcast", i.e. it's an assertion that the cast actually has been verified as being safe.
msg203789 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-11-22 15:45
> -1. The macro name doesn't claim the cast to be safe, i.e. it's not
> "Py_SAFELY_DOWNCAST", but "safe downcast", i.e. it's an assertion that
> the cast actually has been verified as being safe.

It's not an assertion, it's a cast.
Otherwise it should be named Py_ASSERT_SAFE_DOWNCAST (and it should only
do the assertion, not the cast).
msg203790 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2013-11-22 15:51
Goodness.  Name it

_Py_DOWNCAST_AND_IN_DEBUG_MODE_ASSERT_UPCASTING_THE_RESULT_COMPARES_EQUAL_TO_THE_ORIGINAL_ARGUMENT

;-)
msg203855 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-11-22 20:13
Well, I obviously won't fight very hard for this one.

But I would like to point out that APIs with "safe" (not "safely" :-)) in their name usually imply that the API is safe, not that the input has been sanitized beforehand.

For example in the stdlib: pprint.saferepr, string.safe_substitute, xmlrpc.client.SafeTransport. In the C API: Py_TRASHCAN_SAFE_BEGIN.
msg348946 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2019-08-03 07:39
Rename Py_SAFE_DOWNCAST in PR_15090.

In the C API: Py_TRASHCAN_SAFE_BEGIN and Py_TRASHCAN_SAFE_END should be removed or keep it due to compatibility?

In the stdlib: Looks that it's not changed is ok.
msg348951 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2019-08-03 10:55
Why would one not abort() in release mode? If the cast is inexact, the results will usually be so wrong that even on a web server a hard exit is preferable.

I don't think the check costs much time with branch prediction.
msg348952 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-08-03 11:10
I dislike this macro. A cast is either safe or unsafe. If it is safe, (type)var would be better. If it is unsafe, well, it would be better to add a runtime check. No? (I mean better error reporting than abort() pnly in debug mode.)
msg349029 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2019-08-05 06:49
While Py_SAFE_DOWNCAST is not documented, it doesn't start with underscore.
How many 3rd party code are broken by changing the name?
msg349049 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2019-08-05 13:49
It's a big problem, no ability to answer this question :(
msg352460 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2019-09-15 04:11
Looks we have a fast version rhythm. IMHO, If user would be affected and reported, reversing the PR is ok.
History
Date User Action Args
2022-04-11 14:57:53adminsetgithub: 63891
2019-09-15 04:11:33shihai1991setmessages: + msg352460
2019-08-10 19:14:40CuriousLearnersetnosy: + CuriousLearner

versions: + Python 3.7, Python 3.8, Python 3.9, - Python 3.4
2019-08-05 13:49:10shihai1991setmessages: + msg349049
2019-08-05 06:49:27methanesetnosy: + methane
messages: + msg349029
2019-08-03 11:10:28vstinnersetmessages: + msg348952
2019-08-03 10:55:35skrahsetnosy: + skrah
messages: + msg348951
2019-08-03 07:39:31shihai1991setnosy: + shihai1991
messages: + msg348946
2019-08-03 07:31:52shihai1991setkeywords: + patch
stage: patch review
pull_requests: + pull_request14835
2013-11-22 20:13:21pitrousetmessages: + msg203855
2013-11-22 15:51:18tim.peterssetmessages: + msg203790
2013-11-22 15:45:04pitrousetmessages: + msg203789
2013-11-22 15:41:56loewissetnosy: + loewis
messages: + msg203787
2013-11-22 15:15:47pitrousetmessages: + msg203782
2013-11-22 14:31:49vstinnersetmessages: + msg203767
2013-11-22 14:28:17christian.heimessetmessages: + msg203766
2013-11-22 14:24:55pitroucreate