Skip to content

Commit a76e6ef

Browse files
committed
Avoid unnecessary container copies; refresh stale precondition docs (#30)
Move local containers into the returned wrapper instead of copying them. Many non-mutating operations (filtered, sorted, removing_*, inserting_*, replace/replacing_range, map, set difference/union/intersect/map/zip, and the map removing/inserting/filtered helpers) built a local std::vector/std::set/ std::map and passed it as an lvalue into a wrapper constructor that has both a copy and a move overload, selecting the copy. They now std::move the local. Also: - vector::insert_back(T) / inserting_back(T) now push_back(std::move(value)) instead of copying the by-value parameter a second time. - set::keys() returns the local directly instead of std::move(vec), which was a pessimizing move that defeated NRVO. Docs: the operator[] / map const operator[] comments still claimed bounds checking was "enabled for debug builds". After the precondition changes the checks are always on (abort in debug and release); updated the comments and mention FCPP_NO_PRECONDITION_CHECKS.
1 parent 7955eab commit a76e6ef

3 files changed

Lines changed: 40 additions & 35 deletions

File tree

‎include/map.h‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ class map
452452
++it;
453453
}
454454
}
455-
return map(copy);
455+
return map(std::move(copy));
456456
}
457457

458458
// Executes the given operation for each key/value pair in the map. The operation must not
@@ -543,7 +543,7 @@ class map
543543
{
544544
auto copy(m_map);
545545
copy.erase(key);
546-
return map(copy);
546+
return map(std::move(copy));
547547
}
548548

549549
// Inserts a key/value pair in place (mutating). If an equivalent key already exists, the
@@ -583,7 +583,7 @@ class map
583583
{
584584
auto copy(m_map);
585585
copy.insert(std::make_pair(key, value));
586-
return map(copy);
586+
return map(std::move(copy));
587587
}
588588

589589
// Returns a copy of this instance with the key/value pair inserted (non-mutating). If an
@@ -592,7 +592,7 @@ class map
592592
{
593593
auto copy(m_map);
594594
copy.insert(element);
595-
return map(copy);
595+
return map(std::move(copy));
596596
}
597597

598598
// Removes all key/value pairs from the map (mutating)
@@ -651,8 +651,9 @@ class map
651651
return m_map.end();
652652
}
653653

654-
// Returns a reference to the value that is mapped to a key,
655-
// assuming that such key already exists.
654+
// Returns a reference to the value that is mapped to a key, which must already exist.
655+
// If the key is not present the program calls std::abort() (in both debug and release
656+
// builds). Define FCPP_NO_PRECONDITION_CHECKS to disable the check.
656657
const TValue& operator[](const TKey& key) const
657658
{
658659
const auto it = m_map.find(key);

‎include/set.h‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ namespace fcpp {
518518
other.begin(),
519519
other.end(),
520520
std::inserter(diff, diff.begin()));
521-
return set(diff);
521+
return set(std::move(diff));
522522
}
523523

524524
[[nodiscard]] set difference_with(const std::set<TKey, TCompare>& other) const
@@ -545,7 +545,7 @@ namespace fcpp {
545545
other.begin(),
546546
other.end(),
547547
std::inserter(combined, combined.begin()));
548-
return set(combined);
548+
return set(std::move(combined));
549549
}
550550

551551
[[nodiscard]] set union_with(const std::set<TKey, TCompare>& other) const
@@ -572,7 +572,7 @@ namespace fcpp {
572572
other.begin(),
573573
other.end(),
574574
std::inserter(intersection, intersection.begin()));
575-
return set(intersection);
575+
return set(std::move(intersection));
576576
}
577577

578578
[[nodiscard]] set intersect_with(const std::set<TKey, TCompare>& other) const
@@ -656,7 +656,7 @@ namespace fcpp {
656656
for (const auto& key : m_set) {
657657
transformed_set.insert(transform(key));
658658
}
659-
return set<UKey, UCompare>(transformed_set);
659+
return set<UKey, UCompare>(std::move(transformed_set));
660660
}
661661

662662
// Returns true if all keys match the predicate (return true)
@@ -830,7 +830,7 @@ namespace fcpp {
830830
copy.insert(*it);
831831
}
832832
}
833-
return set(copy);
833+
return set(std::move(copy));
834834
}
835835

836836
#ifdef CPP17_AVAILABLE
@@ -928,7 +928,7 @@ namespace fcpp {
928928
for_each([&vec](const TKey& key){
929929
vec.insert_back(key);
930930
});
931-
return std::move(vec);
931+
return vec;
932932
}
933933

