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

num_get::do_get incorrect digit grouping check #29078

Closed
llvmbot opened this issue Jul 25, 2016 · 6 comments
Closed

num_get::do_get incorrect digit grouping check #29078

llvmbot opened this issue Jul 25, 2016 · 6 comments
Assignees
Labels
bugzilla libc++

Comments

@llvmbot
Copy link
Collaborator

@llvmbot llvmbot commented Jul 25, 2016

Bugzilla Link 28704
Resolution FIXED
Resolved on Jun 17, 2019 06:42
Version unspecified
OS All
Reporter LLVM Bugzilla Contributor
CC @Quuxplusone,@mclow

Extended Description

For a locale which specifies digit grouping, num_get::do_get will enforce
that the number read in contains the grouping character at the correct
locations. It does this even if the number read in didn't have any grouping
characters present.

This is shown best with a code example:

#include
#include

int main()
{
std::locale::global(std::locale("en_US.UTF-8"));
std::istringstream iss("1024");
int a = 0;
iss >> a;
std::cout << a << ", " << iss.fail() << std::endl;
}

With libc++ this prints:

1024, 1

With libstdc++ this prints:

1024, 0

http://coliru.stacked-crooked.com/a/9283df64c2bd5142

The standard says:

Digit grouping is checked. That is, the positions of discarded separators is
examined for consistency with
use_facet<numpunct >(loc).grouping(). If they are not consistent then
ios_base::failbit is assigned to err.

http://eel.is/c++draft/facet.num.get.virtuals#4

Whilst the standard could be clearer, it seems one can take it to mean that as
there are no discarded characters, there is nothing to examine for consistency.

This is the approach that libstdc++ takes, as well as boost's lexical_cast:

https://svn.boost.org/trac/boost/ticket/5585

I think it's reasonable that libc++ changes its implementation to match.

@llvmbot
Copy link
Collaborator Author

@llvmbot llvmbot commented Jul 25, 2016

assigned to @mclow

@mclow
Copy link
Contributor

@mclow mclow commented Aug 15, 2017

Reduced test case
Reduced test case.
Passes with Visual Studio online compiler.
Passes (with a warning) with GCC 7.
Fails with libc++

@mclow
Copy link
Contributor

@mclow mclow commented Aug 15, 2017

If you've said "this is a facet that does grouping", then it's hard to argue that "no grouping" is consistent with that facet.

On the other hand, the original example would be nice to support.

@mclow
Copy link
Contributor

@mclow mclow commented Jun 4, 2019

Fixed in revision 362508.

@Quuxplusone
Copy link
Contributor

@Quuxplusone Quuxplusone commented Jun 8, 2019

Marshall, your libcxx commit changes a line in src/locale.cpp from

-    if (__grouping.size() != 0)

to

+       if (__grouping.size() >= 0 && __g_end - __g > 1)

which might have been meant to be != instead of >= in the new line.
At least, according to my compiler, __grouping.size() is unsigned, so >= 0 is invariably true. Should it have been != (or >), or did you mean to remove the now-tautological condition entirely?

@mclow
Copy link
Contributor

@mclow mclow commented Jun 17, 2019

thanks. Commit 363557

@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
Labels
bugzilla libc++
Projects
None yet
Development

No branches or pull requests

3 participants