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

Add numeric conversions #26

Open
3 tasks
unterumarmung opened this issue Aug 16, 2021 · 12 comments
Open
3 tasks

Add numeric conversions #26

unterumarmung opened this issue Aug 16, 2021 · 12 comments
Labels
api enhancement New feature or request good first issue Good for newcomers

Comments

@unterumarmung
Copy link
Owner

unterumarmung commented Aug 16, 2021

Suggested API:

    // numeric conversions
    template <size_t N>
    constexpr int stoi(const fixed_string<N>& str, int base = 10);
    
    template <size_t N>
    constexpr unsigned stou(const fixed_string<N>& str, int base = 10);
    
    template <size_t N>
    constexpr long stol(const fixed_string<N>& str, int base = 10);
    
    template <size_t N>
    constexpr unsigned long stoul(const fixed_string<N>& str, int base = 10);
    
    template <size_t N>
    constexpr long long stoll(const fixed_string<N>& str, int base = 10);
    
    template <size_t N>
    constexpr unsigned long long stoull(const fixed_string<N>& str, int base = 10);
    
    template <size_t N>
    constexpr float stof(const fixed_string<N>& str);
    
    template <size_t N>
    constexpr double stod(const fixed_string<N>& str);
    
    template <size_t N>
    constexpr long double stold(const fixed_string<N>& str);
    
    template <auto val> // where decltype(val) is an integral type but not any of char types
    constexpr fixed_string</*...*/> to_fixed_string() noexcept;

Proposed implementation: using to_chars and from_chars from <charconv>
Known issues: cannot be really constexpr

  • Create implementation
  • Create tests
  • Add docs
@unterumarmung unterumarmung added enhancement New feature or request good first issue Good for newcomers api labels Aug 16, 2021
@abel-mak
Copy link

Hi, i implemented stoi can i open pr for it?

@unterumarmung
Copy link
Owner Author

@abel-mak sure!

@abel-mak
Copy link

@unterumarmung Hey, i didn't use from_chars in the implementation, must i use it?

@unterumarmung
Copy link
Owner Author

@abel-mak
from_chars is not constexpr
So:

  1. For C++17 we can use from_chars and doesn't allow to call our function in the constant context
  2. For C++20 with if (std::is_constant_evaluated()) we can use our own implementation that is constexpr and use from_chars at the run-time

@unterumarmung
Copy link
Owner Author

unterumarmung commented Jan 21, 2022

You can open the PR and I will take a look what have so far

@abel-mak
Copy link

@unterumarmung this what i've done so far
abel-mak@1f9725e

@unterumarmung
Copy link
Owner Author

@abel-mak I've looked on what you already written
Below you can find rules from C++ Core Guidelines that will make your code better:

  1. Do not declare before actual use (for example sign variable): https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-init
  2. After applying Rule 1, it will be easier to apply this one: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rconst-immutable
  3. Also, please use auto for long type names

@unterumarmung
Copy link
Owner Author

You can open the PR: it's easier to review code with opened pull request

@unterumarmung
Copy link
Owner Author

Also, there's a tradeoff to be made with C++17, we either should:

  1. Provide not as effective stoi implementation for run-time but keep the ability to call the function at compile-time
  2. Or remove constexpr in C++17 and provide effective run-time implementation with from_chars

There is no tradeoff in C++20 since we can distinguish run-time and compile-time with if (std::is_constant_evaluated())

@unterumarmung
Copy link
Owner Author

unterumarmung commented Jan 22, 2022

@GeorgyFirsov @DymOK93 What do you think about all that (the code and the tradeoff)?

@DymOK93
Copy link
Contributor

DymOK93 commented Jan 23, 2022

  1. It is unsafe to use std::is* functions without conversion to unsigned char: for example, see description of the std::isspace
  2. I tried to use the __is_constant_evaluated compiler intrinsics, but they seem to have appeared much later than C++17. It might be wise to try to apply them, but in general, you will have to leave the C++17 version without constexpr.
    I'll think about this question... The answer depends on the most promising use of fixed_string

@abel-mak
Copy link

abel-mak commented Feb 5, 2022

@unterumarmung hey, i opened PR and tried to add some changes from what you said about rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants