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

Named phi #426

Merged
merged 4 commits into from Jul 24, 2019
Merged

Named phi #426

merged 4 commits into from Jul 24, 2019

Conversation

cdonovick
Copy link
Collaborator

@cdonovick cdonovick commented Jul 23, 2019

Adds utility function gen_free_name

changes ssa:

  • ssa now takes an optional argument phi which can be either a function or a string
    • default value "phi"
    • if phi is a string then ssa will assume that such a name exists in the environment of the decorated function
    • if phi is a function then ssa will generate a new name and associate phi with the name in the generated code
  • moves the stripping of decorators from convert_tree_to_ssa to ssa

@cdonovick cdonovick requested a review from leonardt July 23, 2019 21:15
@coveralls
Copy link

coveralls commented Jul 23, 2019

Pull Request Test Coverage Report for Build 1895

  • 39 of 47 (82.98%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 74.227%

Changes Missing Coverage Covered Lines Changed/Added Lines %
magma/ast_utils.py 22 30 73.33%
Totals Coverage Status
Change from base Build 1892: 0.03%
Covered Lines: 4271
Relevant Lines: 5754

💛 - Coveralls

name = f_str.format(c)
while name in used_names:
c += 1
name = f_str.format(c)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add a timeout (e.g. if c >= 1000: raise Exception). Unlikely to come up and would incur a performance cost, but you never know?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As python ints don't overflow don't see the need to give up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just realized we can assume that the space of names is finite, so we will eventually find a name.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(because the length of the program must be finite)

Copy link
Collaborator

@leonardt leonardt left a comment

Choose a reason for hiding this comment

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

LGTM

@leonardt leonardt merged commit a3f2ada into master Jul 24, 2019
@leonardt leonardt deleted the named-phi branch July 24, 2019 16:40
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.

None yet

3 participants