934934
// Removes an element from the set, if it exists, potentially changing the set's contents (mutating)
@@ -958,7 +958,7 @@ namespace fcpp {
958958
{
959959
auto copy(m_set);
960960
copy.erase(element);
961-
return set(copy);
961+
return set(std::move(copy));
962962
}
963963

964964
// Inserts an element in the set, if it does not already exist, potentially changing the set's contents (mutating)
@@ -988,7 +988,7 @@ namespace fcpp {
988988
{
989989
auto copy(m_set);
990990
copy.insert(element);
991-
return set(copy);
991+
return set(std::move(copy));
992992
}
993993

994994
// Removes all keys from the set (mutating)
@@ -1089,7 +1089,8 @@ namespace fcpp {
10891089
}
10901090

10911091
// Returns a copy of the key at the given sorted position.
1092-
// Bounds checking (assert) is enabled for debug builds.
1092+
// Bounds checking is always enabled (in both debug and release builds): an out-of-bounds
1093+
// index calls std::abort(). Define FCPP_NO_PRECONDITION_CHECKS to disable the check.
10931094
// Performance is O(n), so be careful for performance critical code sections.
10941095
TKey operator[](size_t index)
10951096
{
@@ -1100,7 +1101,8 @@ namespace fcpp {
11001101
}
11011102

11021103
// Returns a copy of the key at the given sorted position.
1103-
// Bounds checking (assert) is enabled for debug builds.
1104+
// Bounds checking is always enabled (in both debug and release builds): an out-of-bounds
1105+
// index calls std::abort(). Define FCPP_NO_PRECONDITION_CHECKS to disable the check.
11041106
// Performance is O(n), so be careful for performance critical code sections.
11051107
TKey operator[](size_t index) const
11061108
{
@@ -1180,7 +1182,7 @@ namespace fcpp {
11801182
for (; it1 != end() && it2 != set_end; ++it1, ++it2) {
11811183
combined_set.insert({*it1, *it2});
11821184
}
1183-
return set<std::pair<TKey, UKey>>(combined_set);
1185+
return set<std::pair<TKey, UKey>>(std::move(combined_set));
11841186
}
11851187
};
11861188

‎include/vector.h‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ namespace fcpp {
454454
m_vector.end(),
455455
std::back_inserter(transformed_vector),
456456
std::forward<Transform>(transform));
457-
return vector<U>(transformed_vector);
457+
return vector<U>(std::move(transformed_vector));
458458
}
459459

460460
#ifdef PARALLEL_ALGORITHM_AVAILABLE
@@ -470,7 +470,7 @@ namespace fcpp {
470470
m_vector.cend(),
471471
transformed_vector.begin(),
472472
std::forward<Transform>(transform));
473-
return vector<U>(transformed_vector);
473+
return vector<U>(std::move(transformed_vector));
474474
}
475475
#endif
476476

@@ -704,7 +704,7 @@ namespace fcpp {
704704
m_vector.end(),
705705
std::back_inserter(filtered_vector),
706706
std::forward<Callable>(predicate_to_keep));
707-
return vector(filtered_vector);
707+
return vector(std::move(filtered_vector));
708708
}
709709

710710
#ifdef PARALLEL_ALGORITHM_AVAILABLE
@@ -726,7 +726,7 @@ namespace fcpp {
726726
m_vector.end(),
727727
std::back_inserter(filtered_vector),
728728
std::forward<Callable>(predicate_to_keep));
729-
return vector(filtered_vector);
729+
return vector(std::move(filtered_vector));
730730
#endif
731731
}
732732
#endif
@@ -999,7 +999,7 @@ namespace fcpp {
999999
std::sort(sorted_vector.begin(),
10001000
sorted_vector.end(),
10011001
std::forward<Sortable>(comparison_predicate));
1002-
return vector(sorted_vector);
1002+
return vector(std::move(sorted_vector));
10031003
}
10041004

10051005
#ifdef PARALLEL_ALGORITHM_AVAILABLE
@@ -1013,7 +1013,7 @@ namespace fcpp {
10131013
sorted_vector.begin(),
10141014
sorted_vector.end(),
10151015
std::forward<Sortable>(comparison_predicate));
1016-
return fcpp::vector(sorted_vector);
1016+
return fcpp::vector(std::move(sorted_vector));
10171017
}
10181018
#endif
10191019

