[RFC,v2,03/22] libcamera: base: Add file for C++20 polyfills
diff mbox series

Message ID 20250721104622.1550908-4-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • libcamera: Add `MetadataList`
Related show

Commit Message

Barnabás Pőcze July 21, 2025, 10:46 a.m. UTC
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

Comments

Jacopo Mondi July 25, 2025, 12:58 p.m. UTC | #1
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
>
Laurent Pinchart July 27, 2025, 1:30 p.m. UTC | #2
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.
Barnabás Pőcze July 27, 2025, 5:15 p.m. UTC | #3
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
Laurent Pinchart July 27, 2025, 6:37 p.m. UTC | #4
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.
Barnabás Pőcze July 27, 2025, 6:44 p.m. UTC | #5
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.
Laurent Pinchart July 27, 2025, 6:56 p.m. UTC | #6
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.
Barnabás Pőcze July 27, 2025, 7:02 p.m. UTC | #7
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.
Jacopo Mondi July 28, 2025, 6:37 a.m. UTC | #8
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.
Barnabás Pőcze Aug. 15, 2025, 9:04 a.m. UTC | #9
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.
Paul Elder Sept. 19, 2025, 11:09 a.m. UTC | #10
Quoting Barnabás Pőcze (2025-08-15 18:04:21)
> 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.

Ok, so the public libcamera headers require them, therefore cxx20.h needs to be
in the public headers. Since they are in the public headers, applications are
able use them regardless of if we tell them they cannot use it. This is the
premise.

So do we consider cxx20.h to be part of the public API that needs to be checked
for changes every time we make a release? And if it changes then we need to
bump the version number etc etc.

> 
> > 
> >> their existence. There are no stability guarantees, etc. of any kind, they may
> >> be changed, removed, moved arbitrarily between even point releases.
> 

Or are we permitted to change it anytime we want and just say "deal with it" to
applications that decided to use it against our policy?

I personally don't think that the latter is reasonable. We exposed it as a
public API whether it was intended to be used by applications or not. Therefore
it's probably more reasonable (and responsible) to go the full mile and just
expose it as public API and include it in public API versioning/policies.

I understand that it's due to a limitation of the language, but with this
direction in design, I think it's the more responsible choice to make it fully
part of the public API, just like how spans are.

Also considering that the API surface isn't that big and I don't expect it to
change very frequently (maybe grow but not change) I don't think it'll be that
big of an issue to support properly.

My two cents.


Paul

Patch
diff mbox series

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')