[PATCH v1] libcamera: base: span: Explicitly default copy assignment

[Barnabás Pőcze]

Hi


2025. 03. 17. 22:53 keltezéssel, Kieran Bingham írta:
> Quoting Barnabás Pőcze (2025-03-10 17:03:56)
>> The `dynamic_extent` specialization is currently not trivially copyable
>> unlike its standard counterpart, `std::span`. This is because the copy
>> assignment operator is user-defined. Explicitly default it just like
>> it is done in the main template definition.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
>> ---
>>   include/libcamera/base/span.h | 8 +-------
>>   1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/include/libcamera/base/span.h b/include/libcamera/base/span.h
>> index 92cce4f05..806db106e 100644
>> --- a/include/libcamera/base/span.h
>> +++ b/include/libcamera/base/span.h
>> @@ -346,13 +346,7 @@ public:
>>          }
>>   
>>          constexpr Span(const Span &other) noexcept = default;
>> -
>> -       constexpr Span &operator=(const Span &other) noexcept
>> -       {
>> -               data_ = other.data_;
>> -               size_ = other.size_;
>> -               return *this;
>> -       }
>> +       constexpr Span &operator=(const Span &other) noexcept = default;
> 
> This sounds sane, as the default copy should do identically as above I
> think.
> 
> But wasn't there some rule of 5+-2 that says you're supposed to define
> all if you define one ?

Rule of 0/3/5: https://en.cppreference.com/w/cpp/language/rule_of_three

I think another option would be removing all special member functions and letting
the compiler implicitly define them (rule of 0). I guess I went with this change
because it was the simplest/shortest considering the current code.


> 
> Is there any ABI breakage in this patch ? Or will it compile to
> something directly compatible ?

I'm pretty sure even if it is theoretically an ABI break, it will
not be one in practice.


Regards,
Barnabás Pőcze

> 
> For the change itself:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> But if it's an ABI change I'd like to batch up all the ABI changes
> together.
> 
> --
> Kieran
> 
> 
>>   
>>          constexpr iterator begin() const { return data(); }
>>          constexpr const_iterator cbegin() const { return begin(); }
>> -- 
>> 2.48.1
>>