Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Improve ROT_TWO support #182

Merged
merged 1 commit into from Apr 25, 2016
Merged

Improve ROT_TWO support #182

merged 1 commit into from Apr 25, 2016

Conversation

meadori
Copy link
Contributor

@meadori meadori commented Apr 22, 2016

This partially fixes #88 by improving how code for ROT_TWO
is generated. In particular, optimized locals are used for
integers and floats. A similar fix will be made for ROT_THREE
in a following patch.

@msftclas
Copy link

Hi @meadori, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@msftclas
Copy link

@meadori, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@brettcannon
Copy link
Member

I just wanted to quickly say thanks for the PR, @meadori ! I don't know how long we will take to review the PR but I promise we will get to it (both @DinoV and I have presentations to prep for next week).

@DinoV
Copy link
Contributor

DinoV commented Apr 22, 2016

Mostly looks great, but what happens with mixed variables, e.g:
x = 1.0
y = 'abc'
x,y = (y, x)

It seems like we should be marking them as escaping if they differ?

This partially fixes #88 by improving how code for ROT_TWO
is generated.  In particular, optimized locals are used for
integers and floats.  A similar fix will be made for ROT_THREE
in a following patch.
@meadori
Copy link
Contributor Author

meadori commented Apr 25, 2016

Good point. I updated the code to escape if the types differ and I added a few more test cases.

@DinoV DinoV merged commit 68cacfe into microsoft:master Apr 25, 2016
@DinoV
Copy link
Contributor

DinoV commented Apr 25, 2016

Thanks for your contribution! That's the first significant contribution we've received!

@meadori meadori deleted the issue-88 branch April 26, 2016 15:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve ROT_TWO/ROT_THREE support
4 participants