-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
Clarify which "identifiers" in the C API are macros #93733
Comments
+1 In my opinion, even better would be the requirement that the C API functions, macros, and so on -- that are described in the documentation -- also have a corresponding link to the file in which they are defined. Of course, there are other ways to find where something is defined, but it could be very helpful if it were a part of the documentation. The line number shouldn't be factored in since it would very quickly become invalidated through PRs that don't even touch these API items. The link to the file should only be changed whenever one wishes to add/remove an API item from a file (perhaps to relocate the definition to a different file). I could be missing other considerations though. |
The documentation is writting using "sphinx".
The annotation "c:macro" is described as:
Is step one to extend Sphinx with a new annotation "c:macrofunction" ? |
Some of these (maybe all?) are deliberately left underspecified so we can switch if they're macros or functions. |
In that case, I think, should state that fact explicitly. Btw - I see it as a positive that there is an ongoing effort to convert macros into static, inline functions: |
If we were to specify whether it is a macro or a function, is there something documented or about the status quo that would dictate that it must stay that way for x amount of time? I think we mainly guarantee behavior (via the docs) and very rarely the implementation, no? If not, I think the net benefit of distinguishing them on the documentation is positive, no? |
There are no guarantees beyond the limited API and stable ABI. Maybe @vstinner can give his opinion. |
Documentation patches are welcomed ;-) |
Before I make a patch, I need to know, what kind of change is wanted. Take the C macro
When I look at the documentation for Sphinx [2], I see:
This suggests that I should change The Right™ solution would be to add an The faster way is to add an explanation in the text:
Is this phrasing acceptable - or is it better to avoid the word "currently". [1] https://github.com/python/cpython/blob/main/Doc/c-api/import.rst [2] https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html |
I suggest adding "Function added as a macro." If it becomes a regular tomorrow, this sentence can be removed. cc @encukou |
well if you ask me... |
Well, if you ask me, macros should be converted to functions: https://peps.python.org/pep-0670/ :-) I converted a few macros to static inline functions, but not to regular functions yet. As said in PEP 670, there is a question about the performance cost of such change. Since there are very few users requesting to call functions currently implemented as macros or static inline functions, there is active work on converting these to regular functions. It's more done on a case by case basis. Well, in general, IMO the overhead is not significan or can be ignored. |
@soegaard Will you be making a PR? FWIW, if you wanted to just add to the text, I would suggest ``Function-like macro'' or going with Victor's suggestion. Following Petr's suggestion, it may be worth creating a separate issue to see how other maintainers feel about creating actual functions in addition to the existing function-like macros in the C API to facilitate embedding Python. I wouldn't try to address this in your PR. |
I'll make some PR, but first let me know if the example below looks okay. I added the phrase "Function-like macro" to I used your phrase "Function-like macro" since that's the term used in the C specification. The rendered documentation looks like this: Ideally I would have liked no line between "Return value: New reference." and "Function-like macro.", If this looks okay, I'll look at the other files and make a PR. The diff is here: |
@soegaard Thanks for getting back to me and for posting screenshots of your suggestions. I agree with you that the ``Function-like macro'' info is best presented along with the signature and return value. If I had to choose between the two screenshots, I would go with the second. @AA-Turner I was wondering if I could get your input on how to best present the ``Function-like macro'' info? |
If this is documented, we should agree when this can be changed (function become a macro, and when a macro can become a function), and also document that in the devguide. (AFAICS, currently, such a change can't happen in a point release due to ABI stability needs; but it can happen in a minor release – 3.x.) AFAICS, static inline functions have the same issue as macros for Will users be able to tell the difference between an actual exported function, and a macro-function for which no one added the note yet? Or will there be some mechanism to ensure notes are always added where relevant? (FWIW, I've been tackling similar issues in the Limited API: all of the functions there are actually exported, with tests to ensure they stay that way.) |
Thanks @encukou. All very good points.
I personally would prefer seeing Static inline function whenever it is a static inline function and Function-like macro whenever it is implemented as a macro. One concern is that exported is not a term used in the C Standard. Another is that the semantics differ and so there could be benefit to being a little bit more specific. However, I also like Not exported so I am not sure what is best.
My intuition says there should be some kind of mechanism to ensure that notes are always added where relevant though no obvious solution comes to mind. Of course, updating the devguide will help and over time maintainers will become accustomed to the new procedure but it is not foolproof. Could you maybe point me to an example of how you are ''ensur[ing] they stay that way"? It might provide some ideas. Thanks again! |
AFAIK, the C standard doesn't cover dynamically linked libraries at all. The note would definitely need to be linked to an explanation, anyway.
The current test does
My pessimistic view is that until it is made foolproof, it's better for you as the user to try loading the function from libpython rather than look in the docs. |
@encukou wrote:
I have added a note to the document. When the bindings are done, I'll report back. |
Documentation
The "Python/C API Reference Manual" is an excellent resource but could be even better
if C macros were marked as such.
For a programmer that writes directly in C it doesn't matter which identifiers are real C functions
and which are C macros. However, when Python is used through
libpython
only theC functions are available. This is the case for languages embedding Python through
ffilib
.As an example: Consider the C macro
PyImport_ImportModuleEx
and the C functionPyImport_ImportModuleLevel
. They are documented in a way that makes it impossibleto guess that one is a C macro.
A look in "Python.c" (or friends
import.h
here) will reveal thatPyImport_ImportModuleEx
is a C macro.But it would be a quality of life-improvement, if a simple "C Macro" were added below the
signature of C macros.
This "C macro" annotation could have the same style as the "Return value is a new reference" annotation.
(maybe with a different color).
The text was updated successfully, but these errors were encountered: