| Message ID | 20250721104622.1550908-4-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás On Mon, Jul 21, 2025 at 12:46:03PM +0200, Barnabás Pőcze wrote: > Add `cxx20.h` that will contain (possibly limited) C++17 implementations > of some C++20 standard library features. Our span.h implementation qualifies in this category as well. As I see it 1) We move span.h in /details/ 2) We move cxx20.h in include/libcamera/base What's your take ? > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > include/libcamera/base/details/cxx20.h | 12 ++++++++++++ > include/libcamera/base/meson.build | 7 +++++++ > 2 files changed, 19 insertions(+) > create mode 100644 include/libcamera/base/details/cxx20.h > > diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h > new file mode 100644 > index 000000000..0d6173d1b > --- /dev/null > +++ b/include/libcamera/base/details/cxx20.h > @@ -0,0 +1,12 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas on Board Oy > + * > + * C++20 polyfills > + */ > + > +#pragma once > + > +namespace libcamera::details::cxx20 { > + > +} /* namespace libcamera::details::cxx20 */ > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build > index f28ae4d42..9836daff9 100644 > --- a/include/libcamera/base/meson.build > +++ b/include/libcamera/base/meson.build > @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ > 'utils.h', > ]) > > +libcamera_base_details_headers = files([ > + 'details/cxx20.h', > +]) > + > libcamera_base_headers = [ > libcamera_base_public_headers, > libcamera_base_private_headers, > + libcamera_base_details_headers, > ] > > install_headers(libcamera_base_public_headers, > subdir : libcamera_base_include_dir) > +install_headers(libcamera_base_details_headers, > + subdir : libcamera_base_include_dir / 'details') > -- > 2.50.1 >
On Fri, Jul 25, 2025 at 02:58:06PM +0200, Jacopo Mondi wrote: > Hi Barnabás > > On Mon, Jul 21, 2025 at 12:46:03PM +0200, Barnabás Pőcze wrote: > > Add `cxx20.h` that will contain (possibly limited) C++17 implementations > > of some C++20 standard library features. > > Our span.h implementation qualifies in this category as well. > > As I see it > 1) We move span.h in /details/ > 2) We move cxx20.h in include/libcamera/base > > What's your take ? The issue with our Span class is that it's part of the public API, while I understand libcamera::details::cxx20 to be meant for internal usage. Although... > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > --- > > include/libcamera/base/details/cxx20.h | 12 ++++++++++++ > > include/libcamera/base/meson.build | 7 +++++++ > > 2 files changed, 19 insertions(+) > > create mode 100644 include/libcamera/base/details/cxx20.h > > > > diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h > > new file mode 100644 > > index 000000000..0d6173d1b > > --- /dev/null > > +++ b/include/libcamera/base/details/cxx20.h > > @@ -0,0 +1,12 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas on Board Oy > > + * > > + * C++20 polyfills > > + */ > > + > > +#pragma once > > + > > +namespace libcamera::details::cxx20 { > > + > > +} /* namespace libcamera::details::cxx20 */ > > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build > > index f28ae4d42..9836daff9 100644 > > --- a/include/libcamera/base/meson.build > > +++ b/include/libcamera/base/meson.build > > @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ > > 'utils.h', > > ]) > > > > +libcamera_base_details_headers = files([ > > + 'details/cxx20.h', > > +]) > > + > > libcamera_base_headers = [ > > libcamera_base_public_headers, > > libcamera_base_private_headers, > > + libcamera_base_details_headers, > > ] > > > > install_headers(libcamera_base_public_headers, > > subdir : libcamera_base_include_dir) > > +install_headers(libcamera_base_details_headers, > > + subdir : libcamera_base_include_dir / 'details') ... this seems to indicate the contrary ? Barnabás, could you please explain what your plan is ? If we want to use this in the public API, I think we should shorten the namespace for public classes, possibly to libcamera::cxx20. If it's purely internal, we can't move the span class here, and the headers should probably not be installed.
Hi 2025. 07. 27. 15:30 keltezéssel, Laurent Pinchart írta: > On Fri, Jul 25, 2025 at 02:58:06PM +0200, Jacopo Mondi wrote: >> Hi Barnabás >> >> On Mon, Jul 21, 2025 at 12:46:03PM +0200, Barnabás Pőcze wrote: >>> Add `cxx20.h` that will contain (possibly limited) C++17 implementations >>> of some C++20 standard library features. >> >> Our span.h implementation qualifies in this category as well. >> >> As I see it >> 1) We move span.h in /details/ >> 2) We move cxx20.h in include/libcamera/base >> >> What's your take ? > > The issue with our Span class is that it's part of the public API, while > I understand libcamera::details::cxx20 to be meant for internal usage. > Although... > >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>> --- >>> include/libcamera/base/details/cxx20.h | 12 ++++++++++++ >>> include/libcamera/base/meson.build | 7 +++++++ >>> 2 files changed, 19 insertions(+) >>> create mode 100644 include/libcamera/base/details/cxx20.h >>> >>> diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h >>> new file mode 100644 >>> index 000000000..0d6173d1b >>> --- /dev/null >>> +++ b/include/libcamera/base/details/cxx20.h >>> @@ -0,0 +1,12 @@ >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>> +/* >>> + * Copyright (C) 2025, Ideas on Board Oy >>> + * >>> + * C++20 polyfills >>> + */ >>> + >>> +#pragma once >>> + >>> +namespace libcamera::details::cxx20 { >>> + >>> +} /* namespace libcamera::details::cxx20 */ >>> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build >>> index f28ae4d42..9836daff9 100644 >>> --- a/include/libcamera/base/meson.build >>> +++ b/include/libcamera/base/meson.build >>> @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ >>> 'utils.h', >>> ]) >>> >>> +libcamera_base_details_headers = files([ >>> + 'details/cxx20.h', >>> +]) >>> + >>> libcamera_base_headers = [ >>> libcamera_base_public_headers, >>> libcamera_base_private_headers, >>> + libcamera_base_details_headers, >>> ] >>> >>> install_headers(libcamera_base_public_headers, >>> subdir : libcamera_base_include_dir) >>> +install_headers(libcamera_base_details_headers, >>> + subdir : libcamera_base_include_dir / 'details') > > ... this seems to indicate the contrary ? Barnabás, could you please > explain what your plan is ? > > If we want to use this in the public API, I think we should shorten the > namespace for public classes, possibly to libcamera::cxx20. If it's > purely internal, we can't move the span class here, and the headers > should probably not be installed. > The plan is that these are not part of the public API, merely implementation details that need to be available in public header files, so that they can be used from other public libcamera header files. Applications are not supposed to use them. Regards, Barnabás Pőcze
On Sun, Jul 27, 2025 at 07:15:56PM +0200, Barnabás Pőcze wrote: > 2025. 07. 27. 15:30 keltezéssel, Laurent Pinchart írta: > > On Fri, Jul 25, 2025 at 02:58:06PM +0200, Jacopo Mondi wrote: > >> On Mon, Jul 21, 2025 at 12:46:03PM +0200, Barnabás Pőcze wrote: > >>> Add `cxx20.h` that will contain (possibly limited) C++17 implementations > >>> of some C++20 standard library features. > >> > >> Our span.h implementation qualifies in this category as well. > >> > >> As I see it > >> 1) We move span.h in /details/ > >> 2) We move cxx20.h in include/libcamera/base > >> > >> What's your take ? > > > > The issue with our Span class is that it's part of the public API, while > > I understand libcamera::details::cxx20 to be meant for internal usage. > > Although... > > > >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >>> --- > >>> include/libcamera/base/details/cxx20.h | 12 ++++++++++++ > >>> include/libcamera/base/meson.build | 7 +++++++ > >>> 2 files changed, 19 insertions(+) > >>> create mode 100644 include/libcamera/base/details/cxx20.h > >>> > >>> diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h > >>> new file mode 100644 > >>> index 000000000..0d6173d1b > >>> --- /dev/null > >>> +++ b/include/libcamera/base/details/cxx20.h > >>> @@ -0,0 +1,12 @@ > >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > >>> +/* > >>> + * Copyright (C) 2025, Ideas on Board Oy > >>> + * > >>> + * C++20 polyfills > >>> + */ > >>> + > >>> +#pragma once > >>> + > >>> +namespace libcamera::details::cxx20 { > >>> + > >>> +} /* namespace libcamera::details::cxx20 */ > >>> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build > >>> index f28ae4d42..9836daff9 100644 > >>> --- a/include/libcamera/base/meson.build > >>> +++ b/include/libcamera/base/meson.build > >>> @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ > >>> 'utils.h', > >>> ]) > >>> > >>> +libcamera_base_details_headers = files([ > >>> + 'details/cxx20.h', > >>> +]) > >>> + > >>> libcamera_base_headers = [ > >>> libcamera_base_public_headers, > >>> libcamera_base_private_headers, > >>> + libcamera_base_details_headers, > >>> ] > >>> > >>> install_headers(libcamera_base_public_headers, > >>> subdir : libcamera_base_include_dir) > >>> +install_headers(libcamera_base_details_headers, > >>> + subdir : libcamera_base_include_dir / 'details') > > > > ... this seems to indicate the contrary ? Barnabás, could you please > > explain what your plan is ? > > > > If we want to use this in the public API, I think we should shorten the > > namespace for public classes, possibly to libcamera::cxx20. If it's > > purely internal, we can't move the span class here, and the headers > > should probably not be installed. > > The plan is that these are not part of the public API, merely implementation > details that need to be available in public header files, so that they can > be used from other public libcamera header files. Applications are not supposed > to use them. Then unless I'm mistaken you don't need to install those headers. I also wouldn't put this in a "details" namespace, libcamera::cxx20::* should be fine.
2025. 07. 27. 20:37 keltezéssel, Laurent Pinchart írta: > On Sun, Jul 27, 2025 at 07:15:56PM +0200, Barnabás Pőcze wrote: >> 2025. 07. 27. 15:30 keltezéssel, Laurent Pinchart írta: >>> On Fri, Jul 25, 2025 at 02:58:06PM +0200, Jacopo Mondi wrote: >>>> On Mon, Jul 21, 2025 at 12:46:03PM +0200, Barnabás Pőcze wrote: >>>>> Add `cxx20.h` that will contain (possibly limited) C++17 implementations >>>>> of some C++20 standard library features. >>>> >>>> Our span.h implementation qualifies in this category as well. >>>> >>>> As I see it >>>> 1) We move span.h in /details/ >>>> 2) We move cxx20.h in include/libcamera/base >>>> >>>> What's your take ? >>> >>> The issue with our Span class is that it's part of the public API, while >>> I understand libcamera::details::cxx20 to be meant for internal usage. >>> Although... >>> >>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>>> --- >>>>> include/libcamera/base/details/cxx20.h | 12 ++++++++++++ >>>>> include/libcamera/base/meson.build | 7 +++++++ >>>>> 2 files changed, 19 insertions(+) >>>>> create mode 100644 include/libcamera/base/details/cxx20.h >>>>> >>>>> diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h >>>>> new file mode 100644 >>>>> index 000000000..0d6173d1b >>>>> --- /dev/null >>>>> +++ b/include/libcamera/base/details/cxx20.h >>>>> @@ -0,0 +1,12 @@ >>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>>>> +/* >>>>> + * Copyright (C) 2025, Ideas on Board Oy >>>>> + * >>>>> + * C++20 polyfills >>>>> + */ >>>>> + >>>>> +#pragma once >>>>> + >>>>> +namespace libcamera::details::cxx20 { >>>>> + >>>>> +} /* namespace libcamera::details::cxx20 */ >>>>> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build >>>>> index f28ae4d42..9836daff9 100644 >>>>> --- a/include/libcamera/base/meson.build >>>>> +++ b/include/libcamera/base/meson.build >>>>> @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ >>>>> 'utils.h', >>>>> ]) >>>>> >>>>> +libcamera_base_details_headers = files([ >>>>> + 'details/cxx20.h', >>>>> +]) >>>>> + >>>>> libcamera_base_headers = [ >>>>> libcamera_base_public_headers, >>>>> libcamera_base_private_headers, >>>>> + libcamera_base_details_headers, >>>>> ] >>>>> >>>>> install_headers(libcamera_base_public_headers, >>>>> subdir : libcamera_base_include_dir) >>>>> +install_headers(libcamera_base_details_headers, >>>>> + subdir : libcamera_base_include_dir / 'details') >>> >>> ... this seems to indicate the contrary ? Barnabás, could you please >>> explain what your plan is ? >>> >>> If we want to use this in the public API, I think we should shorten the >>> namespace for public classes, possibly to libcamera::cxx20. If it's >>> purely internal, we can't move the span class here, and the headers >>> should probably not be installed. >> >> The plan is that these are not part of the public API, merely implementation >> details that need to be available in public header files, so that they can >> be used from other public libcamera header files. Applications are not supposed >> to use them. > > Then unless I'm mistaken you don't need to install those headers. I also > wouldn't put this in a "details" namespace, libcamera::cxx20::* should > be fine. > They need to be installed because the plan is that public libcamera headers files will include them.
On Sun, Jul 27, 2025 at 08:44:31PM +0200, Barnabás Pőcze wrote: > 2025. 07. 27. 20:37 keltezéssel, Laurent Pinchart írta: > > On Sun, Jul 27, 2025 at 07:15:56PM +0200, Barnabás Pőcze wrote: > >> 2025. 07. 27. 15:30 keltezéssel, Laurent Pinchart írta: > >>> On Fri, Jul 25, 2025 at 02:58:06PM +0200, Jacopo Mondi wrote: > >>>> On Mon, Jul 21, 2025 at 12:46:03PM +0200, Barnabás Pőcze wrote: > >>>>> Add `cxx20.h` that will contain (possibly limited) C++17 implementations > >>>>> of some C++20 standard library features. > >>>> > >>>> Our span.h implementation qualifies in this category as well. > >>>> > >>>> As I see it > >>>> 1) We move span.h in /details/ > >>>> 2) We move cxx20.h in include/libcamera/base > >>>> > >>>> What's your take ? > >>> > >>> The issue with our Span class is that it's part of the public API, while > >>> I understand libcamera::details::cxx20 to be meant for internal usage. > >>> Although... > >>> > >>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >>>>> --- > >>>>> include/libcamera/base/details/cxx20.h | 12 ++++++++++++ > >>>>> include/libcamera/base/meson.build | 7 +++++++ > >>>>> 2 files changed, 19 insertions(+) > >>>>> create mode 100644 include/libcamera/base/details/cxx20.h > >>>>> > >>>>> diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h > >>>>> new file mode 100644 > >>>>> index 000000000..0d6173d1b > >>>>> --- /dev/null > >>>>> +++ b/include/libcamera/base/details/cxx20.h > >>>>> @@ -0,0 +1,12 @@ > >>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > >>>>> +/* > >>>>> + * Copyright (C) 2025, Ideas on Board Oy > >>>>> + * > >>>>> + * C++20 polyfills > >>>>> + */ > >>>>> + > >>>>> +#pragma once > >>>>> + > >>>>> +namespace libcamera::details::cxx20 { > >>>>> + > >>>>> +} /* namespace libcamera::details::cxx20 */ > >>>>> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build > >>>>> index f28ae4d42..9836daff9 100644 > >>>>> --- a/include/libcamera/base/meson.build > >>>>> +++ b/include/libcamera/base/meson.build > >>>>> @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ > >>>>> 'utils.h', > >>>>> ]) > >>>>> > >>>>> +libcamera_base_details_headers = files([ > >>>>> + 'details/cxx20.h', > >>>>> +]) > >>>>> + > >>>>> libcamera_base_headers = [ > >>>>> libcamera_base_public_headers, > >>>>> libcamera_base_private_headers, > >>>>> + libcamera_base_details_headers, > >>>>> ] > >>>>> > >>>>> install_headers(libcamera_base_public_headers, > >>>>> subdir : libcamera_base_include_dir) > >>>>> +install_headers(libcamera_base_details_headers, > >>>>> + subdir : libcamera_base_include_dir / 'details') > >>> > >>> ... this seems to indicate the contrary ? Barnabás, could you please > >>> explain what your plan is ? > >>> > >>> If we want to use this in the public API, I think we should shorten the > >>> namespace for public classes, possibly to libcamera::cxx20. If it's > >>> purely internal, we can't move the span class here, and the headers > >>> should probably not be installed. > >> > >> The plan is that these are not part of the public API, merely implementation > >> details that need to be available in public header files, so that they can > >> be used from other public libcamera header files. Applications are not supposed > >> to use them. > > > > Then unless I'm mistaken you don't need to install those headers. I also > > wouldn't put this in a "details" namespace, libcamera::cxx20::* should > > be fine. > > They need to be installed because the plan is that public libcamera headers > files will include them. Ah, then they *are* part of the public API :-) I think you can jsut add those headers to libcamera_base_public_headers.
2025. 07. 27. 20:56 keltezéssel, Laurent Pinchart írta: > On Sun, Jul 27, 2025 at 08:44:31PM +0200, Barnabás Pőcze wrote: >> 2025. 07. 27. 20:37 keltezéssel, Laurent Pinchart írta: >>> On Sun, Jul 27, 2025 at 07:15:56PM +0200, Barnabás Pőcze wrote: >>>> 2025. 07. 27. 15:30 keltezéssel, Laurent Pinchart írta: >>>>> On Fri, Jul 25, 2025 at 02:58:06PM +0200, Jacopo Mondi wrote: >>>>>> On Mon, Jul 21, 2025 at 12:46:03PM +0200, Barnabás Pőcze wrote: >>>>>>> Add `cxx20.h` that will contain (possibly limited) C++17 implementations >>>>>>> of some C++20 standard library features. >>>>>> >>>>>> Our span.h implementation qualifies in this category as well. >>>>>> >>>>>> As I see it >>>>>> 1) We move span.h in /details/ >>>>>> 2) We move cxx20.h in include/libcamera/base >>>>>> >>>>>> What's your take ? >>>>> >>>>> The issue with our Span class is that it's part of the public API, while >>>>> I understand libcamera::details::cxx20 to be meant for internal usage. >>>>> Although... >>>>> >>>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>>>>> --- >>>>>>> include/libcamera/base/details/cxx20.h | 12 ++++++++++++ >>>>>>> include/libcamera/base/meson.build | 7 +++++++ >>>>>>> 2 files changed, 19 insertions(+) >>>>>>> create mode 100644 include/libcamera/base/details/cxx20.h >>>>>>> >>>>>>> diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h >>>>>>> new file mode 100644 >>>>>>> index 000000000..0d6173d1b >>>>>>> --- /dev/null >>>>>>> +++ b/include/libcamera/base/details/cxx20.h >>>>>>> @@ -0,0 +1,12 @@ >>>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>>>>>> +/* >>>>>>> + * Copyright (C) 2025, Ideas on Board Oy >>>>>>> + * >>>>>>> + * C++20 polyfills >>>>>>> + */ >>>>>>> + >>>>>>> +#pragma once >>>>>>> + >>>>>>> +namespace libcamera::details::cxx20 { >>>>>>> + >>>>>>> +} /* namespace libcamera::details::cxx20 */ >>>>>>> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build >>>>>>> index f28ae4d42..9836daff9 100644 >>>>>>> --- a/include/libcamera/base/meson.build >>>>>>> +++ b/include/libcamera/base/meson.build >>>>>>> @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ >>>>>>> 'utils.h', >>>>>>> ]) >>>>>>> >>>>>>> +libcamera_base_details_headers = files([ >>>>>>> + 'details/cxx20.h', >>>>>>> +]) >>>>>>> + >>>>>>> libcamera_base_headers = [ >>>>>>> libcamera_base_public_headers, >>>>>>> libcamera_base_private_headers, >>>>>>> + libcamera_base_details_headers, >>>>>>> ] >>>>>>> >>>>>>> install_headers(libcamera_base_public_headers, >>>>>>> subdir : libcamera_base_include_dir) >>>>>>> +install_headers(libcamera_base_details_headers, >>>>>>> + subdir : libcamera_base_include_dir / 'details') >>>>> >>>>> ... this seems to indicate the contrary ? Barnabás, could you please >>>>> explain what your plan is ? >>>>> >>>>> If we want to use this in the public API, I think we should shorten the >>>>> namespace for public classes, possibly to libcamera::cxx20. If it's >>>>> purely internal, we can't move the span class here, and the headers >>>>> should probably not be installed. >>>> >>>> The plan is that these are not part of the public API, merely implementation >>>> details that need to be available in public header files, so that they can >>>> be used from other public libcamera header files. Applications are not supposed >>>> to use them. >>> >>> Then unless I'm mistaken you don't need to install those headers. I also >>> wouldn't put this in a "details" namespace, libcamera::cxx20::* should >>> be fine. >> >> They need to be installed because the plan is that public libcamera headers >> files will include them. > > Ah, then they *are* part of the public API :-) > > I think you can jsut add those headers to libcamera_base_public_headers. > I would argue that they are not part of it. They are installed like this because of the limitations of the language, not because these functions/types/etc. are intended to be used by applications, in fact, they should not even be aware of their existence. There are no stability guarantees, etc. of any kind, they may be changed, removed, moved arbitrarily between even point releases.
Hi Barnabás On Sun, Jul 27, 2025 at 09:02:10PM +0200, Barnabás Pőcze wrote: > 2025. 07. 27. 20:56 keltezéssel, Laurent Pinchart írta: > > On Sun, Jul 27, 2025 at 08:44:31PM +0200, Barnabás Pőcze wrote: > > > 2025. 07. 27. 20:37 keltezéssel, Laurent Pinchart írta: > > > > On Sun, Jul 27, 2025 at 07:15:56PM +0200, Barnabás Pőcze wrote: > > > > > 2025. 07. 27. 15:30 keltezéssel, Laurent Pinchart írta: > > > > > > On Fri, Jul 25, 2025 at 02:58:06PM +0200, Jacopo Mondi wrote: > > > > > > > On Mon, Jul 21, 2025 at 12:46:03PM +0200, Barnabás Pőcze wrote: > > > > > > > > Add `cxx20.h` that will contain (possibly limited) C++17 implementations > > > > > > > > of some C++20 standard library features. > > > > > > > > > > > > > > Our span.h implementation qualifies in this category as well. > > > > > > > > > > > > > > As I see it > > > > > > > 1) We move span.h in /details/ > > > > > > > 2) We move cxx20.h in include/libcamera/base > > > > > > > > > > > > > > What's your take ? > > > > > > > > > > > > The issue with our Span class is that it's part of the public API, while > > > > > > I understand libcamera::details::cxx20 to be meant for internal usage. > > > > > > Although... > > > > > > > > > > > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > > > > > --- > > > > > > > > include/libcamera/base/details/cxx20.h | 12 ++++++++++++ > > > > > > > > include/libcamera/base/meson.build | 7 +++++++ > > > > > > > > 2 files changed, 19 insertions(+) > > > > > > > > create mode 100644 include/libcamera/base/details/cxx20.h > > > > > > > > > > > > > > > > diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h > > > > > > > > new file mode 100644 > > > > > > > > index 000000000..0d6173d1b > > > > > > > > --- /dev/null > > > > > > > > +++ b/include/libcamera/base/details/cxx20.h > > > > > > > > @@ -0,0 +1,12 @@ > > > > > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > > > > > +/* > > > > > > > > + * Copyright (C) 2025, Ideas on Board Oy > > > > > > > > + * > > > > > > > > + * C++20 polyfills > > > > > > > > + */ > > > > > > > > + > > > > > > > > +#pragma once > > > > > > > > + > > > > > > > > +namespace libcamera::details::cxx20 { > > > > > > > > + > > > > > > > > +} /* namespace libcamera::details::cxx20 */ > > > > > > > > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build > > > > > > > > index f28ae4d42..9836daff9 100644 > > > > > > > > --- a/include/libcamera/base/meson.build > > > > > > > > +++ b/include/libcamera/base/meson.build > > > > > > > > @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ > > > > > > > > 'utils.h', > > > > > > > > ]) > > > > > > > > > > > > > > > > +libcamera_base_details_headers = files([ > > > > > > > > + 'details/cxx20.h', > > > > > > > > +]) > > > > > > > > + > > > > > > > > libcamera_base_headers = [ > > > > > > > > libcamera_base_public_headers, > > > > > > > > libcamera_base_private_headers, > > > > > > > > + libcamera_base_details_headers, > > > > > > > > ] > > > > > > > > > > > > > > > > install_headers(libcamera_base_public_headers, > > > > > > > > subdir : libcamera_base_include_dir) > > > > > > > > +install_headers(libcamera_base_details_headers, > > > > > > > > + subdir : libcamera_base_include_dir / 'details') > > > > > > > > > > > > ... this seems to indicate the contrary ? Barnabás, could you please > > > > > > explain what your plan is ? > > > > > > > > > > > > If we want to use this in the public API, I think we should shorten the > > > > > > namespace for public classes, possibly to libcamera::cxx20. If it's > > > > > > purely internal, we can't move the span class here, and the headers > > > > > > should probably not be installed. > > > > > > > > > > The plan is that these are not part of the public API, merely implementation > > > > > details that need to be available in public header files, so that they can > > > > > be used from other public libcamera header files. Applications are not supposed > > > > > to use them. > > > > > > > > Then unless I'm mistaken you don't need to install those headers. I also > > > > wouldn't put this in a "details" namespace, libcamera::cxx20::* should > > > > be fine. > > > > > > They need to be installed because the plan is that public libcamera headers > > > files will include them. > > > > Ah, then they *are* part of the public API :-) > > > > I think you can jsut add those headers to libcamera_base_public_headers. > > > > I would argue that they are not part of it. They are installed like this because > of the limitations of the language, not because these functions/types/etc. are > intended to be used by applications, in fact, they should not even be aware of Isn't the point that applications, once the header has been made available to them, can decide to use it even if we don't suggest them to do so ? > their existence. There are no stability guarantees, etc. of any kind, they may > be changed, removed, moved arbitrarily between even point releases.
2025. 07. 28. 8:37 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Sun, Jul 27, 2025 at 09:02:10PM +0200, Barnabás Pőcze wrote: >> 2025. 07. 27. 20:56 keltezéssel, Laurent Pinchart írta: >>> On Sun, Jul 27, 2025 at 08:44:31PM +0200, Barnabás Pőcze wrote: >>>> 2025. 07. 27. 20:37 keltezéssel, Laurent Pinchart írta: >>>>> On Sun, Jul 27, 2025 at 07:15:56PM +0200, Barnabás Pőcze wrote: >>>>>> 2025. 07. 27. 15:30 keltezéssel, Laurent Pinchart írta: >>>>>>> On Fri, Jul 25, 2025 at 02:58:06PM +0200, Jacopo Mondi wrote: >>>>>>>> On Mon, Jul 21, 2025 at 12:46:03PM +0200, Barnabás Pőcze wrote: >>>>>>>>> Add `cxx20.h` that will contain (possibly limited) C++17 implementations >>>>>>>>> of some C++20 standard library features. >>>>>>>> >>>>>>>> Our span.h implementation qualifies in this category as well. >>>>>>>> >>>>>>>> As I see it >>>>>>>> 1) We move span.h in /details/ >>>>>>>> 2) We move cxx20.h in include/libcamera/base >>>>>>>> >>>>>>>> What's your take ? >>>>>>> >>>>>>> The issue with our Span class is that it's part of the public API, while >>>>>>> I understand libcamera::details::cxx20 to be meant for internal usage. >>>>>>> Although... >>>>>>> >>>>>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>>>>>>> --- >>>>>>>>> include/libcamera/base/details/cxx20.h | 12 ++++++++++++ >>>>>>>>> include/libcamera/base/meson.build | 7 +++++++ >>>>>>>>> 2 files changed, 19 insertions(+) >>>>>>>>> create mode 100644 include/libcamera/base/details/cxx20.h >>>>>>>>> >>>>>>>>> diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h >>>>>>>>> new file mode 100644 >>>>>>>>> index 000000000..0d6173d1b >>>>>>>>> --- /dev/null >>>>>>>>> +++ b/include/libcamera/base/details/cxx20.h >>>>>>>>> @@ -0,0 +1,12 @@ >>>>>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>>>>>>>> +/* >>>>>>>>> + * Copyright (C) 2025, Ideas on Board Oy >>>>>>>>> + * >>>>>>>>> + * C++20 polyfills >>>>>>>>> + */ >>>>>>>>> + >>>>>>>>> +#pragma once >>>>>>>>> + >>>>>>>>> +namespace libcamera::details::cxx20 { >>>>>>>>> + >>>>>>>>> +} /* namespace libcamera::details::cxx20 */ >>>>>>>>> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build >>>>>>>>> index f28ae4d42..9836daff9 100644 >>>>>>>>> --- a/include/libcamera/base/meson.build >>>>>>>>> +++ b/include/libcamera/base/meson.build >>>>>>>>> @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ >>>>>>>>> 'utils.h', >>>>>>>>> ]) >>>>>>>>> >>>>>>>>> +libcamera_base_details_headers = files([ >>>>>>>>> + 'details/cxx20.h', >>>>>>>>> +]) >>>>>>>>> + >>>>>>>>> libcamera_base_headers = [ >>>>>>>>> libcamera_base_public_headers, >>>>>>>>> libcamera_base_private_headers, >>>>>>>>> + libcamera_base_details_headers, >>>>>>>>> ] >>>>>>>>> >>>>>>>>> install_headers(libcamera_base_public_headers, >>>>>>>>> subdir : libcamera_base_include_dir) >>>>>>>>> +install_headers(libcamera_base_details_headers, >>>>>>>>> + subdir : libcamera_base_include_dir / 'details') >>>>>>> >>>>>>> ... this seems to indicate the contrary ? Barnabás, could you please >>>>>>> explain what your plan is ? >>>>>>> >>>>>>> If we want to use this in the public API, I think we should shorten the >>>>>>> namespace for public classes, possibly to libcamera::cxx20. If it's >>>>>>> purely internal, we can't move the span class here, and the headers >>>>>>> should probably not be installed. >>>>>> >>>>>> The plan is that these are not part of the public API, merely implementation >>>>>> details that need to be available in public header files, so that they can >>>>>> be used from other public libcamera header files. Applications are not supposed >>>>>> to use them. >>>>> >>>>> Then unless I'm mistaken you don't need to install those headers. I also >>>>> wouldn't put this in a "details" namespace, libcamera::cxx20::* should >>>>> be fine. >>>> >>>> They need to be installed because the plan is that public libcamera headers >>>> files will include them. >>> >>> Ah, then they *are* part of the public API :-) >>> >>> I think you can jsut add those headers to libcamera_base_public_headers. >>> >> >> I would argue that they are not part of it. They are installed like this because >> of the limitations of the language, not because these functions/types/etc. are >> intended to be used by applications, in fact, they should not even be aware of > > Isn't the point that applications, once the header has been made available > to them, can decide to use it even if we don't suggest them to do so ? That's true, but it's a limitation of the language. They can certainly use it, but my point is that libcamera should provide no guarantees regarding these parts. Yes, you can use it, but that is not supported. I'd argue that the use of "details" namespace is quite a common pattern, so this shouldn't be a surprise to anyone familiar. > >> their existence. There are no stability guarantees, etc. of any kind, they may >> be changed, removed, moved arbitrarily between even point releases.
diff --git a/include/libcamera/base/details/cxx20.h b/include/libcamera/base/details/cxx20.h new file mode 100644 index 000000000..0d6173d1b --- /dev/null +++ b/include/libcamera/base/details/cxx20.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas on Board Oy + * + * C++20 polyfills + */ + +#pragma once + +namespace libcamera::details::cxx20 { + +} /* namespace libcamera::details::cxx20 */ diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build index f28ae4d42..9836daff9 100644 --- a/include/libcamera/base/meson.build +++ b/include/libcamera/base/meson.build @@ -31,10 +31,17 @@ libcamera_base_private_headers = files([ 'utils.h', ]) +libcamera_base_details_headers = files([ + 'details/cxx20.h', +]) + libcamera_base_headers = [ libcamera_base_public_headers, libcamera_base_private_headers, + libcamera_base_details_headers, ] install_headers(libcamera_base_public_headers, subdir : libcamera_base_include_dir) +install_headers(libcamera_base_details_headers, + subdir : libcamera_base_include_dir / 'details')
Add `cxx20.h` that will contain (possibly limited) C++17 implementations of some C++20 standard library features. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- include/libcamera/base/details/cxx20.h | 12 ++++++++++++ include/libcamera/base/meson.build | 7 +++++++ 2 files changed, 19 insertions(+) create mode 100644 include/libcamera/base/details/cxx20.h