RetroSearch Browse

Home - News ( United States | United Kingdom | Italy | Germany ) - Football scores

Showing content from https://cplusplus.github.io/LWG/issue4121 below:

to constructs associative containers via c.emplace(c.end(), *it)

This page is a snapshot from the LWG issues list, see the Library Active Issues List for more information and the meaning of New status.

4121. ranges::to constructs associative containers via c.emplace(c.end(), *it)

Section: 25.5.7.1 [range.utility.conv.general] Status: New Submitter: Hewill Kang Opened: 2024-07-16 Last modified: 2025-03-17

Priority: 2

View all issues with New status.

Discussion:

When ranges::to constructs an associative container, if there is no range version constructor for C and the input range is not common range, ranges::to will dispatch to the bullet 25.5.7.2 [range.utility.conv.to] (2.1.4), which first default-constructs the container and emplaces the element through c.emplace(c.end(), *it).

However, this is not the correct way to call emplace() on an associative container as it does not expect an iterator as the first argument, and since map::emplace(), for instance, is not constrained, which turns out a hard error because we are trying to make a pair with {it, pair}. Given that libstdc++ currently does not implement the range constructor for associative containers, the following illustrates the issue:

#include <ranges>
#include <set>
  
auto s = std::views::iota(0)
       | std::views::take(5)
       | std::ranges::to<std::set>(); // hard error

The proposed resolution simply removes the emplace() branch. Although this means that we always use insert() to fill associative containers, such an impact seems negligible.

[2024-07-23; This was caused by LWG 4016⁽ⁱ⁾.]

[2024-08-02; Reflector poll]

Set priority to 2 after reflector poll. "Would like to preserve the ability to use emplace. Tim suggested trying emplace_hint first, then emplace." "I tried it, it gets very verbose, because we might also want to try insert(*it) instead of insert(c.end(), *it) if emplace(*it) is not valid for associative containers, because c.end() might not be a good hint." "It might be suboptimal, but it still works."

Previous resolution [SUPERSEDED]:

This wording is relative to N4986.

Modify 25.5.7.1 [range.utility.conv.general] as indicated:

-4- Let container-appendable be defined as follows:

template<class Container, class Ref>
constexpr bool container-appendable =         // exposition only
  requires(Container& c, Ref&& ref) {
           requires (requires { c.emplace_back(std::forward<Ref>(ref)); } ||
                     requires { c.push_back(std::forward<Ref>(ref)); } ||
                     requires { c.emplace(c.end(), std::forward<Ref>(ref)); } ||
                     requires { c.insert(c.end(), std::forward<Ref>(ref)); });
};

-5- Let container-append be defined as follows:

template<class Container>
constexpr auto container-append(Container& c) {     // exposition only
  return [&c]<class Ref>(Ref&& ref) {
    if constexpr (requires { c.emplace_back(declval<Ref>()); })
      c.emplace_back(std::forward<Ref>(ref));
    else if constexpr (requires { c.push_back(declval<Ref>()); })
      c.push_back(std::forward<Ref>(ref));
    else if constexpr (requires { c.emplace(c.end(), declval<Ref>()); })
      c.emplace(c.end(), std::forward<Ref>(ref));
    else
      c.insert(c.end(), std::forward<Ref>(ref));
  };
};

[2025-03-13; Jonathan provides improved wording for Tim's suggestion]

It's true that for some cases it might be optimal to use c.emplace(ref) instead of c.emplace_hint(c.end(), ref) but I don't think I care. A bad hint is not expensive, it's just an extra comparison then the hint is ignored. And this code path isn't going to be used for std::set or std::map, only for user-defined associative containers that don't have a from_range_t constructor or a C(Iter,Sent) constructor. I think just fixing the original issue is all we need, rather than trying to handle every possible way to insert elements.

This is a simpler, portable reproducer that doesn't depend on the current implementation status of std::set in libstdc++:


#include <ranges>
#include <set>
struct Set : std::set<int> {
  Set() { }; // No other constructors
};
int main() {
  int a[1];
  auto okay = std::ranges::to<std::set<int>>(a);
  auto ohno = std::ranges::to<Set>(a);
}

Proposed resolution:

This wording is relative to N5001.

Modify 25.5.7.1 [range.utility.conv.general] as indicated:

-4- Let container-appendable be defined as follows:

template<class Container, class Ref>
constexpr bool container-appendable =         // exposition only
  requires(Container& c, Ref&& ref) {
           requires (requires { c.emplace_back(std::forward<Ref>(ref)); } ||
                     requires { c.push_back(std::forward<Ref>(ref)); } ||
                     requires { c.emplace_hint(c.end(), std::forward<Ref>(ref)); } ||
                     requires { c.emplace(c.end(), std::forward<Ref>(ref)); } ||
                     requires { c.insert(c.end(), std::forward<Ref>(ref)); });
};

-5- Let container-append be defined as follows:

template<class Container>
constexpr auto container-append(Container& c) {     // exposition only
  return [&c]<class Ref>(Ref&& ref) {
    if constexpr (requires { c.emplace_back(declval<Ref>()); })
      c.emplace_back(std::forward<Ref>(ref));
    else if constexpr (requires { c.push_back(declval<Ref>()); })
      c.push_back(std::forward<Ref>(ref));
    else if constexpr (requires { c.emplace_hint(c.end(), declval<Ref>()); })
      c.emplace_hint(c.end(), std::forward<Ref>(ref));
    else if constexpr (requires { c.emplace(c.end(), declval<Ref>()); })
      c.emplace(c.end(), std::forward<Ref>(ref));
    else
      c.insert(c.end(), std::forward<Ref>(ref));
  };
};

RetroSearch is an open source project built by @garambo | Open a GitHub Issue

Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo

HTML: 3.2 | Encoding: UTF-8 | Version: 0.7.4