[PATCH] D37417: Add range based wrapper around std::for_each.

[Keith Wyss via llvm-commits]

I was thinking in the case where one has a function, member function, or
std::function already on hand that using "for_each(Range, Fn);" is
succinct, but when digging into the details I think this makes it too easy
to copy std::function objects or lambdas with value captures unnecessarily.

Here are some example use cases and why I'm convinced you're right and
range based for is a more reliable syntax:

void doStuff(const Parameter& P); // doStuff may be a free function
declaration
std::function<void (const Parameter&)> doStuff; // It might be a
std::function

for (const Parameter &P : Parameters) // <-- Can expand copies here
accidentally
   doStuff(P);

for_each(Parameters, doStuff); // <-- One less declaration.
                                                  // Less chance to copy
the values from the iterators, but will
                                                  // pass doStuff by value.
Prefer range based for.

for_each(Parameters, [&](const auto &P) -> void { doStuff(P) });
// Less readable than range based for and copies the lamba object. I prefer
range based for.



On Mon, Sep 4, 2017 at 6:32 PM, David Blaikie <dblaikie at gmail.com> wrote:

> Not sure I follow - you have to write the type name in the parameter or in
> the range-based-for, similar mistakes in either case, right?
>
> On Mon, Sep 4, 2017 at 5:50 PM Keith Wyss <wyssman at gmail.com> wrote:
>
>> I like for_each because it offers one less place to mistakenly capture by
>> value.
>>
>> On Sep 4, 2017 1:22 PM, "David Blaikie" <dblaikie at gmail.com> wrote:
>>
>>>
>>>
>>> On Mon, Sep 4, 2017 at 1:16 PM Keith Wyss <wyssman at gmail.com> wrote:
>>>
>>>> It is an equivalent concept to range based for loop. Its not strictly
>>>> necessary, but neither is std::for_each.
>>>>
>>>
>>> Right - but std::for_each existed before the range based for loop (&
>>> admittedly, before lambdas too). I'm not sure of the motivation for this
>>> given C++11 features/today's code.
>>>
>>>
>>>>
>>>> On Sep 4, 2017 9:20 AM, "David Blaikie" <dblaikie at gmail.com> wrote:
>>>>
>>>>> What use case do you have for this over a range-based-for loop?
>>>>>
>>>>> On Sat, Sep 2, 2017 at 6:08 PM Keith via Phabricator via llvm-commits <
>>>>> llvm-commits at lists.llvm.org> wrote:
>>>>>
>>>>>> kpw created this revision.
>>>>>>
>>>>>> There are a few range based wrappers llvm provides already. They're
>>>>>> handy and I
>>>>>> couldn't find one for std::for_each, so I'm adding it.
>>>>>>
>>>>>>
>>>>>> https://reviews.llvm.org/D37417
>>>>>>
>>>>>> Files:
>>>>>>   include/llvm/ADT/STLExtras.h
>>>>>>   unittests/ADT/STLExtrasTest.cpp
>>>>>>
>>>>>>
>>>>>> Index: unittests/ADT/STLExtrasTest.cpp
>>>>>> ===================================================================
>>>>>> --- unittests/ADT/STLExtrasTest.cpp
>>>>>> +++ unittests/ADT/STLExtrasTest.cpp
>>>>>> @@ -318,4 +318,14 @@
>>>>>>    EXPECT_EQ(7, V[3]);
>>>>>>  }
>>>>>>
>>>>>> +TEST(STLExtrasTest, ForEach) {
>>>>>> +  std::vector<std::pair<int, int>> V = {{1, 2}, {3, 4}, {5, 6}};
>>>>>> +  std::vector<int> Evens{};
>>>>>> +  auto BackInserter = std::back_inserter(Evens);
>>>>>> +  for_each(V,
>>>>>> +           [&](std::pair<int, int> Element) { BackInserter =
>>>>>> Element.second; });
>>>>>> +  std::vector<int> ExpectedEvens = {2, 4, 6};
>>>>>> +  EXPECT_EQ(Evens, ExpectedEvens);
>>>>>> +}
>>>>>> +
>>>>>>  }
>>>>>> Index: include/llvm/ADT/STLExtras.h
>>>>>> ===================================================================
>>>>>> --- include/llvm/ADT/STLExtras.h
>>>>>> +++ include/llvm/ADT/STLExtras.h
>>>>>> @@ -852,6 +852,13 @@
>>>>>>    return std::find_if_not(std::begin(Range), std::end(Range), P);
>>>>>>  }
>>>>>>
>>>>>> +/// Provide wrapper to std::for_each which take ranges instead of
>>>>>> having to
>>>>>> +/// pass begin/end explicitly.
>>>>>> +template <typename R, typename UnaryFunction>
>>>>>> +UnaryFunction for_each(R &&Range, UnaryFunction F) {
>>>>>> +  return std::for_each(std::begin(Range), std::end(Range), F);
>>>>>> +}
>>>>>> +
>>>>>>  /// Provide wrappers to std::remove_if which take ranges instead of
>>>>>> having to
>>>>>>  /// pass begin/end explicitly.
>>>>>>  template <typename R, typename UnaryPredicate>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170905/4fbd9ee3/attachment.html>