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

Clean up kludges for handling metaclasses and Type[...] in check_op_reversible #5491

Open
Michael0x2a opened this issue Aug 16, 2018 · 4 comments

Comments

@Michael0x2a
Copy link
Collaborator

The current implementation for checking reversible operators contains two kludges that seem to be related to operators, metaclasses, and Type[...]

  1. This call to has_member feels like it ought to be unnecessary/redundant due to the following analyze_member_access -- but removing it causes pythoneval's OpMetaclassAccess test case to fail.

    (Note: if you remove just that call, you also need to add in a elif isinstance(member, AnyTyp): return None after calling analyze_member_access)

  2. If the reversible operators logic was unable to find a matching operator, it just tries calling check_call on the name directly to get back some generic error message. It seems as if this call ought to always return an error message, but sometimes it surprisingly will not. E.g. see the TypeTypeComparisonWorks tests in check-classes -- adding an assert local_errors.is_errors() causes that test case to crash.

    This feels related to the first TODO in some way -- either we're not handling operators + metaclasses + Type directly, or there are some weird shenanigans happening due to Any...

For additional context, see #5136 and #1806.

@Michael0x2a Michael0x2a self-assigned this Aug 16, 2018
@elazarg
Copy link
Contributor

elazarg commented Aug 17, 2018

I should work on this, if I will find the time.

@Michael0x2a Michael0x2a removed their assignment Aug 17, 2018
@Michael0x2a
Copy link
Collaborator Author

@elazarg -- sounds good to me!

(I added myself mostly out of a sense of obligation, since the TODOs were a fallout of my operator refactors. But I also don't know when I'll find the time to do this, so feel free to tackle this whenever.)

@elazarg
Copy link
Contributor

elazarg commented Sep 16, 2018

@Michael0x2a - sorry, I can't seem to find the time to do that.

@Michael0x2a
Copy link
Collaborator Author

@elazarg -- no worries, I'm also in the same boat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants