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 LLVM lexer #1376

Merged
merged 3 commits into from Dec 15, 2019
Merged

Clean up LLVM lexer #1376

merged 3 commits into from Dec 15, 2019

Conversation

pyrmont
Copy link
Contributor

@pyrmont pyrmont commented Dec 13, 2019

The impetus for cleaning up the LLVM lexer was the failure to properly match keywords as discussed in #1372. Partly this was due to a keyword missing but partly it was due to the use of String#join to create brittle regex patterns.

This PR uses memoising singleton methods and the use of Array#include? to match against a set of keywords. Doing this reduces memory usage by only allocation the keyword strings when the method is called, allows for the keywords to be ordered alphabetically which aids in legibility and, frankly, just looks nicer.

Finally, the addrspacecast keyword is added.

This fixes #1372.

@pyrmont pyrmont added the needs-review The PR needs to be reviewed label Dec 13, 2019
@pyrmont pyrmont self-assigned this Dec 13, 2019
@pyrmont
Copy link
Contributor Author

pyrmont commented Dec 13, 2019

@jholewinski I added the string from the issue to the visual sample and it's lexing correctly now on my system. Does everything look OK to you?

@jholewinski
Copy link

Looks good, thanks!

@pyrmont pyrmont merged commit c93ca78 into rouge-ruby:master Dec 15, 2019
@pyrmont pyrmont removed the needs-review The PR needs to be reviewed label Dec 16, 2019
@pyrmont pyrmont deleted the bugfix.llvm-cleanup branch April 3, 2020 21:59
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.

LLVM lexer does not handle addrspace or addrspacecast
2 participants