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

Reassign variable names : still considering (but not replacing) text in strings #76

Closed
Siorki opened this issue Feb 28, 2017 · 2 comments
Assignees
Labels
Milestone

Comments

@Siorki
Copy link
Owner

Siorki commented Feb 28, 2017

Issue found while working on my entry in js1k 2017.

The module log lists $ as variable, although I only use it in template strings. This can still be observed with the final code for the demo.

A new name is assigned for it, however it does not get replaced inside the string (see #57) so the output is correct.

However, the new name is squandered - it could have been assigned to another (real) variable instead. If there are not enough names left, this would lead to a character being unnecessarily allocated from outside the name pool, effectively removing a possible token.

@Siorki Siorki self-assigned this Feb 28, 2017
@Siorki Siorki added the bug label Feb 28, 2017
@Siorki
Copy link
Owner Author

Siorki commented Sep 29, 2018

Example code showing the issue, used in the non-regression test for #82

p=c.getImageData(0,0,512,512);d=p.data;for(j=0;j<512;++j){c.fillStyle=`#${`200`,`300`,`fff`,`000`][j&3]}`;d[j*4]=0;d[j*4+1]=255;d[j*4+2]=j>>1;d[j*4+3]=0;c.beginPath();c.moveTo(0,j);c.lineTo(511,j);c.stroke();}

$ gets renamed to D, even though there is no D showing in the preprocessed code.

@Siorki Siorki added this to the 5.0.2 milestone Oct 11, 2018
Siorki added a commit that referenced this issue Oct 19, 2018
@Siorki
Copy link
Owner Author

Siorki commented Oct 19, 2018

Added an identification of template literals, stored in PackerData.

Variable identification and replacement are only performed outside of strings and inside template literals.

01b66ea

@Siorki Siorki closed this as completed Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant