Initial implementation of digits -> Nat -> digits #120
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is expected to be the namespace it will live in once it's rolled out into its own library; thus when it is, code changes will be minimal for clients (should only require an additional include file if they want to be really strict about it, but technically speaking even that can be omitted as arby will include it itself)
These tests appear to actually hang, don't know why...
If you don't instruct Catch2 generators to take just 1 value, using random() will loop forever
…digits back out again TODO: - add ctor for digits initializer_list - add ctor for other iterables of digits? - consider what happens when Nat is initialised from an empty iterable - consider what happens when a Nat object with empty digits gets digits() called on it
For some reason, the product test case one doesn't compile unless sign-conversion warnings are disabled. Not clear why...
This is due to a Catch2 bug in v2: catchorg/Catch2#2348 Time to move to Catch2 v3?
…O answer:" This doesn't work on MSVC (but does on Clang) This reverts commit 0fa8674.
Not required for it to work, but bypasses a manual push_back() of each element into the container in favour of passing the initializer_list directly to the container --not much different currently, as the container just does repeated push_back()s itself, but this leaves any future optimisation of these kinds of initialisation up to the container (for example, a container could read the size of the initializer_list and allocate the exact required amount of memory for the elements in advance, then just read the values into storage, rather than re-allocate memory each time).
Took this as an opportunity to also refactor out leading-zero-elision into a dedicated private method, given that it was already in use in 2 or 3 other places prior to the addition of these 3 additional uses.
Changed digits() to deliberately return a non-const reference to verify this (make it fail)
We don't have it as a reference for real, and we can't store the result of a stack value as reference type, so we've removed the test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TODO:
Closes #114