Skip to content
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

Closed
nedbat opened this issue Jun 9, 2015 · 6 comments
Closed

C tracer: lookups fail in "should_trace_cache" #374

nedbat opened this issue Jun 9, 2015 · 6 comments
Labels
bug Something isn't working run

Comments

@nedbat
Copy link
Owner

nedbat commented Jun 9, 2015

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.


@nedbat
Copy link
Owner Author

nedbat commented Jun 9, 2015

@sbehnel Do you have a test case that demonstrates the problem?

@nedbat
Copy link
Owner Author

nedbat commented Jun 9, 2015

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.

@nedbat
Copy link
Owner Author

nedbat commented Jun 10, 2015

by "Cython code tracing fail," do you mean a Cython plugin for coverage? Either way, I'd like to know more.

@nedbat
Copy link
Owner Author

nedbat commented Jun 10, 2015

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.

@nedbat
Copy link
Owner Author

nedbat commented Jun 14, 2015

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 coverage run call. I would see messages of the form "AttributeError: 'bool' object has no attribute 'trace'" with a stacktrace to a pure python element. All the problems went away when I applied the patch to coverage.py. I can probably create a subset of the problems in a minimal cython repo (0.23dev) if it helps get this patch applied/merged.

@nedbat
Copy link
Owner Author

nedbat commented Jun 14, 2015

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.

@nedbat nedbat closed this as completed Jun 14, 2015
@nedbat nedbat added major bug Something isn't working run labels Jun 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working run
Projects
None yet
Development

No branches or pull requests

1 participant