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

Current LLVM copy of to_chars is wrong for integers with more than 9 digits #1595

Closed
smehringer opened this issue Feb 17, 2020 · 2 comments · Fixed by #1976
Closed

Current LLVM copy of to_chars is wrong for integers with more than 9 digits #1595

smehringer opened this issue Feb 17, 2020 · 2 comments · Fixed by #1976
Assignees
Labels
bug faulty or wrong behaviour of code
Milestone

Comments

@smehringer
Copy link
Member

Platform

  • SeqAn version: tested with 3.0.1
  • Operating system: Linux
  • Compiler: tested with GCC-7/GCC-9

Description

Converting a number with 9 digits or more fails with the current implementation in the std/charconv header. If I include the stl <charconv> with GCC-9 directly the error does not occur.

There is also the possibility that we introduced the error while copying..
We should update the code and may want to consider copying the MSVC version that is supposed to be fast on floats.

How to repeat the problem

The following test fails with the current implementation

TEST(to_chars_test, large_number)
{
    uint64_t val{123456789};
    std::array<char, 100> buffer{};

    auto res = std::to_chars(buffer.data(), buffer.data() + buffer.size(), val);

    EXPECT_EQ(res.ptr, &buffer[9]);
    EXPECT_EQ(res.ec, std::errc{});
    EXPECT_EQ((std::string_view{buffer.data(), 9}), std::string_view{"123456789"});
}
@smehringer smehringer added the bug faulty or wrong behaviour of code label Feb 17, 2020
@marehr
Copy link
Member

marehr commented May 11, 2020

Core-Meeting:

@marehr has free reign to do this; main idea is to put the llvm implementation into contrib and add the custom bits, like the float overloads, in std/charconv.

@marehr marehr self-assigned this May 11, 2020
@marehr
Copy link
Member

marehr commented Jun 30, 2020

2020-06-30 Core Meeting

Discussed license change in the LLVM project from

This file is dual licensed under the MIT and the University of Illinois Open Source Licenses.
https://github.com/llvm/llvm-project/blob/d27489645bb8829a952f79e3b211202844238391/libcxx/src/charconv.cpp

to

SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
https://github.com/llvm/llvm-project/blob/master/libcxx/src/charconv.cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug faulty or wrong behaviour of code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants