Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add rewrite rules to remove inlining. #24

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

jhance commented Apr 4, 2012

Literals generate a huge amount of code for almost no benefit (in fact,
the literals are converted from an array of bytes to a list back to
text). Add rules to fix this and make generated core much nicer.

See issue #19.

Add rewrite rules to remove inlining.
Literals generate a huge amount of code for almost no benefit (in fact,
the literals are converted from an array of bytes to a list back to
text). Add rules to fix this and make generated core much nicer.

bos commented on 2ddf0a8 Apr 8, 2012

You're barking up a pretty good tree here, but this patch has some problems.

  1. It doesn't follow the coding style of the surrounding code. Lines too long, indentation wrong, and unqualified, unlimited imports, just from what I can see in my browser window.
  2. There are no tests. If there were, you'd see that this patch does not work for strings containing ASCII NUL.
Collaborator

bos commented Apr 8, 2012

Thanks for the patch. I wrote a similar version myself that doesn't have the NUL problem; see 2b2cb08 for details.

@bos bos closed this Apr 8, 2012

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