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

[Keith Wyss via llvm-commits]

A for_each utility that automatically coverts to function_ref on the other
hamd could be useful.

On Sep 5, 2017 8:42 AM, "Keith Wyss" <wyssman at gmail.com> wrote:

> 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/3ac79977/attachment.html>