-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
New value implementation #27
Conversation
Review is still in progress. I've read through everything once but I want to go over it again more thoroughly now that I understand a bit better what's going on. I like that it simplifies a lot of the ad-hoc decisions I had to make around the value system as I extended it to meet all its needs. A lot of the constexpr code around the casting matrix is really confusing but I'm slowly figuring it out. I'm not sure how I feel about how many lines have been added in template specializations but I suppose they are all optimized out by the compiler. It becomes quite hard to follow but admittedly GitHub is serving me the files in a non-intuitive order. For a lot of those files it might be nice to put a header comment at the top just explaining the purpose of the file so a reader doesn't have to puzzle through all the templates as much. I did read the new OperationNotes.md file which helped. I can see however that this will make adding lists a lot easier since they add a whole host of new value operations that the rest of the types do not need to handle. |
I added some descriptions. |
Oh, now that's test also failed, interesting, probably related to the error in #26 |
+ add numeric casting + Tests
- remove redundant value to string conversion - define own enable_if
Included in #17 |
New implementation for values at the evaluation stack, and operation handling
for evaluation stack operations.
notes/OperationNotes.md