GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-148871: make `LOAD_COMMON_CONSTANT` use immortal stackref borrows by NekoAsakura · Pull Request #149625 · python/cpython Conversation
|
| // Keep in sync with _common_constants in opcode.py | ||
| assert(oparg < NUM_COMMON_CONSTANTS); | ||
| value = PyStackRef_FromPyObjectNew(tstate->interp->common_consts[oparg]); | ||
| value = PyStackRef_DUP(tstate->interp->common_consts[oparg]); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This needs to check the tag.
Could you add PyStackRef_DupImmortal(ref) which would return its argument unchanged and assert that the referenced object is immortal, otherwise it defeats the point of changing PyObject * to _PyStackRef here
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| if (opcode == LOAD_COMMON_CONSTANT) { | ||
| assert(oparg < NUM_COMMON_CONSTANTS); | ||
| return Py_NewRef(_PyInterpreterState_GET()->common_consts[oparg]); | ||
| return Py_NewRef(PyStackRef_AsPyObjectBorrow( |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
| return Py_NewRef(PyStackRef_AsPyObjectBorrow( | |
| return PyStackRef_AsPyObjectBorrow( |
These objects are immortal, you can borrow references to them.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for reviewing :') |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for doing this
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.