Search code examples
c++undefined-behaviorlifetimeobject-lifetimetemporary-objects

Avoiding undefined behaviour: temporary objects


I've written a class for using it as a convenient view e.g. in range-based fors. Overall, it is just a pair of iterators with bound checking:

template<typename I> class Range {
private:
  I begin;
  I end;

public:
  Range(I const begin, I const end)
    : begin(begin), end(end)
  {}

  Range<I>& skip(int const amount) {
    begin = std::min(begin + amount, end);
    return *this;
  }
};

template<typename C> auto whole(C const& container) {
  using Iterator = decltype(std::begin(container));
  return Range<Iterator>(std::begin(container), std::end(container));
}

Here's the intended usage of it (which led to UB):

std::vector<int> const vector{1, 2, 3};
for (int const value : whole(vector).skip(1)) {
  std::cout << value << ' ';
}

Removing the skip(1) part helps, same about the following refactoring of Range::skip:

Range<I> skip(int const amount) const {
  I const new_begin = std::min(begin + amount, end);
  return Range<I>(new_begin, end);
}

It seems like a temporary mustn't return a reference to itself. Here's what cppreference says:

All temporary objects are destroyed as the last step in evaluating the full-expression that (lexically) contains the point where they were created, and if multiple temporary objects were created, they are destroyed in the order opposite to the order of creation.

Though I'm not sure it's the case and I don't now how to interpret it practically. What's the actual problem and how can I reliably avoid such UB? Are alike expressions e.g. auto string = std::string("abc").append("def") also unsafe?


Solution

  • In ranged-based for loop the range-expression is bound to reference as (prototype code)

    auto && __range = whole(vector).skip(1) ;
    

    The problem is the temporary created by whole(vector) is destroyed immediately after the full expression, the reference __range (which binds to the returned reference from skip, i.e. the temporary) becomes dangled; after that any dereference on it leads to UB.

    auto string = std::string("abc").append("def") is fine, string gets copied, it's an independent object from the temporary std::string.

    Since C++20 you can add init-statement:

    If range_expression returns a temporary, its lifetime is extended until the end of the loop, as indicated by binding to the forwarding reference __range, but beware that the lifetime of any temporary within range_expression is not extended.

    E.g.

    std::vector<int> const vector{1, 2, 3};
    for (auto thing = whole(vector); int const value : thing.skip(1)) {
      std::cout << value << ' ';
    }