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.

Logged In: YES 
user_id=31435

Looks good to me -- thanks!

Logged In: YES 
user_id=31435

Oops -- I hit "Submit" prematurely.  More to come.

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.

Logged In: YES 
user_id=6380

I'm looking at this now...

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???

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.

Logged In: YES 
user_id=31435

Clarifying or just repeating Guido here:

+ Binary compatibility is important.  It's better on Unix 
than it appears <wink> -- while you'll get a warning if you 
run an old 1.5.2 extension with 2.2 today and without 
recompiling, it will almost certainly work anyway.  So in 
the case of macros that expanded to a private API function 
before, that private API function must still exist, but the 
macro needn't expand to that anymore (nor even *be* a macro 
anymore).  _PyObject_Del is a particular problem cuz it's 
even documented in the C API manual -- there simply wasn't 
a public API function before that did the same thing and 
could be used as a function designator.  You're making life 
better for future generations.

+ Casts on tp_free slots are par for the course, 
because "destructor" has an impractical signature.  I'm 
afraid that can't change either, so the casts stay.

+ Fred and I agreed to add PyObject_Del to the "minimal 
recommended API", so, for the next round of this, feel 
wholly righteous in leaving existing PyObject_Del calls 
alone.

If anything's unclear, hit me.

Logged In: YES 
user_id=35752

It might be a day or two before I get to this.

Regarding the type of tp_free, could we change it to be
something like:

  typedef void (*freefunc)(void *);
  ...
  freefunc tp_free;

and leave the type of tp_dealloc alone.  Maybe it's too
late now that 2.2 is out and uses 'destructor'.  I don't
see how this relates to binary compatibility though.
Why does it matter if the function takes a PyObject pointer
or a void pointer?  The worse I see happening is that people
could get warnings when they compile their extension
modules.

Logged In: YES 
user_id=31435

It'll be a day or two before PLabs can get back to Python 
work anyway.

Reassigning to Guido -- I'm not even going to try to 
channel him on backwards compatibility, or the feasibility 
of introducing possible warnings.  If I were you I'd check 
in the patch with the casts in; they can be taken out again 
later if Guido is agreeable.

Logged In: YES 
user_id=6380

The binary compatibility issue is extensions compiled for
2.2 that have references to _PyObject_Del compiled into them
and aren't recompiled for 2.3. I think that should work
(even if they get a warning). To make it work, the
_PyObject_Del entry point must continue to exist.

Back to Neil, I think my instructions are clear enough.

Logged In: YES 
user_id=35752

A modified version of the patch has been commited.