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
Improve the lltrace feature with the Py_Debug mode #69757
Comments
When we use the Py_Debug flag for the compilation, we can see the Bytecodes and the opcode arguments. Here is a small patch, it will show the label of the opcode in the ceval.c file. |
Here is my patch with the improvement. |
What is lltrace? I never used it :-( Is it documented? Can you give examples of output before/after? We may make your change optional to give the choice of the output. |
Here is a small example of lltrace when you enable it.
|
here is the patch, if you want to test it, just use the REPL and add __ltrace__ = None in the REPL. |
I made some comment on code in review. A thing that worry me that there is zero automated test. In my opinion the minimal should be to test the expected output. A nice to have would be a test for the write_contents function of makeopcodetranslations.py |
Totally agree with you, I am going to check that, because currently, I am going to discuss with Victor about this point. Thanks, Stephane On 07/20, Xavier Combelle wrote:
|
It would be nice to have unit tests and docs :-) For unit test, you can use a script like: def func():
return 1
__ltrace__ = True
func() Run the script with test.support.assert_python_ok(), and check stdout. To skip the test if Python is compiled in released mode, you can use: # Were we compiled --with-pydebug or with #define Py_DEBUG?
Py_DEBUG = hasattr(sys, 'gettotalrefcount') ... |
Sorry, I don't know what is the best place to add new unit tests. I don't know neither what is the best place to document the __ltrace__ feature. Maybe somewhere near: Or maybe: |
Here is a small patch for the __ltrace__ feature. I would like to propose a new test, __ltrace__ should be defined in the globals() dictionary and there is no check on the value associated to this key. Maybe we could defined it as a boolean, True or False. |
Left some comments on Rietveld; mostly documentation-related (and pointed out some typos). I'm also +1 on having to specify a True value rather than just defining the variable at all (i.e. setting __ltrace__ to False is the same as not defining it). If you change this, I'd probably force the value to be an actual bool, i.e. True or False (much like __debug__; except that it's read-only and a syntax error to assign to it). |
Here is a new version of my patch "bpo-25571-4.diff" with the comments of Emanuel Barry. |
Maybe someone who knows this feature better can weigh in, but otherwise LGTM. |
I have to update my patch to python-3.7, I will provide a patch asap. |
@Haypo, you told me there is an alternative to my patch, provided by an other dev. what's the bpo issue for the alternative. |
@matrixise, I'm the author of the alternative in bpo-29400, and I'm finally finding the time to get back into it. I'm going to make a push this week to clean it up; your feedback would be much appreciated! |
I still strongly prefer bpo-29400 over "lltrace", since it would be usable in release mode. |
@gwk no problem for a review of your patch @Haypo as discussed before, maybe we could remove the lltrace feature because this one is never used by the developers. or we could ask on @python-dev ML. If there is a better solution and this one could replace the current |
I suggest to wait until sys.settrace() supports bytecode level tracing, |
I opened issue #91462 and PR #91463, which are similar to some of the patches here. I see Say I implement some new Python feature that relies on a new opcode, but I unknowingly have an off-by-one error that makes the code accidentally skip over a I want something dead simple and transparent and understandable at the c level, as if I had just littered the code with my own |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: