8

Does the following invoke undefined behavior?

#include <iostream>
#include <iomanip>
#include <algorithm>
#include <experimental/iterator>

int main() {
    long double values[] = {1, 2, 3};
    std::transform(
        std::begin(values), std::end(values),
        std::experimental::make_ostream_joiner(std::cout, ", "),
        [](long double v) {
            return std::put_money(v + 1);
        }
    );
    return 0;
}

My worry is that return std::put_money(v + 1) returns a reference to the temporary v + 1.

Eric
  • 95,302
  • 53
  • 242
  • 374
  • According to [iomanip.syn](http://eel.is/c++draft/iomanip.syn), the return value is an "unspecified implementation type." So the whether or not this example has UB will depend on your compiler and/or standard library implementation. You might need to dig into standard library source code to find out for your exact platform. – alter_igel Jun 07 '19 at 18:09
  • Is it a defect that the standard does not mandate any lifetime requirements? – Eric Jun 07 '19 at 18:13
  • 4
    Seems like `[](const int& i){...}` might work around any potential lifetime issues? – 0x5453 Jun 07 '19 at 18:18
  • 1
    I do not see anything in any part of standard which would specify lifetime of manipulator-returned objects. I think, it is under-specified. – SergeyA Jun 07 '19 at 18:24
  • 1
    @0x5453: Indeed, that would work, but doesn't help if I replace the body with `i + 1` – Eric Jun 07 '19 at 18:27
  • One hackish solution to certainly avoid this would be to create a `stringstream` inside the lambda, dump the result of `put_money` in there, and then return the stream's string contents. But that has all the overhead that comes with stringstream and would be somewhat inefficient. – alter_igel Jun 07 '19 at 18:35
  • Note `int` is an invalid argument to `std::put_money`; the argument must be either `long double` or some specialization of `std::basic_string`. It seems like for a `long double`, either implementation would be valid, since the only use of the saved argument is to pass it to `std::money_put::put`, which takes the number by value. Though the string version takes the string by reference, and some custom override of `money_put::do_put` might do something weird with its argument's address. – aschepler Jun 07 '19 at 18:51
  • @aschepler: Well, `int` can be converted to `long double`. Either way, updated for clarity. @0x5453, updated to make your point moot too. – Eric Jun 07 '19 at 20:09

3 Answers3

6

The standard ([ext.manip]/6) only defines this specific expression:

out << put_­money(mon, intl);

It is unspecified how mon is stored in the mean time, and it is definitely possible for it to become a dangling reference and be UB.

An "easy" fix is making your own class to know you store the value:

struct money_putter {
    long double value;

    template<class charT, class traits>
    friend std::basic_ostream<charT, traits>& operator<<(std::basic_ostream<charT, traits>& os, const money_putter& mon) {
        return os << std::put_money(mon.value);
    }
};


int main() {
    int values[] = {1, 2, 3};
    std::transform(
        std::begin(values), std::end(values),
        std::experimental::make_ostream_joiner(std::cout, ", "),
        [](int i)  {
            return money_putter{i};  // or i + 1
        }
    );
    return 0;
}
Artyer
  • 31,034
  • 3
  • 47
  • 75
  • I assume you could reduce the boilerplate here with a `lambda_putter` capturing `i` by value`, for want of a better name. – Eric Jun 07 '19 at 22:53
2

You could test it, though this wont tell you anything about whether it is guaranteed, but then as the return type of put_money is not specified, you cannot assume that the returned value does not hold a reference.

...anyhow let's test it:

#include <iostream>
#include <iomanip>
#include <algorithm>
#include <experimental/iterator>

int main() {
    int i = 42;
    std::cout << std::put_money(i) << "\n";
    auto x = std::put_money(i);
    i = 43;
    std::cout << x;    
    return 0;
}

Output with clang:

42
43

So actually the answer is positive. With clang the returned value does hold a reference and the output is the same with gcc. Hence, yes your code has UB.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
1

This answer does a great job at answering my question, but I thought I'd supply a more generic solution to the problem of ensuring the object output to an ostream_joiner takes no dangling references, one that uses a lambda to capture those references:

#include <type_traits>
#include <ostream>

template<typename F>
class put_invocation_t {
public:
    constexpr put_invocation_t(F const& f) : f(f) {}
    constexpr put_invocation_t(F&& f) : f(std::move(f)) {}
    template<class charT, class traits>
    friend std::basic_ostream<charT, traits>& operator<<(
        std::basic_ostream<charT, traits>& os, put_invocation_t const& pi
    ) {
        return pi.f(os);
    }
private:
    F f;
};

// or use a deduction guide in C++17
template<typename F>
put_invocation_t<std::decay_t<F>> put_invocation(F&& f) {
    return put_invocation_t<std::decay_t<F>>(std::forward<F>(f));
}

Used as

std::transform(
    std::begin(values), std::end(values),
    std::experimental::make_ostream_joiner(std::cout, ", "),
    [](long double v) {
        return put_invocation([=](auto& os) -> auto& {
            return os << std::put_money(v + 1);
        });
    }
);

This has the bonus of also scaling to outputing multiple values, by using something like the following within the transform:

return put_invocation([=](auto& os) -> auto& {
    return os << "Value is: " << std::put_money(v + 1);
});
Eric
  • 95,302
  • 53
  • 242
  • 374