On Tue, 4 Oct 2022 at 15:09, Patrick Palka wrote: > > On Tue, 4 Oct 2022, Jonathan Wakely wrote: > > > On Tue, 4 Oct 2022 at 02:11, Patrick Palka via Libstdc++ > > <libstd...@gcc.gnu.org> wrote: > > > > > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? FWIW using > > > > OK, thanks. > > Thanks a lot, patch committed. > > > > > > variant<_PatternIter, _InnerIter> in the implementation means we need to > > > include <variant> from <ranges>, which increases the preprocessed size > > > of <ranges> by 3% (51.5k vs 53k). I suppose that's an acceptable cost? > > > > Yeah, I don't think we want to reimplement a lightweight std::variant, > > because that would just add even more code. > > Sounds good. > > > > > As I mentioned on IRC, maybe we could optimize the compilation time > > for some of the visitation using P2637R0, but that can be done later. > > Ah, I didn't consider the compile time impact of using std::visit. > Since we already use/instantiate std::get elsewhere in the implementation, > what do you think about doing the visitation manually via index() and > std::get like so? Seems to reduce compile time/memory usage for > join_with/1.cc by around 6% and doesn't look too messy since we're > dealing with only two alternatives. (And IIUC this should be equivalent > to std::visit wrt valueless_by_exception handling, since the call to > std::get<1> in each else branch will throw bad_variant_access for us > like std::visit would.)
Nice, 6% seems worth it, and I agree it's not too messy. Please check this in too!