This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [test] Fix Container::insert(value_type const&) tests
ClosedPublic

Authored by jloser on Mar 12 2018, 7:49 PM.

Details

Summary

[libcxx] [test] Fix Container::insert(value_type const&) tests

Several unit tests meaning to test the behavior of lvalue insertion incorrectly pass rvalues. Fixes bug PR # 27394

Diff Detail

Event Timeline

jloser created this revision.Mar 12 2018, 7:49 PM
jloser edited the summary of this revision. (Show Details)Mar 13 2018, 2:39 PM
EricWF added a comment.Apr 3 2018, 5:13 PM

Have you verified that we're not losing test coverage here? That is, are you sure we still have tests for the rvalue overloads in other test files?

libcxx/test/std/containers/associative/multiset/insert_cv.pass.cpp
41

Should be v3?

jloser retitled this revision from [libc++] Fix Container::insert(value_type const&) tests to [libcxx] [test] Fix Container::insert(value_type const&) tests.Apr 8 2018, 1:46 PM
jloser edited the summary of this revision. (Show Details)
jloser updated this revision to Diff 141573.Apr 8 2018, 2:15 PM

Use v3 rather than an rvalue of 3 in libcxx/test/std/containers/associative/multiset/insert_cv.pass.cpp

jloser marked an inline comment as done.Apr 8 2018, 2:59 PM
EricWF accepted this revision.Apr 8 2018, 3:00 PM

LGTM. Thanks!

This revision is now accepted and ready to land.Apr 8 2018, 3:00 PM
EricWF closed this revision.Apr 8 2018, 3:00 PM

Committed as r329541.

jloser added a comment.Apr 8 2018, 3:00 PM

Have you verified that we're not losing test coverage here? That is, are you sure we still have tests for the rvalue overloads in other test files?

Yep. These containers already have tests for inserts with rvalues. No loss in test coverage here.