This is an archive of the discontinued LLVM Phabricator instance.

Don't prohibit conditional noexcept hash for unique_ptr and optional
ClosedPublic

Authored by BillyONeal on Apr 5 2017, 6:12 PM.

Details

Summary

Allow a standard library to implement conditional noexcept for optional and unique_ptr hash functions.

These tests were unconditionally asserting that optional and unique_ptr declare throwing hashes, but MSVC++ implements conditional noexcept forwarding that of the underlying hash function. As a result we were failing these tests but there's nothing forbidding strengthening noexcept in that way.

Changed the ASSERT_NOT_NOEXCEPT asserts to use types which themselves have non-noexcept hash functions.

Diff Detail

Event Timeline

BillyONeal created this revision.Apr 5 2017, 6:12 PM
mclow.lists accepted this revision.Apr 6 2017, 6:47 AM

This looks good to me - though an extra comment would help in the future.

test/std/utilities/optional/optional.hash/hash.pass.cpp
29

How about a comment here saying "deliberately not marked noexcept"?

This revision is now accepted and ready to land.Apr 6 2017, 6:47 AM
CaseyCarter added inline comments.
test/std/utilities/optional/optional.hash/hash.pass.cpp
29

An explicit noexcept(false) would be more concise.

BillyONeal updated this revision to Diff 94412.Apr 6 2017, 11:51 AM

Added noexcept(false).

BillyONeal marked 2 inline comments as done.Apr 6 2017, 11:52 AM

So this change got reverted because PointerDeleter is guarded by TEST_STD_VER. @mclow.lists would you prefer I move that assert into the 11+ block here or do you have another suggestion?

Thanks!

mclow.lists added a comment.EditedApr 11 2017, 1:59 PM

would you prefer I move that assert into the 11+ block here or do you have another suggestion?

11+ block please. [Comment about optional removed]
(and run the tests under C++03 before committing - I get bit by that far too often)

would you prefer I move that assert into the 11+ block here or do you have another suggestion?

11+ block please. [Comment about optional removed]
(and run the tests under C++03 before committing - I get bit by that far too often)

How do I do that? C1xx doesn't have a C++03 mode.

EricWF accepted this revision.Apr 13 2017, 3:36 AM

would you prefer I move that assert into the 11+ block here or do you have another suggestion?

11+ block please. [Comment about optional removed]
(and run the tests under C++03 before committing - I get bit by that far too often)

How do I do that? C1xx doesn't have a C++03 mode.

God bless C1xx for killing C++03.

I'll handle any test fallout for you. IF you implement the inline comments the patch should work in all dialects.

test/std/utilities/memory/util.smartptr/util.smartptr.hash/hash_unique_ptr.pass.cpp
53

Instead of noexcept(false) use the just newly added macro TEST_NOEXCEPT_FALSE, which is C++03 friendly.

Note: A lot of tests have a // UNSUPPORTED: c++98, c++03, ... line at the top. If you don't see that the test is expected to work in C++03, Even if it's a C++11 library feature such as unique_ptr. (Libc++ does crazy things to support unique_ptr in C++03)

Hi folks!

I tried and failed to run C++03 tests:

Is there some incantation I'm missing to get lit to pick up that I want C++03 testing mode?

Thanks!

BillyONeal updated this revision to Diff 95514.Apr 17 2017, 5:30 PM

Here's an updated diff, but I don't seem to be able to run those tests :(

Billy3

BillyONeal closed this revision.Apr 20 2017, 1:43 PM