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

tuple comparison operators return true for tuples of different sizes #38531

Closed
tonyelewis mannequin opened this issue Oct 4, 2018 · 8 comments
Closed

tuple comparison operators return true for tuples of different sizes #38531

tonyelewis mannequin opened this issue Oct 4, 2018 · 8 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@tonyelewis
Copy link
Mannequin

tonyelewis mannequin commented Oct 4, 2018

Bugzilla Link 39183
Resolution FIXED
Resolved on Feb 10, 2019 04:33
Version 7.0
OS Linux
CC @mclow

Extended Description

The following compiles cleanly under clang++ -stdlib=libc++ -S a.cpp :

#include

static_assert( std::tuple{ 1 } == std::tuple<int, int>{ 1, 2 }, "" );
static_assert( std::tuple{ 0 } != std::tuple<int, int>{ 1, 2 }, "" );
static_assert( std::tuple{ 0 } < std::tuple<int, int>{ 1, 2 }, "" );
static_assert( std::tuple{ 2 } >= std::tuple<int, int>{ 1, 2 }, "" );

..demonstrating that these comparison operators are returning true for pairs of tuples of different sizes. This feels particularly problematic in the case of operator==().

Under "tuple.rel", the standard says that these operators require sizeof...(TTypes) == sizeof...(UTypes), though I must confess I don't know whether the standard mandates detection and reporting of violations of requirements like these. Either way, I think it'd be very much better to do so in this case.

I can reproduce this on Godbolt with "clang version 8.0.0 (trunk 343649)" (and Clangs 6 and 7).

Thanks very much for all work on libc++.

@tonyelewis
Copy link
Mannequin Author

tonyelewis mannequin commented Oct 4, 2018

assigned to @mclow

@mclow
Copy link
Contributor

mclow commented Oct 4, 2018

"Require" is how we specify a precondition - something the caller must satisfy.
So, no, that's perfectly legal (though unhelpful) result.

I have a paper in flight (https://wg21.link/P0805) that relaxes the requirement that only equal-length tuples can be compared.

@mclow
Copy link
Contributor

mclow commented Oct 4, 2018

When P0805 is adopted, then your examples #​2 and #​4 will fail.

@mclow
Copy link
Contributor

mclow commented Oct 4, 2018

Wow - I can't read.
Only #​1 will fail.

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 5, 2018

My personal preference is to fix this and force libc++ to diagnose it as ill-formed.

That being said, this looks like a conforming extension, since it should only be observable in programs that are otherwise ill-formed NDR.

@tonyelewis
Copy link
Mannequin Author

tonyelewis mannequin commented Oct 5, 2018

Thanks for the responses. Understood re the meaning of "Require". P0805's suggestions seems like good ideas to me. Thanks for that (and for all the other vast amount of great work you do for LWG, and for WG21 in general).

Of my examples, #​1 is the one that concerns me. I recently had tests that were erroneously passing under this scenario. It would have been great if libc++ had given me early indication that I was being an idiot.

My vote (FWIW) would be that if Clang 8.0 is likely to be released before libc++ is likely to be able to adopt a post-P0805 approach, it'd be really good if the equality operator could do something more helpful than at present: compile-time diagnosing, SFINAEing-away or at least returning false.

Thanks very much.

@mclow
Copy link
Contributor

mclow commented Feb 7, 2019

As of revision revision 353450 libc++ now static_asserts if you try to compare tuples of different sizes.

@tonyelewis
Copy link
Mannequin Author

tonyelewis mannequin commented Feb 10, 2019

Yep - I'm able to see that fix on Godbolt.

Fantastic stuff - thanks very much.

@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 Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

2 participants