[libcxx-commits] [PATCH] D119589: [libc++][ranges] Implement ranges::min

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.min.max/ranges.min.pass.cpp:79
+    assert(ret == 5);
+  }
+  { // test comparator
----------------
jwakely wrote:
> Quuxplusone wrote:
> > >>! In D119589#3339433, @philnik wrote:
> > > Does someone see why https://reviews.llvm.org/harbormaster/unit/view/2761914/ fails?
> > ```
> > __iterator/projected.h:28:34: error: 'std::__1::indirect_result_t<_Proj&, _It> std::__1::projected<_It, _Proj>::operator*() const [with _It = const test_initializer_list()::<lambda(int, int)>*; _Proj = std::__1::identity; std::__1::indirect_result_t<_Proj&, _It> = const test_initializer_list()::<lambda(int, int)>&]', declared using local type 'const test_initializer_list()::<lambda(int, int)>', is used but never defined [-fpermissive]
> >    28 |   indirect_result_t<_Proj&, _It> operator*() const; // not defined
> >       |                                  ^~~~~~~~
> > ```
> > 
> > Kind of, but I think it's a GCC bug. GCC is complaining about the use of the "local type" `decltype(comparator)`. We're instantiating `projected<int*, decltype(comparator)>` and declaring its `operator*`, but there's no way that `operator*` can ever be defined outside of this TU, and it's also not defined inside this TU, so that's a problem... at least for GCC, for some reason. (That is, I think GCC is mistakenly considering `operator*` to have been ODR-used, even though we're only using it in non-evaluated contexts AFAICT.)
> > I have not been able to reproduce the problem on Godbolt's version(s) of GCC. https://godbolt.org/z/hs8fTodM9 Can anyone else reproduce it?
> > (also attn @jwakely in case he has any ideas)
> Looks like a similar problem to https://gcc.gnu.org/PR92894 (and its https://gcc.gnu.org/PR94241 dup) which we decided was a problem with using a deduced return type in the library code, not a compiler bug.
Ah, I (think I) get it. I've posted D120417 to check my guess.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119589/new/

https://reviews.llvm.org/D119589