| Message ID | 20241204145422.968446-1-pobrn@protonmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, Thank you for the patch. On Wed, Dec 04, 2024 at 02:54:24PM +0000, Barnabás Pőcze wrote: > If `cpp_debugstl=true` in the build configuration, then libstdc++ will > try to use `operator==` and the build will fail. I didn't know about cpp_debugstl. Is that something we should enable in CI debug builds ? > Implement `operator==` in terms of `operator!=` to avoid the build failure. > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > --- > include/libcamera/base/utils.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h > index 957150cb..782d5c97 100644 > --- a/include/libcamera/base/utils.h > +++ b/include/libcamera/base/utils.h > @@ -205,6 +205,10 @@ public: > iterator &operator++(); > std::string operator*() const; > bool operator!=(const iterator &other) const; > + bool operator==(const iterator &other) const > + { > + return !(*this != other); > + } Should we make operator!=() inline while at it ? It's a trivial function, and it would let the compiler optimize the operator==() implementation. Actually, maybe we should instead define operator==() as the canonical comparison operator, and implemente operator!=() as !operator==(). I think that's what we usually do. > > private: > const StringSplitter *ss_;
Hi 2024. december 4., szerda 17:21 keltezéssel, Laurent Pinchart <laurent.pinchart@ideasonboard.com> írta: > Hi Barnabás, > > Thank you for the patch. > > On Wed, Dec 04, 2024 at 02:54:24PM +0000, Barnabás Pőcze wrote: > > If `cpp_debugstl=true` in the build configuration, then libstdc++ will > > try to use `operator==` and the build will fail. > > I didn't know about cpp_debugstl. Is that something we should enable in > CI debug builds ? I believe so, yes. > > > Implement `operator==` in terms of `operator!=` to avoid the build failure. > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > --- > > include/libcamera/base/utils.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h > > index 957150cb..782d5c97 100644 > > --- a/include/libcamera/base/utils.h > > +++ b/include/libcamera/base/utils.h > > @@ -205,6 +205,10 @@ public: > > iterator &operator++(); > > std::string operator*() const; > > bool operator!=(const iterator &other) const; > > + bool operator==(const iterator &other) const > > + { > > + return !(*this != other); > > + } > > Should we make operator!=() inline while at it ? It's a trivial > function, and it would let the compiler optimize the operator==() > implementation. Actually, maybe we should instead define operator==() as > the canonical comparison operator, and implemente operator!=() as > !operator==(). I think that's what we usually do. > [...] When I originally made this change a long time ago I think I did it this way so that the ABI is not broken. But I would be strongly in favour of inlining `operator==` and expressing `operator!=` in terms of that. Regards, Barnabás Pőcze
On Wed, Dec 04, 2024 at 05:57:13PM +0000, Barnabás Pőcze wrote: > 2024. december 4., szerda 17:21 keltezéssel, Laurent Pinchart írta: > > On Wed, Dec 04, 2024 at 02:54:24PM +0000, Barnabás Pőcze wrote: > > > If `cpp_debugstl=true` in the build configuration, then libstdc++ will > > > try to use `operator==` and the build will fail. > > > > I didn't know about cpp_debugstl. Is that something we should enable in > > CI debug builds ? > > I believe so, yes. I'll give it a try once this fix gets merged. > > > Implement `operator==` in terms of `operator!=` to avoid the build failure. > > > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > > --- > > > include/libcamera/base/utils.h | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h > > > index 957150cb..782d5c97 100644 > > > --- a/include/libcamera/base/utils.h > > > +++ b/include/libcamera/base/utils.h > > > @@ -205,6 +205,10 @@ public: > > > iterator &operator++(); > > > std::string operator*() const; > > > bool operator!=(const iterator &other) const; > > > + bool operator==(const iterator &other) const > > > + { > > > + return !(*this != other); > > > + } > > > > Should we make operator!=() inline while at it ? It's a trivial > > function, and it would let the compiler optimize the operator==() > > implementation. Actually, maybe we should instead define operator==() as > > the canonical comparison operator, and implemente operator!=() as > > !operator==(). I think that's what we usually do. > > [...] > > When I originally made this change a long time ago I think I did it this way so > that the ABI is not broken. But I would be strongly in favour of inlining `operator==` > and expressing `operator!=` in terms of that. Could you send a new version of this patch that does so ? No need to split it in two different patches, you can combine them.
diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h index 957150cb..782d5c97 100644 --- a/include/libcamera/base/utils.h +++ b/include/libcamera/base/utils.h @@ -205,6 +205,10 @@ public: iterator &operator++(); std::string operator*() const; bool operator!=(const iterator &other) const; + bool operator==(const iterator &other) const + { + return !(*this != other); + } private: const StringSplitter *ss_;
If `cpp_debugstl=true` in the build configuration, then libstdc++ will try to use `operator==` and the build will fail. Implement `operator==` in terms of `operator!=` to avoid the build failure. Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- include/libcamera/base/utils.h | 4 ++++ 1 file changed, 4 insertions(+)