[libcamera-devel] [PATCH] libcamera: revert C++17 specific code for public API
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jan 6 12:47:17 CET 2021
Hi Shik,
On Wed, Jan 06, 2021 at 05:28:38PM +0800, Shik Chen wrote:
> On Mon, Jan 4, 2021 at 1:54 PM Laurent Pinchart wrote:
> > On Thu, Dec 31, 2020 at 04:47:10PM +0100, Jean-Michel Hautbois wrote:
> > > Some applications may not be compliant with C++17 (Chromium, as an
> > > example). Keep the C++17 features for libcamera internals, and C++14
> > > compliance for public API.
> >
> > This is (unfortunately :-)) a good point. I was expecting Chromium to
> > use a more recent C++ version, I'm a bit surprised. There seem to be two
> > separate questions here:
> >
> > - Could we build the libcamera support in Chromium with C++17
> > (specifying the C++ version explicitly in the BUILD.gn file in the
> > corresponding subdirectory, overriding the default C++14 version) ?
> >
> > - As a more general question, can we expect C++17 support in the not too
> > distant future in code that will use libcamera, or is that an
> > unreasonable expectation and should we stick to c++14 in the public
> > API ?
>
> Some information for Chromium specifically:
>
> According to https://chromium-cpp.appspot.com/:
> > C++17: Not yet supported in Chromium, unlikely before mid-2021
We found the same information :-)
For Chromium specifically, I wonder if the libcamera integration code
could be compiled with C++-17, with the rest of the code base using
C++14.
> Quote from the tracking bug https://crbug.com/752720#c147:
> > the NaCl toolchain's lack of C++17 support is the sole significant blocker.
>
> The EOL of NaCl toolchain was punted to June 2022 (it was June 2021):
> https://blog.chromium.org/2020/08/changes-to-chrome-app-support-timeline.html
>
> > If the answer to the first question is no, then we won't have a choice
> > for the second question.
> >
> > We could stick to C++14 in the public API as it should not block us (at
> > least for now), this revert isn't that bad. I'm however concerned that
> > C++17 features may creep in unnoticed in the API, as we can't test
> > building libcamera with C++14, given that the internals depend on C++17.
> >
> > > This reverts commits 8e42c2feb7ff7c350ffbbf97dd963dfd54e21faa and
> > > 6cbdc2859963e17bc897a4022f1d68170477d888.
> >
> > Reverts are usually submitted separately, to make is easier to track
> > changes.
> >
> > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > > ---
> > > include/libcamera/bound_method.h | 2 +-
> > > include/libcamera/controls.h | 24 +++++------
> > > include/libcamera/object.h | 2 +-
> > > include/libcamera/signal.h | 4 +-
> > > include/libcamera/span.h | 70 +++++++++++++++-----------------
> > > 5 files changed, 48 insertions(+), 54 deletions(-)
> > >
> > > diff --git a/include/libcamera/bound_method.h b/include/libcamera/bound_method.h
> > > index feac51da..95a95653 100644
> > > --- a/include/libcamera/bound_method.h
> > > +++ b/include/libcamera/bound_method.h
> > > @@ -63,7 +63,7 @@ public:
> > > }
> > > virtual ~BoundMethodBase() = default;
> > >
> > > - template<typename T, typename std::enable_if_t<!std::is_same_v<Object, T>> * = nullptr>
> > > + template<typename T, typename std::enable_if_t<!std::is_same<Object, T>::value> * = nullptr>
> > > bool match(T *obj) { return obj == obj_; }
> > > bool match(Object *object) { return object == object_; }
> > >
> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > > index 3634dc43..3b7f3347 100644
> > > --- a/include/libcamera/controls.h
> > > +++ b/include/libcamera/controls.h
> > > @@ -96,9 +96,9 @@ public:
> > > ControlValue();
> > >
> > > #ifndef __DOXYGEN__
> > > - template<typename T, typename std::enable_if_t<!details::is_span_v<T> &&
> > > + template<typename T, typename std::enable_if_t<!details::is_span<T>::value &&
> > > details::control_type<T>::value &&
> > > - !std::is_same_v<std::string, std::remove_cv_t<T>>,
> > > + !std::is_same<std::string, std::remove_cv_t<T>>::value,
> > > std::nullptr_t> = nullptr>
> > > ControlValue(const T &value)
> > > : type_(ControlTypeNone), numElements_(0)
> > > @@ -107,8 +107,8 @@ public:
> > > &value, 1, sizeof(T));
> > > }
> > >
> > > - template<typename T, typename std::enable_if_t<details::is_span_v<T> ||
> > > - std::is_same_v<std::string, std::remove_cv_t<T>>,
> > > + template<typename T, typename std::enable_if_t<details::is_span<T>::value ||
> > > + std::is_same<std::string, std::remove_cv_t<T>>::value,
> > > std::nullptr_t> = nullptr>
> > > #else
> > > template<typename T>
> > > @@ -141,8 +141,8 @@ public:
> > > }
> > >
> > > #ifndef __DOXYGEN__
> > > - template<typename T, typename std::enable_if_t<!details::is_span_v<T> &&
> > > - !std::is_same_v<std::string, std::remove_cv_t<T>>,
> > > + template<typename T, typename std::enable_if_t<!details::is_span<T>::value &&
> > > + !std::is_same<std::string, std::remove_cv_t<T>>::value,
> > > std::nullptr_t> = nullptr>
> > > T get() const
> > > {
> > > @@ -152,8 +152,8 @@ public:
> > > return *reinterpret_cast<const T *>(data().data());
> > > }
> > >
> > > - template<typename T, typename std::enable_if_t<details::is_span_v<T> ||
> > > - std::is_same_v<std::string, std::remove_cv_t<T>>,
> > > + template<typename T, typename std::enable_if_t<details::is_span<T>::value ||
> > > + std::is_same<std::string, std::remove_cv_t<T>>::value,
> > > std::nullptr_t> = nullptr>
> > > #else
> > > template<typename T>
> > > @@ -169,8 +169,8 @@ public:
> > > }
> > >
> > > #ifndef __DOXYGEN__
> > > - template<typename T, typename std::enable_if_t<!details::is_span_v<T> &&
> > > - !std::is_same_v<std::string, std::remove_cv_t<T>>,
> > > + template<typename T, typename std::enable_if_t<!details::is_span<T>::value &&
> > > + !std::is_same<std::string, std::remove_cv_t<T>>::value,
> > > std::nullptr_t> = nullptr>
> > > void set(const T &value)
> > > {
> > > @@ -178,8 +178,8 @@ public:
> > > reinterpret_cast<const void *>(&value), 1, sizeof(T));
> > > }
> > >
> > > - template<typename T, typename std::enable_if_t<details::is_span_v<T> ||
> > > - std::is_same_v<std::string, std::remove_cv_t<T>>,
> > > + template<typename T, typename std::enable_if_t<details::is_span<T>::value ||
> > > + std::is_same<std::string, std::remove_cv_t<T>>::value,
> > > std::nullptr_t> = nullptr>
> > > #else
> > > template<typename T>
> > > diff --git a/include/libcamera/object.h b/include/libcamera/object.h
> > > index 423208db..a1882f05 100644
> > > --- a/include/libcamera/object.h
> > > +++ b/include/libcamera/object.h
> > > @@ -32,7 +32,7 @@ public:
> > > void postMessage(std::unique_ptr<Message> msg);
> > >
> > > template<typename T, typename R, typename... FuncArgs, typename... Args,
> > > - typename std::enable_if_t<std::is_base_of_v<Object, T>> * = nullptr>
> > > + typename std::enable_if_t<std::is_base_of<Object, T>::value> * = nullptr>
> > > R invokeMethod(R (T::*func)(FuncArgs...), ConnectionType type,
> > > Args... args)
> > > {
> > > diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h
> > > index 46d917d5..5bcd7a77 100644
> > > --- a/include/libcamera/signal.h
> > > +++ b/include/libcamera/signal.h
> > > @@ -44,7 +44,7 @@ public:
> > > }
> > >
> > > #ifndef __DOXYGEN__
> > > - template<typename T, typename R, typename std::enable_if_t<std::is_base_of_v<Object, T>> * = nullptr>
> > > + template<typename T, typename R, typename std::enable_if_t<std::is_base_of<Object, T>::value> * = nullptr>
> > > void connect(T *obj, R (T::*func)(Args...),
> > > ConnectionType type = ConnectionTypeAuto)
> > > {
> > > @@ -52,7 +52,7 @@ public:
> > > SignalBase::connect(new BoundMethodMember<T, void, Args...>(obj, object, func, type));
> > > }
> > >
> > > - template<typename T, typename R, typename std::enable_if_t<!std::is_base_of_v<Object, T>> * = nullptr>
> > > + template<typename T, typename R, typename std::enable_if_t<!std::is_base_of<Object, T>::value> * = nullptr>
> > > #else
> > > template<typename T, typename R>
> > > #endif
> > > diff --git a/include/libcamera/span.h b/include/libcamera/span.h
> > > index e7ffef12..91e9f974 100644
> > > --- a/include/libcamera/span.h
> > > +++ b/include/libcamera/span.h
> > > @@ -31,9 +31,6 @@ template<typename U, std::size_t N>
> > > struct is_array<std::array<U, N>> : public std::true_type {
> > > };
> > >
> > > -template<typename T>
> > > -inline constexpr bool is_array_v = is_array<T>::value;
> > > -
> > > template<typename U>
> > > struct is_span : public std::false_type {
> > > };
> > > @@ -42,9 +39,6 @@ template<typename U, std::size_t Extent>
> > > struct is_span<Span<U, Extent>> : public std::true_type {
> > > };
> > >
> > > -template<typename T>
> > > -inline constexpr bool is_span_v = is_span<T>::value;
> > > -
> > > } /* namespace details */
> > >
> > > namespace utils {
> > > @@ -131,8 +125,8 @@ public:
> > >
> > > template<std::size_t N>
> > > constexpr Span(element_type (&arr)[N],
> > > - std::enable_if_t<std::is_convertible_v<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > - element_type (*)[]> &&
> > > + std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > + element_type (*)[]>::value &&
> > > N == Extent,
> > > std::nullptr_t> = nullptr) noexcept
> > > : data_(arr)
> > > @@ -141,8 +135,8 @@ public:
> > >
> > > template<std::size_t N>
> > > constexpr Span(std::array<value_type, N> &arr,
> > > - std::enable_if_t<std::is_convertible_v<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > - element_type (*)[]> &&
> > > + std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > + element_type (*)[]>::value &&
> > > N == Extent,
> > > std::nullptr_t> = nullptr) noexcept
> > > : data_(arr.data())
> > > @@ -151,8 +145,8 @@ public:
> > >
> > > template<std::size_t N>
> > > constexpr Span(const std::array<value_type, N> &arr,
> > > - std::enable_if_t<std::is_convertible_v<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > - element_type (*)[]> &&
> > > + std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > + element_type (*)[]>::value &&
> > > N == Extent,
> > > std::nullptr_t> = nullptr) noexcept
> > > : data_(arr.data())
> > > @@ -161,11 +155,11 @@ public:
> > >
> > > template<class Container>
> > > explicit constexpr Span(Container &cont,
> > > - std::enable_if_t<!details::is_span_v<Container> &&
> > > - !details::is_array_v<Container> &&
> > > - !std::is_array_v<Container> &&
> > > - std::is_convertible_v<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> > > - element_type (*)[]>,
> > > + std::enable_if_t<!details::is_span<Container>::value &&
> > > + !details::is_array<Container>::value &&
> > > + !std::is_array<Container>::value &&
> > > + std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> > > + element_type (*)[]>::value,
> > > std::nullptr_t> = nullptr)
> > > : data_(utils::data(cont))
> > > {
> > > @@ -173,11 +167,11 @@ public:
> > >
> > > template<class Container>
> > > explicit constexpr Span(const Container &cont,
> > > - std::enable_if_t<!details::is_span_v<Container> &&
> > > - !details::is_array_v<Container> &&
> > > - !std::is_array_v<Container> &&
> > > - std::is_convertible_v<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> > > - element_type (*)[]>,
> > > + std::enable_if_t<!details::is_span<Container>::value &&
> > > + !details::is_array<Container>::value &&
> > > + !std::is_array<Container>::value &&
> > > + std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> > > + element_type (*)[]>::value,
> > > std::nullptr_t> = nullptr)
> > > : data_(utils::data(cont))
> > > {
> > > @@ -186,7 +180,7 @@ public:
> > >
> > > template<class U, std::size_t N>
> > > explicit constexpr Span(const Span<U, N> &s,
> > > - std::enable_if_t<std::is_convertible_v<U (*)[], element_type (*)[]> &&
> > > + std::enable_if_t<std::is_convertible<U (*)[], element_type (*)[]>::value &&
> > > N == Extent,
> > > std::nullptr_t> = nullptr) noexcept
> > > : data_(s.data())
> > > @@ -299,8 +293,8 @@ public:
> > >
> > > template<std::size_t N>
> > > constexpr Span(element_type (&arr)[N],
> > > - std::enable_if_t<std::is_convertible_v<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > - element_type (*)[]>,
> > > + std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > + element_type (*)[]>::value,
> > > std::nullptr_t> = nullptr) noexcept
> > > : data_(arr), size_(N)
> > > {
> > > @@ -308,8 +302,8 @@ public:
> > >
> > > template<std::size_t N>
> > > constexpr Span(std::array<value_type, N> &arr,
> > > - std::enable_if_t<std::is_convertible_v<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > - element_type (*)[]>,
> > > + std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
> > > + element_type (*)[]>::value,
> > > std::nullptr_t> = nullptr) noexcept
> > > : data_(utils::data(arr)), size_(N)
> > > {
> > > @@ -323,11 +317,11 @@ public:
> > >
> > > template<class Container>
> > > constexpr Span(Container &cont,
> > > - std::enable_if_t<!details::is_span_v<Container> &&
> > > - !details::is_array_v<Container> &&
> > > - !std::is_array_v<Container> &&
> > > - std::is_convertible_v<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> > > - element_type (*)[]>,
> > > + std::enable_if_t<!details::is_span<Container>::value &&
> > > + !details::is_array<Container>::value &&
> > > + !std::is_array<Container>::value &&
> > > + std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> > > + element_type (*)[]>::value,
> > > std::nullptr_t> = nullptr)
> > > : data_(utils::data(cont)), size_(utils::size(cont))
> > > {
> > > @@ -335,11 +329,11 @@ public:
> > >
> > > template<class Container>
> > > constexpr Span(const Container &cont,
> > > - std::enable_if_t<!details::is_span_v<Container> &&
> > > - !details::is_array_v<Container> &&
> > > - !std::is_array_v<Container> &&
> > > - std::is_convertible_v<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> > > - element_type (*)[]>,
> > > + std::enable_if_t<!details::is_span<Container>::value &&
> > > + !details::is_array<Container>::value &&
> > > + !std::is_array<Container>::value &&
> > > + std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
> > > + element_type (*)[]>::value,
> > > std::nullptr_t> = nullptr)
> > > : data_(utils::data(cont)), size_(utils::size(cont))
> > > {
> > > @@ -347,7 +341,7 @@ public:
> > >
> > > template<class U, std::size_t N>
> > > constexpr Span(const Span<U, N> &s,
> > > - std::enable_if_t<std::is_convertible_v<U (*)[], element_type (*)[]>,
> > > + std::enable_if_t<std::is_convertible<U (*)[], element_type (*)[]>::value,
> > > std::nullptr_t> = nullptr) noexcept
> > > : data_(s.data()), size_(s.size())
> > > {
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list