| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
The C API part https://docs.python.org/3.7/c-api/init.html?highlight=pyeval_settrace#profiling-and-tracing also needs editing.
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request. |
Sorry, something went wrong.
|
I have made the requested changes; please review again |
Sorry, something went wrong.
|
Thanks for making the requested changes! @zhangyangyu: please review the changes made to this pull request. |
Sorry, something went wrong.
| | :const:`PyTrace_C_RETURN` | Function object being called. | | ||
| +------------------------------+--------------------------------------+ | ||
|
|
||
| Any trace function registered using :c:func:`PyEval_SetTrace` will not recieve |
There was a problem hiding this comment.
/s/recieve/receive
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks! Corrected in 18b88ec
Sorry, something went wrong.
| Any trace function registered using :c:func:`PyEval_SetTrace` will not receive | ||
| :const:`PyTrace_C_CALL`, :const:`PyTrace_C_EXCEPTION`, :const:`PyTrace_C_RETURN` | ||
| or :const:`PyTrace_LINE` as a value for the *what* parameter. | ||
|
|
There was a problem hiding this comment.
Why line event is also here?
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry, I misread that part.
Sorry, something went wrong.
| Set the tracing function to *func*. This is similar to | ||
| :c:func:`PyEval_SetProfile`, except the tracing function does receive line-number | ||
| events. | ||
| events or any event related to C function objects being called. |
There was a problem hiding this comment.
It is does receive, so I think it is wrong here. Maybe just declare it does not receive PyTrace_C_CALL/RETURN/EXCEPTION here, so don't have to write the previous added block.
Sorry, something went wrong.
|
|
||
| Profile functions should have three arguments: *frame*, *event*, and | ||
| *arg*. *frame* is the current stack frame. *event* is a string: ``'call'``, | ||
| ``'line'``, ``'return'``, ``'exception'``, ``'c_call'``, ``'c_return'``, or |
There was a problem hiding this comment.
line will be received here?
Sorry, something went wrong.
|
|
||
| ``'call'`` | ||
| A function is called (or some other code block entered). The | ||
| global trace function is called; *arg* is ``None``; the return value |
There was a problem hiding this comment.
Although the previous doc says profile function is called similiarly to trace function, but from the implementation, there is no concept like global trace function or local trace function. And at least we call it profile function here not trace function.
Sorry, something went wrong.
|
@zhangyangyu Thank you very much for the comments. I have addressed your feedback in a9f381f and 3887c6c. |
Sorry, something went wrong.
| profile function is called; *arg* is ``None``. | ||
|
|
||
| ``'return'`` | ||
| A function (or other code block) is about to return. The local trace |
There was a problem hiding this comment.
Also here and the following return, exception part about the global/local and name.
Sorry, something went wrong.
| ``'c_exception'`` | ||
| A C function has raised an exception. *arg* is the C function object. | ||
|
|
||
| ``'opcode'`` |
There was a problem hiding this comment.
opcode events won't be triggered for profile functions, also note the above list needs also be modified.
Sorry, something went wrong.
| Any trace function registered using :c:func:`PyEval_SetTrace` will not receive | ||
| :const:`PyTrace_C_CALL`, :const:`PyTrace_C_EXCEPTION` or :const:`PyTrace_C_RETURN` | ||
| as a value for the *what* parameter. | ||
|
|
There was a problem hiding this comment.
I prefer to combine this section with the specified PyEval_SetTrace part.
Sorry, something went wrong.
|
@zhangyangyu Upon inspection and testing I think that profile functions cannot receive "exception" events as well. Can you confirm this? |
Sorry, something went wrong.
|
Yes. But profile functions receive even PyTrace_C_EXCEPTION, why not Python exception events? |
Sorry, something went wrong.
|
From the unitest test_sys_setprofile, exception events should never be received by profile functions, so seems it's by design. |
Sorry, something went wrong.
…not being received.
|
PyEval_SetProfile should also be updated that not only line number events but also Python exception events. |
Sorry, something went wrong.
|
I think the following paragraph need some modification, something like "... but it is called with different events, for example ..."
And your NEWS entry needs updated too. It's not only about C events now. BTW, we need a new issue to complete the C part about PyTrace_OPCODE. |
Sorry, something went wrong.
|
Thanks @pablogsal for the PR, and @zhangyangyu for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6. |
Sorry, something went wrong.
|
Sorry, @pablogsal and @zhangyangyu, I could not cleanly backport this to 3.6 due to a conflict. |
Sorry, something went wrong.
|
Sorry, @pablogsal and @zhangyangyu, I could not cleanly backport this to 2.7 due to a conflict. |
Sorry, something went wrong.
…ofile (pythonGH-4056). (cherry picked from commit 131fd7f)
|
GH-5298 is a backport of this pull request to the 3.6 branch. |
Sorry, something went wrong.
…ofile (pythonGH-4056). (cherry picked from commit 131fd7f)
|
GH-5299 is a backport of this pull request to the 2.7 branch. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
The docs for sys.settrace mention that:
But in the code for ceval.c the only point where call_trace is invoked with PyTrace_C_CALL or PyTrace_C_RETURN is under the C_TRACE macro. In this macro this line prevents any function set up using sys.settrace to call call_trace with the mentioned arguments:
Notice that from the code of PyEval_SetTrace and PyEval_SetProfile, only the later sets tstate->c_profilefunc and therefore only functions installed using sys.setprofile will recieve a c_call for the event:
This PR updates the documentation clarifying the real behaviour of both functions.
https://bugs.python.org/issue17799