This is an archive of the discontinued LLVM Phabricator instance.

Ensure that hash<basic_string> uses char_traits (41876)
ClosedPublic

Authored by zoecarver on May 15 2019, 10:29 AM.

Details

Summary

This patch ensures that std::hash<basic_string> uses char_traits<_CharT> and not a template parameter _Trait. Resolves 41876.

Diff Detail

Event Timeline

zoecarver created this revision.May 15 2019, 10:29 AM

Other than the skimpy traits implementation, this looks good to me.

test/std/strings/basic.string.hash/char_type_hash.fail.cpp
21

You should put a full traits implementation in here; just to be sure that some other failure isn't masking the one that you want. Just copy the default one from <__string>.

30

You can use expected-error-re to avoid having to spell out the entire error message.
Something like this:
// expected-error-re {{call to implicitly-deleted default constructor of 'std::hash<evil_char_string>' aka {{*}}}}

zoecarver marked 2 inline comments as done.May 15 2019, 10:47 AM
zoecarver added inline comments.
test/std/strings/basic.string.hash/char_type_hash.fail.cpp
21

Will do.

30

Good to know! That would also allow my first implementation to work where I created a function to hide all this in.

  • copy trait from __string
  • update expected-error-re
mclow.lists added inline comments.May 15 2019, 1:30 PM
test/std/strings/basic.string.hash/char_type_hash.fail.cpp
25

these may need std:: qualifications

29

You should get rid of the _LIBCPP_CONSTEXPR_AFTER_CXX14 and _LIBCPP_CONSTEXPR and _LIBCPP_INLINE_VISIBILITY and _NOEXCEPT bits.

zoecarver marked 2 inline comments as done.May 15 2019, 2:11 PM
zoecarver added inline comments.
test/std/strings/basic.string.hash/char_type_hash.fail.cpp
25

See line 17.

29

Will do. I was a bit lazy with copying this in :P

mclow.lists added inline comments.May 15 2019, 2:19 PM
test/std/strings/basic.string.hash/char_type_hash.fail.cpp
25

We don't usually do using namespace std; in our tests.

zoecarver updated this revision to Diff 199679.May 15 2019, 2:19 PM
  • remove libc++ macros
  • stlying
zoecarver marked an inline comment as done.May 15 2019, 2:20 PM
zoecarver added inline comments.
test/std/strings/basic.string.hash/char_type_hash.fail.cpp
25

Good to know. I will remove it.

zoecarver updated this revision to Diff 199684.May 15 2019, 2:26 PM
  • remove using namespace std;
This revision is now accepted and ready to land.May 15 2019, 5:22 PM
mclow.lists closed this revision.May 20 2019, 2:54 PM

Committed as revision 361201