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

lang/llvmir/parser: Fix typo in parse_arithmetic() (was "arithmatic"). #124

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pfalcon
Copy link
Contributor

@pfalcon pfalcon commented Feb 11, 2021

Signed-off-by: Paul Sokolovsky [email protected]

@codecov-io
Copy link

codecov-io commented Feb 11, 2021

Codecov Report

Merging #124 (f6412ae) into master (aacfd18) will not change coverage.
The diff coverage is 33.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   78.96%   78.96%           
=======================================
  Files         353      353           
  Lines       47323    47323           
=======================================
  Hits        37370    37370           
  Misses       9953     9953           
Impacted Files Coverage Δ
ppci/lang/llvmir/codegenerator.py 8.75% <ø> (ø)
ppci/lang/llvmir/parser.py 12.54% <33.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aacfd18...f6412ae. Read the comment docs.

@pfalcon
Copy link
Contributor Author

pfalcon commented Feb 11, 2021

@windelbouwman: I'm contemplating splitting out this parser as a standalone module. I searched for other Python LLVM IR parsers, but couldn't find something better. It's definitely an improvement over my older hackish "parser" https://github.com/pfalcon/llvm-codegen-py/blob/master/parse.py ;-). But then this parser has a bunch of its own issues. For example, I definitely belong to a sect of witnesses of recursive descent, but that arguably makes things only worse, because in my sect, "has_consumed()" is named "match()", and "consume()" is named "expect()" (in my sect "match()" couldn't be named "has_consumed()", simply because "has_", just as "is_" is prefix for boolean accessors without side effects).

Bottom line: I'd like to land as many fixes as possible before forking it, but it could also that this is last non-invasive patch, because I probably won't sustain usage of has_consumed() in the code I type. Let's see if I will be able to contain myself ;-).

@pfalcon
Copy link
Contributor Author

pfalcon commented Feb 11, 2021

Ah, and the general idea of why LLVM IR is important - all the recent 1.5yrs of development show that (expectedly) approach of "rewrite the whole world from scratch" doesn't scale. Instead of spreading thin on language frontends, linker, stdlibs, the project should have concentrated on interoperability with existing language frontends, runtimes, and linkers, and focus on the core part - decent IR, optimizer and codegenerator. I'm biased here of course, as I find only those parts "interesting", and the rest "boring" ;-). (I'm also losing faith in decency of PPCI IR, as issues in #43 are not being addressed. That's actually the reason to fork the parser - to use with a different IR).

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

2 participants