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

Jump to bottom

to_chars can puts leading zeros on numbers #41511

Closed
mclow opened this issue Jun 6, 2019 · 6 comments
Closed

to_chars can puts leading zeros on numbers #41511

mclow opened this issue Jun 6, 2019 · 6 comments
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@mclow
Copy link
Contributor

mclow commented Jun 6, 2019

Bugzilla Link 42166
Resolution FIXED
Resolved on Jun 10, 2019 14:14
Version unspecified
OS All
CC @ivafanas,@lichray,@mclow,@jwakely

Extended Description

Given the following code:

#include
#include
#include

template
void test()
{
char buf[100];
auto res = std::to_chars(buf, buf + 100, (T)0xffffffff);
assert(res.ec == std::errc());
*res.ptr = '\0';
std::cout << (const char *) buf << std::endl;
}

int main ()
{
test();
test();
test<int64_t>();

test<unsigned int>();
test<unsigned long>();
test<uint64_t>();

}

I expect this to print:
-1
4294967295
4294967295
4294967295
4294967295
4294967295

but instead it prints:
-1
0004294967295
0004294967295
4294967295
0004294967295
0004294967295
@jwakely
Copy link
Mannequin

jwakely mannequin commented Jun 6, 2019

Runtime tests for std::to_chars
These are my tests for to_chars for integral types. These were written for libstdc++ and contributed to GCC, but as the sole author I'm free to relicense them for distribution in libc++ as well. Which I'm doing now.

There are currently several failures when using libc++. The first few seem to be the issue described here. There are also some failures in base 2 output, but I didn't analyze them.

@lichray
Copy link

lichray commented Jun 6, 2019

These are my tests for to_chars for integral types. These were written for
libstdc++ and contributed to GCC, but as the sole author I'm free to
relicense them for distribution in libc++ as well. Which I'm doing now.

Thanks for your tests.

There are currently several failures when using libc++. The first few seem
to be the issue described here.

Yes, I walked through the original code (branchlut3) contributed by RapidJSON author in debugger and saw the same issue. The issue doesn't present in branchlut2. I'm going to contacting him to look at it.

There are also some failures in base 2
output, but I didn't analyze them.

I looked into them, and I think your test cases are asking for something not required by the standard. The standard does not guarantee that we don't modify the space after r.ptr in the buffer. My code may write to anywhere in the buffer provided https://github.com/llvm-mirror/libcxx/blob/master/include/charconv#L377 then move the content to the front if needed.

@mclow
Copy link
Contributor Author

mclow commented Jun 6, 2019

Vlad has proposed a fix here: https://reviews.llvm.org/D59178
I am looking into it as well.

@jwakely
Copy link
Mannequin

jwakely mannequin commented Jun 6, 2019

Good catch, I should mark those checks as GNU-specific in our copy, and you won't want to keep that part in the libc++ tests.

@ivafanas
Copy link
Mannequin

ivafanas mannequin commented Jun 8, 2019

Bugfix proposal:
https://reviews.llvm.org/D63047

I will be very grateful for a help with review.

@mclow
Copy link
Contributor Author

mclow commented Jun 10, 2019

Fixed in revision 362967.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mclow @lichray