-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-132775: Add _PyObject_GetXIDataWithFallback() #133482
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,7 +131,23 @@ PyAPI_FUNC(void) _PyXIData_Clear(PyInterpreterState *, _PyXIData_t *); | |
|
||
/* getting cross-interpreter data */ | ||
|
||
typedef int (*xidatafunc)(PyThreadState *tstate, PyObject *, _PyXIData_t *); | ||
typedef int xidata_fallback_t; | ||
#define _PyXIDATA_XIDATA_ONLY (0) | ||
#define _PyXIDATA_FULL_FALLBACK (1) | ||
|
||
// Technically, we don't need two different function types; | ||
// we could go with just the fallback one. However, only container | ||
// types like tuple need it, so always having the extra arg would be | ||
// a bit unfortunate. It's also nice to be able to clearly distinguish | ||
// between types that might call _PyObject_GetXIData() and those that won't. | ||
// | ||
typedef int (*xidatafunc)(PyThreadState *, PyObject *, _PyXIData_t *); | ||
typedef int (*xidatafbfunc)( | ||
PyThreadState *, PyObject *, xidata_fallback_t, _PyXIData_t *); | ||
typedef struct { | ||
xidatafunc basic; | ||
xidatafbfunc fallback; | ||
} _PyXIData_getdata_t; | ||
Comment on lines
+147
to
+150
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the layout imply that it would accept two functions in the future? If exclusive, is there any reason why it cannot be a void pointer (or xidatafunc) with a flag? (The C compilers could verify E: No change request from me as long as there are only two options. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I tried both and went with this because I don't expect other options. This is internal API so I wasn't going to worry about it too much for now. That said, maybe it would be better to take the void * approach now. I'll think about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm inclined to leave it the way it is, since I anticipate only those two options will be needed and it's nice to keep the signature explicit (instead of |
||
|
||
PyAPI_FUNC(PyObject *) _PyXIData_GetNotShareableErrorType(PyThreadState *); | ||
PyAPI_FUNC(void) _PyXIData_SetNotShareableError(PyThreadState *, const char *); | ||
|
@@ -140,16 +156,21 @@ PyAPI_FUNC(void) _PyXIData_FormatNotShareableError( | |
const char *, | ||
...); | ||
|
||
PyAPI_FUNC(xidatafunc) _PyXIData_Lookup( | ||
PyAPI_FUNC(_PyXIData_getdata_t) _PyXIData_Lookup( | ||
PyThreadState *, | ||
PyObject *); | ||
PyAPI_FUNC(int) _PyObject_CheckXIData( | ||
PyThreadState *, | ||
PyObject *); | ||
|
||
PyAPI_FUNC(int) _PyObject_GetXIDataNoFallback( | ||
PyThreadState *, | ||
PyObject *, | ||
_PyXIData_t *); | ||
PyAPI_FUNC(int) _PyObject_GetXIData( | ||
PyThreadState *, | ||
PyObject *, | ||
xidata_fallback_t, | ||
_PyXIData_t *); | ||
|
||
// _PyObject_GetXIData() for bytes | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe flags like 0b111?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these with the idea that there is conceptual room for other options and the actual values aren't so important. However, they don't need to be unioned, so I'm not sure there's much value in using another literal form.