-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Kill ctx->h_None & co. #226
Comments
Some thoughts / questions:
|
I thought of it as well, but I don't like it too much. From a high-level point of view, what it returns is a handle to But I still prefer Another possibility is to introduce another namespace instead of just using
yes. Basically, we would autogenerate functions like this, and they would be treated like all the other functions: HPy ctx_None(HPyContext *ctx) {
Py_INCREF(Py_None);
return _py2h(Py_None);
} |
I correct myself here. There is at least another reasonable thing to do with if (HPy_Is(ctx, h, ctx->h_None))
... This idiom becomes unnecessarily complicated with the HPy h_None = HPy_None(ctx);
if (HPy_Is(ctx, h, h_None))
...
HPy_Close(h_None); Possible solutions:
Can we think of other reasonably common operations which are easy to do with the current |
Won't OPs point no. 2 apply to |
Returning a handle which doesn't need to be closed is equivalent to returning a borrowed reference in CPython, and we are explicitly trying to avoid that. As a general rule, all the handles which are returned by a function need to be explicitly closed. The fact that you didn't find this rule anywhere it's our fault: we do have some API design principle and guidelines, but we never wrote them nicely in the docs. But e.g., you can find this precise statement in this note: |
Additional idea from #250: We could define a list of constants via an enum or defines and then have one function And perhaps a separate enum for constants. I'm not sure I particularly like these solutions, but they are ideas that might eventually mutate into something useful. |
Isn't enums vs lots of generated methods orthogonal to the usability issue with My 2c is that
Anything where you'd just immediately forward the handle to some other API function, like
making this into
This can be annoying, but it kind of feels like the proper way. One more example re consistency:
after some refactoring, someone may want to put 42 into the list, so they change the
|
I don't like this idea at all. It's much easier and more consistent to use e.g. The only "advantage" of |
yes, I agree.
that's a very good point, but note that CPython has the same problem: you cannot
[cut]
Good point. I think it will be interesting to look at
also a good point. Thankfully, the debug mode will catch this kind of errors easily, at least. |
Recently we discussed a potential refactoring of
HPyContext
, and one of the problems was what to do with theh_*
fields.I think we can/should just kill them and have normal functions instead. Some points:
HPy_Dup
.dup
or not (they do!)ctx
upon the first access, to check whether it's already initialized, etc.Let's consider some alternatives:
Personally, I prefer
HPy_None()
thanHPy_GetNone()
. The only drawback I can think it's that it might be a bit confusing for people used to the old API, wherePy_None
is a variable and not a function. However, once you are in the righr "HPy mindset" you know that you have to pass thectx
everywhere, so I think it's pretty natural to turn it into a function.The text was updated successfully, but these errors were encountered: