This is an archive of the discontinued LLVM Phabricator instance.

Fix vector<bool>::const_reference to be 'bool'.
AbandonedPublic

Authored by EricWF on Apr 1 2019, 4:40 PM.

Details

Summary
Our vector<bool>::const_reference is non-conforming because the standard requires it to be `bool` but we define it as a bit wrapper type.

This patch changes the const_reference typedef guarded under the ABI flag _LIBCPP_ABI_VECTOR_BOOL_CORRECT_CONST_REFERENCE_TYPE.

Diff Detail

Event Timeline

EricWF created this revision.Apr 1 2019, 4:40 PM
mclow.lists accepted this revision.Apr 1 2019, 4:44 PM

Other than those nits, this looks fine to me.

include/__config
68

Is this testing detritus?

test/std/containers/sequences/vector.bool/insert_iter_size_value.pass.cpp
70

You should also assert(v2.size() == 3) and that v[0] == cv[0]

This revision is now accepted and ready to land.Apr 1 2019, 4:44 PM
mclow.lists added inline comments.Apr 1 2019, 4:44 PM
test/std/containers/sequences/vector.bool/types.pass.cpp
62

ASSERT_SAME_TYPE

EricWF updated this revision to Diff 193204.Apr 1 2019, 4:57 PM
EricWF marked an inline comment as done.
  • Convert the tests to ASSERT_SAME_TYPE
include/vector
2418

This is a drive-by fix because we never actually defined this function and because the const value_type& overload is sufficient in all cases.

ldionne accepted this revision.Apr 2 2019, 9:46 AM

Wow, vector<bool> is such a mess.

EricWF abandoned this revision.Apr 3 2019, 11:47 AM

I've come around to Howards position that this is a beneficial extension, and so I'm abandoning this change.

I'll follow up with a patch that adds documentation for the extension and its rational.