[libcamera-devel,PATCH/RFC,0/3] libcamera: Simplify range-based for loop counters
mbox series

Message ID 20210423020932.2760-1-laurent.pinchart@ideasonboard.com
Headers show
Series
  • libcamera: Simplify range-based for loop counters
Related show

Message

Laurent Pinchart April 23, 2021, 2:09 a.m. UTC
Hello,

This RFC patch series is an attempt to simplify for loops that iterate
over a range, and require a loop counter.

The limitation of the range-based for loop is explained in patch 1/3,
which provides a new utils::enumerate() function to solve the problem.
The API is inspired by the C++23 views::enumerate proposal
(http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2164r1.pdf),
but implemented without the range library as that's a C++20 feature.

Patch 2/3 provides an alternative API that uses an aggregate with named
fields instead of an std::pair with structured binding. I currently have
a slight preference for the named fields, and if we agree that this API
is better, I'll squash 1/3 and 2/3.

Patch 3/3 is an example of how code can be simplified. It also outlines
a limitation of the API. Due to the C++ lifetime rules, we can't safely
pass a temporary to utils::enumerate() as the temporary would be
destroyed once utils::enumerate() returns but before the for loop
completes (actually before it even starts), resulting in use-after-free
errors. The issue is explained in more details in
http://josuttis.com/download/std/D2012R0_fix_rangebasedfor_201029.pdf.
The ensure that such bugs can't be introduced by mistake, the
utils::enumerate() function takes an lvalue reference only. The drawback
is that an loop that iterates over a temporary must now stores that
temporary in a named variable before the loop. On a side note, C++20
would bring us a workaround to declare that variable within the loop
statement. If anyone can think of a clever idea to fix this problem, I'm
all ears. One option may be to store a copy of the iterable in the
adapter when utils::enumerate() is given an rvalue, possibly with move
semantics, but it may still be costly in some cases, I'm not sure how
compilers would optimize that.

Laurent Pinchart (3):
  libcamera: utils: Add enumerate view for range-based for loops
  libcamera: utils: enumerate: Use named fields for result
  v4l2: Replace manual loop counters with utils::enumerate()

 include/libcamera/internal/utils.h | 92 ++++++++++++++++++++++++++++++
 src/libcamera/utils.cpp            | 30 ++++++++++
 src/v4l2/v4l2_compat_manager.cpp   | 18 +++---
 test/utils.cpp                     | 59 +++++++++++++++++++
 4 files changed, 190 insertions(+), 9 deletions(-)