-
Notifications
You must be signed in to change notification settings - Fork 16
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
Expression parser support for order of operations and unary negation operator #37
Expression parser support for order of operations and unary negation operator #37
Conversation
Thanks for the amazing PR! I'm unfortunately personally out of office for another week but I've assigned @pyrco to review it. You should also receive a contributor agreement message shortly as a reply on this PR (our bot is unfortunately still a human and out of office too ;)) One quick question I already have: is it possible to support integer suffixes with your parser (#32)? |
@sMezaOrellana thank you for your contribution! As this is your first code contribution, please read the following Contributor License Agreement (CLA). If you agree with the CLA, please reply with the following information:
Contributor License Agreement
Contribution License AgreementThis Contribution License Agreement ("Agreement") governs your Contribution(s) (as defined below) and conveys certain license rights to Fox-IT B.V. ("Fox-IT") for your Contribution(s) to Fox-IT"s open source Dissect project. This Agreement covers any and all Contributions that you ("You" or "Your"), now or in the future, Submit (as defined below) to this project. This Agreement is between Fox-IT B.V. and You and takes effect when you click an “I Accept” button, check box presented with these terms, otherwise accept these terms or, if earlier, when You Submit a Contribution.
|
@DissectBot agree [] |
Yes this could be supported by slightly modifying the tokenizer. I will look at it |
I have added support for suffixes. This means that when we encounter a number like '10ull' this gets parsed into '10'.
The upper code would output [2] |
…b.com:sMezaOrellana/dissect.cstruct into feature/expression_parser_precedence_negation
Hi I see that the changes were just added. I was busy rounding up some uni work the past few weeks. Glad that this still found a place in the project. |
No worries, I understand! I'd very much like this to be a part of the project so I found some free time to make my suggested changes myself. |
Codecov Report
@@ Coverage Diff @@
## main #37 +/- ##
==========================================
+ Coverage 88.23% 88.98% +0.74%
==========================================
Files 20 20
Lines 1709 1852 +143
==========================================
+ Hits 1508 1648 +140
- Misses 201 204 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@sMezaOrellana again, huge thanks for this PR! |
I realized that the expression parser class was not able to parse more 'complex' expressions.
Things like "4 * 1 + 1" would not equal "5". The previous expression parser did not support the order of operations.
I implemented this using a shunting yard parsing algorithm[1] for evaluating arithmetic expressions. I also added support for unary operators like '-' and '~'.
This implementation mostly follows the order of precedence defined in the C standard[2]. I added some more tests.
Fan of this project.
[1]
https://en.wikipedia.org/wiki/Shunting_yard_algorithm
[2]
https://en.cppreference.com/w/c/language/operator_precedence