[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982
Jan van Dijk changed:
What|Removed |Added
CC||j.v.dijk at tue dot nl
--- Comment #3 from Jan van Dijk ---
Isn't the real problem that the standard does not specify any requirements
about the type of the count argument (or a precondition on its value)?
Should that be a built-in integral type? Then the issue is solved by doing a
static_assert> to the implementation.
One could also argue that the counter merely be convertible to a std::size_t,
and the question (to WG21) is why then the count type is a template argument in
the first place. (One could also argue that it should be a std::ptrdiff_t,
since the algorithm really operates on a range [ptr,ptr+count] --- that would
also allow an assert(count>=0) in the implementation.)
If the intention is really that also user-defined types are supported, the
requirements on such types should be spelled out (by WG21). As an example, the
code below uses a custom counter object that can can be converted to (=> and
compared with) an integer value. Reasonable enough. However, it does not
compile because of what really seems to be an undocumented implementation
detail in stl_algo.h: the usage of decltype(__n + 0) to compute a counter type
for internal usage. Is the code below valid or not?
It is remarkable how much Sunday morning can be spent on such an innocuous
issue :-)
cat 87892_2.cpp
#include
struct counter
{
counter(unsigned n);
operator unsigned() const;
counter& operator--();
private:
template counter operator+(T v) const;
};
void foo()
{
int a[2];
std::generate_n(a, counter(2), []{ return 0;});
}
g++ -c 87892_2.cpp
In file included from /home/jan/local/gcc-head/include/c++/9.0.1/algorithm:62,
from 87892_2.cpp:1:
/home/jan/local/gcc-head/include/c++/9.0.1/bits/stl_algo.h: In instantiation of
‘_OIter std::generate_n(_OIter, _Size, _Generator) [with _OIter = int*; _Size =
counter; _Generator = foo()::]’:
87892_2.cpp:15:48: required from here
/home/jan/local/gcc-head/include/c++/9.0.1/bits/stl_algo.h:4448:27: error:
‘counter counter::operator+(T) const [with T = int]’ is private within this
context
4448 | for (__decltype(__n + 0) __niter = __n;
| ^~~
87892_2.cpp:9:29: note: declared private here
9 | template counter operator+(T v) const;
|
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 Jonathan Wakely changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |redi at gcc dot gnu.org --- Comment #4 from Jonathan Wakely --- The requirement is that Size can be converted to an integer type (so it would be wrong to assert it is an integer type). I think converting it to the iterator's difference type is the right fix. Doing that would make the original example ill-formed, because the pointer isn't convertible to ptrdiff_t. Now we're in stage 1 I'll make that change.
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 --- Comment #5 from Jan van Dijk --- Thanks a lot for this change. One more nit: the standard clause 28.6.7(2) allows (== does not forbid) negative count arguments, in which case generate_n is a no-op returning __first, but this is not reflected by the libstdc++ documentation of the return value, which claims __first+__n unconditionally. Somewhat off-topic: IMHO the standard is not explicit about the fact that gen() is (of course) to be invoked separately for every element in the range: "The generate_n algorithms invoke the function object gen and assign the return value of gen through all the iterators in the range...". But that may be me not being a native English speaker. The libstdc++ docs are more helpful for sure. And indeed, sorry for not being sufficiently precise: the standard does not say to *which* integral type the conversion must be possible so that type could be used internally, or better: simply used in the interface. Having the _Size template argument allows, in principle, the loop to be written without the conversion being actually done, but that would be such a pico-optimization that I still do not understand why the standard did not just fix the count type to (e.g.) std::ptrdiff_t --- but also that is outside the scope of this issue.
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 --- Comment #6 from Jonathan Wakely --- (In reply to Jan van Dijk from comment #5) > Somewhat off-topic: IMHO the standard is not explicit about the fact that > gen() is (of course) to be invoked separately for every element in the range: > "The generate_n algorithms invoke the function object gen and assign the > return value of gen through all the iterators in the range...". But that may > be me not being a native English speaker. The libstdc++ docs are more > helpful for sure. The current working draft is clear: Assigns the result of successive evaluations of gen() through each iterator in the range [first, first + N).
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 --- Comment #7 from Jonathan Wakely --- (In reply to Jonathan Wakely from comment #4) > I think converting it to the iterator's difference type is the right fix. > Doing that would make the original example ill-formed, because the pointer > isn't convertible to ptrdiff_t. Except that output iterators are allowed to defined difference_type as void.
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 --- Comment #8 from Jonathan Wakely --- Author: redi Date: Mon Apr 29 12:12:43 2019 New Revision: 270646 URL: https://gcc.gnu.org/viewcvs?rev=270646&root=gcc&view=rev Log: PR libstdc++/87982 Fix generate_n and fill_n use of _Size parameter The standard only requires that _Size can be converted to an integral type, not that it can be used for arithmetic. Add a new set of __size_to_integer helper functions to do the conversion (which will be ambiguous if there is no one conversion that is better than any others). Also add tests for DR 426 which requires these algorithms and search_n to handle negative values of n. PR libstdc++/87982 * include/bits/stl_algo.h (generate_n): Convert _Size parameter to an integral type. * include/bits/stl_algobase.h (__size_to_integer): New overloaded functions to convert a value to an integral type. (__fill_n_a, __fill_n_a): Assert that __n is already an integral type. (fill_n): Convert _Size parameter to an integral type. * testsuite/25_algorithms/fill_n/dr426.cc: New test. * testsuite/25_algorithms/generate_n/87982.cc: New test. * testsuite/25_algorithms/generate_n/dr426.cc: New test. Added: trunk/libstdc++-v3/testsuite/25_algorithms/fill_n/87982.cc trunk/libstdc++-v3/testsuite/25_algorithms/fill_n/87982_neg.cc trunk/libstdc++-v3/testsuite/25_algorithms/fill_n/dr426.cc trunk/libstdc++-v3/testsuite/25_algorithms/generate_n/87982.cc trunk/libstdc++-v3/testsuite/25_algorithms/generate_n/87982_neg.cc trunk/libstdc++-v3/testsuite/25_algorithms/generate_n/dr426.cc Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/include/bits/stl_algo.h trunk/libstdc++-v3/include/bits/stl_algobase.h
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 Jonathan Wakely changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED Target Milestone|--- |10.0 --- Comment #9 from Jonathan Wakely --- Fixed on trunk.
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 --- Comment #10 from Jan van Dijk --- Thanks a lot. And sorry for being pedantic, but I believe that the documentation of the return value of generate_n is still wrong for negative __n (see the first part of comment #5).
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 --- Comment #11 from Jonathan Wakely --- Addressed in r270651
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 Jonathan Wakely changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2018-11-12 Ever confirmed|0 |1 --- Comment #1 from Jonathan Wakely --- If we change the literal 0 to something like (int)0 then it can't be used in comparisons with pointers: /home/jwakely/gcc/9/include/c++/9.0.0/bits/stl_algo.h:4449:13: error: ISO C++ forbids comparison between pointer and integer [-fpermissive] 4449 | __niter > (int)0; --__niter, (void) ++__first) | ^~~~
[Bug libstdc++/87982] No error for std::generate_n(ptr, ptr, f)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87982 --- Comment #2 from Jonathan Wakely --- std::fill_n has the same problem (in both overloads of __fill_n_a).