@@ -1192,7 +1192,7 @@ namespace fcpp {
11921192
assert_smaller_size(index);
11931193
auto copy(m_vector);
11941194
copy.erase(copy.begin() + index);
1195-
return vector(copy);
1195+
return vector(std::move(copy));
11961196
}
11971197

11981198
// Removes the last element, if present (mutating)
@@ -1221,7 +1221,7 @@ namespace fcpp {
12211221
{
12221222
auto copy(m_vector);
12231223
copy.pop_back();
1224-
return vector(copy);
1224+
return vector(std::move(copy));
12251225
}
12261226

12271227
// Removes the first element, if present (mutating)
@@ -1294,7 +1294,7 @@ namespace fcpp {
12941294
auto shorter_vector(m_vector);
12951295
shorter_vector.erase(shorter_vector.begin() + range.start,
12961296
shorter_vector.begin() + range.start + range.count);
1297-
return vector(shorter_vector);
1297+
return vector(std::move(shorter_vector));
12981298
}
12991299

13001300
// Inserts an element at the given index, therefore changing the vector's contents (mutating)
@@ -1325,7 +1325,7 @@ namespace fcpp {
13251325
assert_smaller_or_equal_size(index);
13261326
auto copy(m_vector);
13271327
copy.insert(copy.begin() + index, element);
1328-
return vector(copy);
1328+
return vector(std::move(copy));
13291329
}
13301330

13311331
// Inserts a range of elements starting at the given index, therefore changing the vector's contents (mutating)
@@ -1422,7 +1422,7 @@ namespace fcpp {
14221422
// numbers -> fcpp::vector({1, 4, 2, 5, 8, 3, 1, 7, 1, 18});
14231423
vector& insert_back(T value)
14241424
{
1425-
m_vector.push_back(value);
1425+
m_vector.push_back(std::move(value));
14261426
return *this;
14271427
}
14281428

@@ -1450,8 +1450,8 @@ namespace fcpp {
14501450
[[nodiscard]] vector inserting_back(T value) const
14511451
{
14521452
auto augmented_vector(m_vector);
1453-
augmented_vector.push_back(value);
1454-
return vector(augmented_vector);
1453+
augmented_vector.push_back(std::move(value));
1454+
return vector(std::move(augmented_vector));
14551455
}
14561456

14571457
// Makes a copy of the vector, inserts value at the beginning of the copy and returns the copy (non-mutating)
@@ -1824,15 +1824,17 @@ namespace fcpp {
18241824
}
18251825

18261826
// Returns a reference to the element in the given index, allowing subscripting and value editing.
1827-
// Bounds checking (assert) is enabled for debug builds.
1827+
// Bounds checking is always enabled (in both debug and release builds): an out-of-bounds
1828+
// index calls std::abort(). Define FCPP_NO_PRECONDITION_CHECKS to disable the check.
18281829
T& operator[](size_t index)
18291830
{
18301831
assert_smaller_size(index);
18311832
return m_vector[index];
18321833
}
18331834

18341835
// Returns a constant reference to the element in the given index, allowing subscripting.
1835-
// Bounds checking (assert) is enabled for debug builds.
1836+
// Bounds checking is always enabled (in both debug and release builds): an out-of-bounds
1837+
// index calls std::abort(). Define FCPP_NO_PRECONDITION_CHECKS to disable the check.
18361838
const T& operator[](size_t index) const
18371839
{
18381840
assert_smaller_size(index);
@@ -1912,7 +1914,7 @@ namespace fcpp {
19121914
augmented_vector.insert(augmented_vector.end(),
19131915
vec_begin,
19141916
vec_end);
1915-
return vector(augmented_vector);
1917+
return vector(std::move(augmented_vector));
19161918
}
19171919

19181920
#ifdef CPP17_AVAILABLE
@@ -1928,7 +1930,7 @@ namespace fcpp {
19281930
augmented_vector.insert(augmented_vector.begin(),
19291931
vec_begin,
19301932
vec_end);
1931-
return vector(augmented_vector);
1933+
return vector(std::move(augmented_vector));
19321934
}
19331935

19341936
#ifdef CPP17_AVAILABLE
@@ -2030,7 +2032,7 @@ namespace fcpp {
20302032
std::copy(vec_begin,
20312033
vec_end,
20322034
replaced_vector.begin() + index);
2033-
return vector(replaced_vector);
2035+
return vector(std::move(replaced_vector));
20342036
}
20352037

20362038
void assert_smaller_size(size_t index) const

0 commit comments

Comments
 (0)