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

Demo: Constification of library for C++20 #42

Closed
wants to merge 8 commits into from

Conversation

johnmcfarlane
Copy link
Contributor

Don't merge this change unless you want to drop C++11/14/17 support! It's just for demo purposes.

@johnmcfarlane johnmcfarlane mentioned this pull request May 1, 2021
@ckormanyos
Copy link
Owner

Nice job @johnmcfarlane and many thanks for this inspirational direction!

Somehow all this is leading to either a newer version of the LIB with signed/unsigned/constexpr support, as I hinted in previous posts. For some time, I have known that I'll be re-factoring or re-loading the wide_integer project, now we need to figure out the best way(s) to do that while both:

  • retaining the functionality for exiasing clients of uintwide_t,
  • and moving forward toward some standardization aspects and C++20, etc. as discussed in Signed extension planned? #10.

All this information needs to solidify in my mind.

@johnmcfarlane
Copy link
Contributor Author

Take your time. I wouldn't rush to do-over from scratch. You'll find that your tests -- in particular -- are very valuable and will want to be kept. I maintain that you can think of the signed and constexpr features as orthogonal. As you can see, constexpr isn't a particularly disruptive change: high churn, but low complexity.

If you're going to spent some time planning, I would think about signedness and how that changes things. But I'd still recommend evolution over revolution.

If it were my choice, I'd indeed no start by changing uintwide_t, but maybe rework the two underlying types and add a signed equivalent of uintwide_t, lets call it intwide_t. In CNL, I aim to add functionality equivalent to uintwide_t, but not in multi-word-specific solution. Thoughts on this are tracked here. (Don't confuse width_integer with a wide integer there!)

So basically, I think our approaches may naturally fork at [u]intwide_t: you provide a minimal, wide-integer-focussed type that is easy for users of the wide-integer library to use. I, on the other hand, suspect I want to deviate from the design of uintwide_t, providing a solution involving something called 'families' and using the cnl::digits_v and cnl::set_digits_t type traits.

@johnmcfarlane johnmcfarlane force-pushed the constexpr branch 2 times, most recently from fa1d124 to 00cf1a8 Compare May 1, 2021 18:08
@ckormanyos
Copy link
Owner

If you're going to spent some time planning, I would think about signedness and how that changes things. But I'd still recommend evolution over revolution.

That seems like good advice.

I do not intend to throw out or break in any way my existing uintwide_t type. There might be a way to add a minimally intrusive template parameter for signed-ness. In Boost we handle (partially) constexpr with macros.

We (and I especially) will need to think about all of this for a while and figure out how best to provide the signed type requested.

@ckormanyos
Copy link
Owner

So basically, I think our approaches may naturally fork at [u]intwide_t: you provide a minimal, wide-integer-focussed type...

OK. I will try.

@johnmcfarlane johnmcfarlane force-pushed the constexpr branch 2 times, most recently from fa04a7c to 102c659 Compare May 2, 2021 10:05
@johnmcfarlane
Copy link
Contributor Author

Hi. I came up with an alternative approach which I think may be worthy of merging in #45. I'll close this PR if that looks acceptable and once it's in.

@ckormanyos
Copy link
Owner

Understood. Now being handled in #45

@johnmcfarlane johnmcfarlane deleted the constexpr branch January 9, 2022 09:27
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.

2 participants