[Patches] [ python-Patches-540394 ] Remove PyMalloc_* symbols
noreply@sourceforge.net
noreply@sourceforge.net
Tue, 09 Apr 2002 09:27:21 -0700
Patches item #540394, was opened at 2002-04-06 20:07
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=540394&group_id=5470
Category: Core (C code)
Group: None
Status: Open
Resolution: Accepted
Priority: 5
Submitted By: Neil Schemenauer (nascheme)
>Assigned to: Neil Schemenauer (nascheme)
Summary: Remove PyMalloc_* symbols
Initial Comment:
This patch removes all PyMalloc_* symbols from the
source. obmalloc now implements PyObject_{Malloc,
Realloc, Free}. PyObject_{New,NewVar} allocate using
pymalloc.
I also changed PyObject_Del and PyObject_GC_Del
so that they be used as function designators. Is
changing the signature of PyObject_Del going to cause
any problems? I had to add some extra typecasts when
assigning to tp_free.
Please review and assign back to me.
The next phase would be to cleanup the memory API
usage. Do we want to replace all PyObject_Del calls
with PyObject_Free? PyObject_Del seems to match better
with PyObject_GC_Del.
Oh yes, we also need to change PyMem_{Free, Del, ...} to
use pymalloc's free.
----------------------------------------------------------------------
>Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-09 12:27
Message:
Logged In: YES
user_id=6380
I've not fully read Tim's response in email, but instead
I've reviewed and discussed the patch with Tim.
I think the only thing to which I object at this point is
the removal of the entry point _PyObject_Del. I believe
that for source and binary compatibility with 2.2, that
entry point should remain, with the same meaning, but it
should not be used at all by the core. (Motivation to keep
it: it's the only thing you can reasonably stick in tp_free
that works for 2.2 as well as for 2.3.)
One minor question: there are a bunch of #undefs in
gcmodule.c (e.g. PyObject_GC_Track) that don't seem to make
sense -- at least I cannot find where these would be
#defined any more. Ditto for #indef PyObject_Malloc in
obmalloc.c.
I suggest that you check this thing in, but keeping
_PyObject_Del alive, and we'll take it from there.
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-08 15:18
Message:
Logged In: YES
user_id=6380
(Wouldn't it be more efficient to take this to email
between the three of us?)
> Extensions that *currently* call PyObject_Del have
> its old macro expansion ("_PyObject_Del((PyObject
> *)(op))") buried in them, so getting rid of
> _PyObject_Del is a binary-API incompatibility
> (existing extensions will no longer link without
> recompilation). I personally don't mind that, but
> I run on Windows and "binary compatability" never
> works there across minor releases for other
> reasons, so I don't have any real feel for how
> much people on other platforms value it. As you
> pointed out recently too, binary compatability
> has, in reality, not been the case since 1.5.2
> anyway.
Still, tradition has it that we keep such entry
points around for a long time. I propose that we do
so now, too.
> So that's one for Python-Dev. If we do break
> binary compatibility, I'd be sorely tempted to
> change the "destructor" typedef to say destructors
> take void*. IMO saying they take PyObject* was a
> poor idea, as you almost never have a PyObject*
> when calling one of these guys.
Huh? "destructor" is used to declare tp_dealloc,
which definitely needs a PyObject * (or some
"subclass" of it, like PyIntObject *).
It's also used to declare tp_free, which arguably
shouldn't take a PyObject * (since by the time
tp_free is called, most of the object's contents
have been destroyed by tp_dealloc). So maybe
tp_free (a newcomer in 2.2) should be declared to
take something else, but then the risk is breaking
code that defines a tp_free with the correct
signature.
> That's why PyObject_Del "had to" be a macro, to
> hide the cast to PyObject* almost everyone needs
> because of destructor's "correct" but impractical
> signature. If "destructor" had a practical
> signature, there would have been no temptation to
> use a macro.
I don't understand this at all.
> Note that if the typedef of destructor were so
> changed, you wouldn't have needed new casts in
> tp_free slots. And I'd rather break binary
> compatability than make extension authors add new
> casts.
Nor this.
> Hmm. I'm assigning this to Guido for comment:
> Guido, what are your feelings about binary
> compatibility here? C didn't define free() as
> taking a void* by mistake <wink>.
I want binary compatibility, but I don't understand
your comments very well.
> Back to Neil: I wouldn't bother changing PyObject_Del
> to PyObject_Free. The former isn't in the
> "recommended" minimal API, but neither is it
> discouraged. I expect TMTOWTDI here forever.
I prefer PyObject_Del -- like PyObject_GC_Del, and
like we did in the past. Plus, I like New to match
Del and Malloc to match Free. Since it's
PyObject_New, it should be _Del.
I'm not sure what to say of Neil's patch, except
that I'm glad to be rid of the PyMalloc_XXX family.
I wish we didn't have to change all the places that
used to say _PyObject_Del. Maybe it's best to keep
that name around? The patch would (psychologically)
become a lot smaller. I almost wish that this would
work:
#define PyObject_Del ((destructor)PyObject_Free)
Or maybe it *does* work???
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-04-08 14:47
Message:
Logged In: YES
user_id=6380
I'm looking at this now...
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2002-04-06 21:59
Message:
Logged In: YES
user_id=31435
Extensions that *currently* call PyObject_Del have its old
macro expansion ("_PyObject_Del((PyObject *)(op))") buried
in them, so getting rid of _PyObject_Del is a binary-API
incompatibility (existing extensions will no longer link
without recompilation).
I personally don't mind that, but I run on Windows
and "binary compatability" never works there across minor
releases for other reasons, so I don't have any real feel
for how much people on other platforms value it. As you
pointed out recently too, binary compatability has, in
reality, not been the case since 1.5.2 anyway.
So that's one for Python-Dev. If we do break binary
compatibility, I'd be sorely tempted to change
the "destructor" typedef to say destructors take void*.
IMO saying they take PyObject* was a poor idea, as you
almost never have a PyObject* when calling one of these
guys. That's why PyObject_Del "had to" be a macro, to hide
the cast to PyObject* almost everyone needs because of
destructor's "correct" but impractical signature.
If "destructor" had a practical signature, there would have
been no temptation to use a macro.
Note that if the typedef of destructor were so changed, you
wouldn't have needed new casts in tp_free slots. And I'd
rather break binary compatability than make extension
authors add new casts.
Hmm. I'm assigning this to Guido for comment: Guido, what
are your feelings about binary compatibility here? C
didn't define free() as taking a void* by mistake <wink>.
Back to Neil: I wouldn't bother changing PyObject_Del to
PyObject_Free. The former isn't in the "recommended"
minimal API, but neither is it discouraged. I expect
TMTOWTDI here forever.
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2002-04-06 21:41
Message:
Logged In: YES
user_id=31435
Oops -- I hit "Submit" prematurely. More to come.
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2002-04-06 21:40
Message:
Logged In: YES
user_id=31435
Looks good to me -- thanks!
----------------------------------------------------------------------
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=540394&group_id=5470