- Notifications
You must be signed in to change notification settings - Fork748
Remove non-existent PInvoke functions#1205
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
src/runtime/runtime.cs Outdated
| @@ -1793,7 +1784,7 @@ internal static IntPtr PyType_GenericAlloc(IntPtr type, long n) | |||
| [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] | |||
| internal static extern IntPtr _PyObject_GetDictPtr(IntPtr obj); | |||
| [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] | |||
| [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl, EntryPoint = "_PyObject_GC_New")] | |||
| internal static extern IntPtr PyObject_GC_New(IntPtr tp); | |||
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.
Is this one used anywhere?
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.
None.
Its implemetantion is
#definePyObject_GC_New(type,typeobj) \ ( (type *) _PyObject_GC_New(typeobj) )
Just compare to thePyObject_CheckBuffer, it's easy to make it be a PInvoke function by specific the entry point.
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.
If it's not used, just remove it.
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.
Well, it's a not big deal though, but what's bad for declaring a valid api?
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.
If we don't use it, it should not be included. The fewer functions we refer to, the easier it will be to keep compatible with different Python versions from the same DLL.
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.
That's a point, although I don't think this API will be deleted in the future.
Removed it.
codecov-commenter commentedAug 24, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## master #1205 +/- ##======================================= Coverage 86.25% 86.25% ======================================= Files 1 1 Lines 291 291 ======================================= Hits 251 251 Misses 40 40
Flags with carried forward coverage won't be shown.Click here to find out more. Continue to review full report at Codecov.
|
What does this implement/fix? Explain your changes.
Deprecated Python2 API:
Mere macros on C side:
_PyObject_GC_New)Check method:
https://gist.github.com/amos402/fb41571856bc77a2bada8056e429b312
Does this close any currently open issues?
...
Any other comments?
...
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG