Skip to content

F
folly
Commit aebe18a7 authored by Xiao Shi's avatar Xiao Shi Committed by Facebook Github Bot
Browse files
Options

work around (fixed) libstdc++ bug for heterogeneous mutation 

Summary:
N4387 lifted the restrictions on `pair` and `tuple` construction that the
element types must be implicitly convertible to the target element types. Prior
to GCC 6, this document was not implemented in libstdc++. There are other
subsequent fixes to address LWG2367, for exmaple.

The upstream implementation for N4387 is here:
https://github.com/gcc-mirror/gcc/commit/d3c64041b32b6962ad6b2d879231537a477631fb

This diff adds the `insert` overloads to work around these issues as well as
test coverage.

Note that the following overload still wouldn't work:
  m.insert({sp, x})
since the compiler is not able to deduce the element types without attempting
to construct a `pair`.

Reviewed By: yfeldblum, nbronson

Differential Revision: D9117504

fbshipit-source-id: c6fae625bf4d9518cb2b4bcc76465b004c4cd253
parent a62ff6ec
Hide whitespace changes
Inline Side-by-side
Showing with 142 additions and 7 deletions
folly/container/F14Map.h
View file @ aebe18a7
......@@ -332,6 +332,27 @@ class F14BasicMap {
return emplace(std::forward<P>(value));
}
// TODO(T31574848): Work around libstdc++ versions (e.g., GCC < 6) with no
// implementation of N4387 ("perfect initialization" for pairs and tuples).
template <typename U1, typename U2>
std::enable_if_t<
std::is_constructible<key_type, U1 const&>::value &&
std::is_constructible<mapped_type, U2 const&>::value,
std::pair<iterator, bool>>
insert(std::pair<U1, U2> const& value) {
return emplace(value);
}
// TODO(T31574848)
template <typename U1, typename U2>
std::enable_if_t<
std::is_constructible<key_type, U1&&>::value &&
std::is_constructible<mapped_type, U2&&>::value,
std::pair<iterator, bool>>
insert(std::pair<U1, U2>&& value) {
return emplace(std::move(value));
}
std::pair<iterator, bool> insert(value_type&& value) {
return emplace(std::move(value));
}
......@@ -458,8 +479,16 @@ class F14BasicMap {
std::pair<ItemIter, bool> emplaceItem(U1&& x, U2&& y) {
using K = KeyTypeForEmplace<key_type, hasher, key_equal, U1>;
K key(std::forward<U1>(x));
// TODO(T31574848): piecewise_construct is to work around libstdc++ versions
// (e.g., GCC < 6) with no implementation of N4387 ("perfect initialization"
// for pairs and tuples). Otherwise we could just pass key, forwarded key,
// and forwarded y to tryEmplaceValue.
return table_.tryEmplaceValue(
key, std::forward<K>(key), std::forward<U2>(y));
key,
std::piecewise_construct,
std::forward_as_tuple(std::forward<K>(key)),
std::forward_as_tuple(std::forward<U2>(y)));
}
template <typename U1, typename U2>
......
folly/container/test/F14MapTest.cpp
View file @ aebe18a7
......@@ -1200,8 +1200,7 @@ TEST(F14FastMap, moveOnly) {
runMoveOnlyTest<F14FastMap<f14::MoveOnlyTestInt, f14::MoveOnlyTestInt>>();
}
TEST(F14ValueMap, heterogeneous) {
// note: std::string is implicitly convertible to but not from StringPiece
TEST(F14ValueMap, heterogeneousLookup) {
using Hasher = folly::transparent<folly::hasher<folly::StringPiece>>;
using KeyEqual = folly::transparent<std::equal_to<folly::StringPiece>>;
......@@ -1210,8 +1209,8 @@ TEST(F14ValueMap, heterogeneous) {
constexpr auto world = "world"_sp;
F14ValueMap<std::string, bool, Hasher, KeyEqual> map;
map.emplace(hello.str(), true);
map.emplace(world.str(), false);
map.emplace(hello, true);
map.emplace(world, false);
auto checks = [hello, buddy](auto& ref) {
// count
......@@ -1362,12 +1361,64 @@ void runHeterogeneousInsertTest() {
<< Tracked<1>::counts;
}
template <typename M>
void runHeterogeneousInsertStringTest() {
using P = std::pair<StringPiece, std::string>;
using CP = std::pair<const StringPiece, std::string>;
M map;
P p{"foo", "hello"};
std::vector<P> v{p};
StringPiece foo{"foo"};
map.insert(P("foo", "hello"));
// TODO(T31574848): the list-initialization below does not work on libstdc++
// versions (e.g., GCC < 6) with no implementation of N4387 ("perfect
// initialization" for pairs and tuples).
// StringPiece sp{"foo"};
// map.insert({sp, "hello"});
map.insert({"foo", "hello"});
map.insert(CP("foo", "hello"));
map.insert(p);
map.insert(v.begin(), v.end());
map.insert(
std::make_move_iterator(v.begin()), std::make_move_iterator(v.end()));
map.insert_or_assign("foo", "hello");
map.insert_or_assign(StringPiece{"foo"}, "hello");
EXPECT_EQ(map["foo"], "hello");
map.emplace(StringPiece{"foo"}, "hello");
map.emplace("foo", "hello");
map.emplace(p);
map.emplace();
map.emplace(
std::piecewise_construct,
std::forward_as_tuple(StringPiece{"foo"}),
std::forward_as_tuple(/* count */ 20, 'x'));
map.try_emplace(StringPiece{"foo"}, "hello");
map.try_emplace(foo, "hello");
map.try_emplace(foo);
map.try_emplace("foo");
map.try_emplace("foo", "hello");
map.try_emplace("foo", /* count */ 20, 'x');
map.erase(StringPiece{"foo"});
map.erase(foo);
map.erase("");
EXPECT_TRUE(map.empty());
}
TEST(F14ValueMap, heterogeneousInsert) {
runHeterogeneousInsertTest<F14ValueMap<
Tracked<1>,
int,
TransparentTrackedHash<1>,
TransparentTrackedEqual<1>>>();
runHeterogeneousInsertStringTest<F14ValueMap<
std::string,
std::string,
transparent<hasher<StringPiece>>,
transparent<DefaultKeyEqual<StringPiece>>>>();
}
TEST(F14NodeMap, heterogeneousInsert) {
......@@ -1376,6 +1427,11 @@ TEST(F14NodeMap, heterogeneousInsert) {
int,
TransparentTrackedHash<1>,
TransparentTrackedEqual<1>>>();
runHeterogeneousInsertStringTest<F14NodeMap<
std::string,
std::string,
transparent<hasher<StringPiece>>,
transparent<DefaultKeyEqual<StringPiece>>>>();
}
TEST(F14VectorMap, heterogeneousInsert) {
......@@ -1384,6 +1440,11 @@ TEST(F14VectorMap, heterogeneousInsert) {
int,
TransparentTrackedHash<1>,
TransparentTrackedEqual<1>>>();
runHeterogeneousInsertStringTest<F14VectorMap<
std::string,
std::string,
transparent<hasher<StringPiece>>,
transparent<DefaultKeyEqual<StringPiece>>>>();
}
TEST(F14FastMap, heterogeneousInsert) {
......@@ -1392,6 +1453,11 @@ TEST(F14FastMap, heterogeneousInsert) {
int,
TransparentTrackedHash<1>,
TransparentTrackedEqual<1>>>();
runHeterogeneousInsertStringTest<F14FastMap<
std::string,
std::string,
transparent<hasher<StringPiece>>,
transparent<DefaultKeyEqual<StringPiece>>>>();
}
///////////////////////////////////
......
folly/container/test/F14SetTest.cpp
View file @ aebe18a7
......@@ -878,8 +878,8 @@ TEST(F14ValueSet, heterogeneous) {
constexpr auto world = "world"_sp;
F14ValueSet<std::string, Hasher, KeyEqual> set;
set.emplace(hello.str());
set.emplace(world.str());
set.emplace(hello);
set.emplace(world);
auto checks = [hello, buddy](auto& ref) {
// count
......@@ -1025,11 +1025,39 @@ void runHeterogeneousInsertTest() {
<< Tracked<1>::counts;
}
template <typename S>
void runHeterogeneousInsertStringTest() {
S set;
StringPiece k{"foo"};
std::vector<StringPiece> v{k};
set.insert(k);
set.insert("foo");
set.insert(StringPiece{"foo"});
set.insert(v.begin(), v.end());
set.insert(
std::make_move_iterator(v.begin()), std::make_move_iterator(v.end()));
set.emplace();
set.emplace(k);
set.emplace("foo");
set.emplace(StringPiece("foo"));
set.erase("");
set.erase(k);
set.erase(StringPiece{"foo"});
EXPECT_TRUE(set.empty());
}
TEST(F14ValueSet, heterogeneousInsert) {
runHeterogeneousInsertTest<F14ValueSet<
Tracked<1>,
TransparentTrackedHash<1>,
TransparentTrackedEqual<1>>>();
runHeterogeneousInsertStringTest<F14ValueSet<
std::string,
transparent<hasher<StringPiece>>,
transparent<DefaultKeyEqual<StringPiece>>>>();
}
TEST(F14NodeSet, heterogeneousInsert) {
......@@ -1037,6 +1065,10 @@ TEST(F14NodeSet, heterogeneousInsert) {
Tracked<1>,
TransparentTrackedHash<1>,
TransparentTrackedEqual<1>>>();
runHeterogeneousInsertStringTest<F14NodeSet<
std::string,
transparent<hasher<StringPiece>>,
transparent<DefaultKeyEqual<StringPiece>>>>();
}
TEST(F14VectorSet, heterogeneousInsert) {
......@@ -1044,6 +1076,10 @@ TEST(F14VectorSet, heterogeneousInsert) {
Tracked<1>,
TransparentTrackedHash<1>,
TransparentTrackedEqual<1>>>();
runHeterogeneousInsertStringTest<F14VectorSet<
std::string,
transparent<hasher<StringPiece>>,
transparent<DefaultKeyEqual<StringPiece>>>>();
}
TEST(F14FastSet, heterogeneousInsert) {
......@@ -1051,6 +1087,10 @@ TEST(F14FastSet, heterogeneousInsert) {
Tracked<1>,
TransparentTrackedHash<1>,
TransparentTrackedEqual<1>>>();
runHeterogeneousInsertStringTest<F14FastSet<
std::string,
transparent<hasher<StringPiece>>,
transparent<DefaultKeyEqual<StringPiece>>>>();
}
namespace {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment