https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115444
Bug ID: 115444
Summary: std::copy_n generates more code than needed
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: arthur.j.odwyer at gmail dot com
Target Milestone: ---
// https://godbolt.org/z/1Yo3h3rqW
#include <algorithm>
#include <compare>
struct It {
explicit It();
It& operator++();
It operator++(int);
It& operator--();
It operator--(int);
const int& operator*() const;
const int& operator[](int) const;
friend It operator+(int, const It&);
friend It operator+(const It&, int);
friend It operator-(const It&, int);
int operator-(const It&) const;
It& operator+=(int);
It& operator-=(int);
bool operator==(const It&) const;
std::strong_ordering operator<=>(const It&) const;
using difference_type = int;
using value_type = int;
using reference = int&;
using pointer = int*;
#ifdef LESS
using iterator_category = std::bidirectional_iterator_tag;
#endif
};
void simple_copy(It first, int n, int *dest) {
std::copy_n(first, n, dest);
}
Compiled with `-std=c++20 -DLESS`, this produces the codegen I'd expect: a
single loop calling `It::operator++()` and `It::operator*()` and no other
user-defined operations. That is, we pass in the `n`, it loops `n` times, we're
done. This is perfect.
But if you compile with `-std=c++20 -ULESS`, you get more verbose codegen:
first a call to `It::operator+(int)`, then a call to `It::operator-(It)`, and
then the same loop as before. That is, it appears that libstdc++ is "lowering"
`std::copy_n(first, n, dest)` into `std::copy(first, first+n, dest)` and then
"lowering" that back into `std::copy_n(first, first+n - first, dest)` before
finally getting to the loop. It would be nice not to do this.
This was reported on Slack by Alfredo Correa, whose iterators allegedly have
expensive (still O(1), but expensive) `+` and `-` operations.
Sidebar: Alfredo also reports that he's trying to use `copy_n` with iterators
where maybe `first + n` isn't even computable — like, he's expecting that the
following program should have well-defined behavior.
// https://godbolt.org/z/zfq36Ker6
struct Dynamite {
int value_;
void operator=(Dynamite d) {
if (d.value_ == -1) throw "Success!";
value_ = d.value_;
}
};
int main() {
const Dynamite s[4] = {{1},{2},{3},{-1}};
Dynamite d[4] = {};
try {
std::copy_n(s, 1000, d);
assert(false);
} catch (const char *s) {
assert(d[0].value_ == 1);
assert(d[1].value_ == 2);
assert(d[2].value_ == 3);
puts(s);
}
}
Today, this program forms the invalid pointer value (s+1000) inside libstdc++'s
`std::copy_n`. I think it's within its rights to do that, and thus the behavior
of this program is undefined by the standard (specifically,
https://eel.is/c++draft/alg.copy#12 doesn't claim that it *is* defined, so it's
undefined). But I'd still encourage libstdc++ to make programs like this Just
Work.
