-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Expand int syntax #1900
Expand int syntax #1900
Conversation
TODO:
|
src/ast/attachpoint_parser.cpp
Outdated
@@ -5,14 +5,42 @@ | |||
#include <functional> | |||
#include <string> | |||
#include <unordered_map> | |||
#include <variant> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accidental include? Don't see any variant usage in this commit
src/ast/int_parser.cpp
Outdated
template <typename T> | ||
T _parse_int(const std::string &num, size_t *idx, int base) | ||
{ | ||
throw std::runtime_error("parse_int not implemented for type"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can make this a compile time error instead. Example: https://godbolt.org/z/sEs3xhac4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah that works :)
7357141
to
60b582c
Compare
By centralising this logic we can re-use it for uniform int handling throughout our AST. This parser supports `_`s as int separators, e.g. 1_000_000
This will allow us use underscores as int digit separator to improve readability, e.g. 1_000_000 and unifies the int parsing logic allowing us to use the same syntax in attach points. E.g. hardware:cache-misses:1e6 or 1_000 are now valid instead.
The exponent parsing thing feels a bit clunky, c++ supports it out of the box but for doubles only, might be easier to just use that instead. But its hidden behind a simple interface so we can always change it if needed.
Fixes #1897
Fixes #1898