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 last traces of 'final_value' field #6791

Merged

Conversation

Michael0x2a
Copy link
Collaborator

This pull request is a follow-up to #6763: it renames some parameter names and variables that got missed earlier.

One interesting side-note: it seems like 'Var' notes also contain a final_value field that does almost the same thing as this last_known_value field, but is ultimately unrelated: it was introduced back in #5522. I decided to leave this field alone.

(I discovered this while updating mypyc and discovered (to my surprise) that nothing broke.)

This pull request is a follow-up to python#6763:
it renames some parameter names and variables that got missed up above.

One interesting side-note: it seems like 'Var' notes also contain a
`final_value` field that does almost the same thing as this
`last_known_value` field, but is ultimately unrelated: it was introduced
back in python#5522. I decided to leave
this field alone.

(I discovered this while updating mypyc and discovered (to my surprise)
that nothing broke.)
@Michael0x2a
Copy link
Collaborator Author

As a tangent, the presence of this new Var.final_value field makes me wonder if it might be worth supporting Literal floats after all.

It'd be nice if we could unify this field with the existing Literal types work, or even remove it altogether in favor of using the inferred types. But that would mean dropping support for floats/dropping support for optimizing code involving floats in mypyc, which feels bad.

@ilevkivskyi
Copy link
Member

As a tangent, ...

I would leave it as is for now, but in long term perspective I see a value in simplifying these things.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ilevkivskyi ilevkivskyi merged commit 285b683 into python:master May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants