-
-
Notifications
You must be signed in to change notification settings - Fork 440
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
C tracer: lookups fail in "should_trace_cache" #374
Comments
@sbehnel Do you have a test case that demonstrates the problem? |
Original comment by sbehnel (Bitbucket: sbehnel, GitHub: Unknown) It currently makes Cython code tracing fail, but I don't have a self-contained example. My guess is that any plugin that uses the "dynamic_source_filename" feature would simply fail to work. |
by "Cython code tracing fail," do you mean a Cython plugin for coverage? Either way, I'd like to know more. |
Original comment by sbehnel (Bitbucket: sbehnel, GitHub: Unknown) Yes. You may remember our discussion about this back in February. The plugin is now available here: https://github.com/cython/cython/blob/master/Cython/Coverage.py Not released yet, but closing up. |
Original comment by Matthew Seal (Bitbucket: mseal, GitHub: mseal) I have a fairly complicated Cython repository I work with, and I needed to generate coverage numbers for the repository. This patch saved me from digging into the tracer code depths. The condition manifested for me with all pure python functions and classes exploding inside pyx files when linetracing was turned on during a |
Fixed in 2c6a6b3a4488 (bb). The reason this happens with Cython and not other dynamic_source_filename uses is that you are returning the same the original filename from dynamic_source_filename. That is, dynamic_source_filename isn't returning a new name, it's just returning the same name we already had. I'm not sure this is what you want. And with this fix, I no longer see AttributeErrors when running the Cython coverage tests, but I still see test failures. |
Originally reported by sbehnel (Bitbucket: sbehnel, GitHub: Unknown)
The C tracer misuses the "should_trace_cache" to store two different things: Disposition objects and True/False, which makes later lookups fail with an AttributeError because True/False don't have a "trace" attribute. I attached a fix. Not sure if it's the correct one.
One thing I'm unsure about is whether the object returned from the cache lookup ("included") should be used as the new disposition object instead of the current one. But my guess is it shouldn't.
The text was updated successfully, but these errors were encountered: