bpo-43973: Use Py_TPFLAGS_IMMUTABLETYPE to check for class assignments by erlend-aasland · Pull Request #25714 · python/cpython (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation11 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

erlend-aasland

@erlend-aasland erlend-aasland changed the titleUse Py_TPFLAGS_IMMUTABLETYPE to check for class assignments bpo-43973: Use Py_TPFLAGS_IMMUTABLETYPE to check for class assignments

Apr 29, 2021

@erlend-aasland

cc. @vstinner

See msg392291: Can we get rid of the huge comment preceding the if statement now? Or at least reduce it considerably.

@erlend-aasland

AFAICS, this does not require a news item, but I might be wrong.

@shreyanavigyan

Doesn't this also require a change?

if (compatible_for_assignment(oldto, newto, "__class__")) {
if (newto->tp_flags & Py_TPFLAGS_HEAPTYPE) {
Py_INCREF(newto);
}
Py_SET_TYPE(self, newto);
if (oldto->tp_flags & Py_TPFLAGS_HEAPTYPE)
Py_DECREF(oldto);
return 0;
}
else {
return -1;
}

Suggested :-

 if (compatible_for_assignment(oldto, newto, "__class__")) { 
     if (!(newto->tp_flags & Py_TPFLAGS_IMMUTABLETYPE)) { 
         Py_INCREF(newto); 
     } 
     Py_SET_TYPE(self, newto); 
     if (!(oldto->tp_flags & Py_TPFLAGS_IMMUTABLETYPE))
         Py_DECREF(oldto); 
     return 0; 
 } 
 else { 
     return -1; 
 }

@erlend-aasland

Doesn't this also require a change?

AFAICT, no. They adjust the ref. count if heap types are used:

If the old class was a heap type, decrement its ref. count. If the new class is a heap type, acquire a strong reference to it (increment the ref. count).

vstinner

@erlend-aasland

@vstinner

Merged, thanks for the fix.

@pitrou

The comment above the check should have been fixed to reflect the new semantics, no?

Also, is the explicit check for PyModule_Type still needed?

@erlend-aasland

The comment above the check should have been fixed to reflect the new semantics, no?

Yes, and I believe it can be considerably reduced.

Also, is the explicit check for PyModule_Type still needed?

I don't know. See bpo-24912.