Documentation: coding-style: Document rule about indirect includes

26890 diff mbox series

libcamera has a coding style policy that forbids relying on indirect
includes, with some exceptions. As many policies, it is currently
undocumented. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/coding-style.rst | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)


base-commit: ff5cb4f17b0b9989dd853b4d4320ddb2c59a9c0e

2026. 06. 15. 20:16 keltezéssel, Laurent Pinchart írta:
> libcamera has a coding style policy that forbids relying on indirect
> includes, with some exceptions. As many policies, it is currently
> undocumented. Fix it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---

This sounds reasonable, but I don't think it will really change much without having a job
that runs iwyu or similar in CI. It's packaged for debian 13, maybe we want to do it?

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   Documentation/coding-style.rst | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
> 
> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> index 5b90a43a6813..2154331596ac 100644
> --- a/Documentation/coding-style.rst
> +++ b/Documentation/coding-style.rst
> @@ -61,6 +61,35 @@ document:
>     Code Style for indentation, braces, spacing, etc
>   * Headers are guarded by the use of '#pragma once'
> 
> +
> +Headers
> +-------
> +
> +Every source file shall ensure that all types and symbols it uses are declared,
> +either by including corresponding headers, or providing forward declarations.
> +Forward declarations should be used as much as possible in header files to
> +reduce compilation time.
> +
> +Relying on types and symbols being provided by indirect includes increases the
> +risk of compilation breakages, due either to code changes altering which
> +headers get indirectly include, or to indirect inclusions not being identical
> +across all configurations. The latter category is particularly difficult to
> +guard against through CI and can affect end users.
> +
> +For these reasons, relying on indirect includes to provide type declarations is
> +prohibited, except when the nature of an included headers guarantees that it
> +provides such declarations:
> +
> +* Code that declares a class inheriting from a base class can assume the header
> +  for the base class provides declarations for all types used as arguments of
> +  virtual functions.
> +* Code that implements member functions of a class can assume the header for
> +  the class provides declarations for all types used as arguments to the class'
> +  member functions.
> +
> +In those cases, header files may skip forward declarations and source files may
> +skip inclusion of headers.
> +
>   Order of Includes
>   ~~~~~~~~~~~~~~~~~
> 
> 
> base-commit: ff5cb4f17b0b9989dd853b4d4320ddb2c59a9c0e
> --
> Regards,
> 
> Laurent Pinchart
>

On Tue, Jun 16, 2026 at 09:40:24AM +0200, Barnabás Pőcze wrote:
> 2026. 06. 15. 20:16 keltezéssel, Laurent Pinchart írta:
> > libcamera has a coding style policy that forbids relying on indirect
> > includes, with some exceptions. As many policies, it is currently
> > undocumented. Fix it.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> 
> This sounds reasonable, but I don't think it will really change much without having a job
> that runs iwyu or similar in CI.

The main reason why I wrote this is to be able to point people to it
during reviews :-) But CI is certainly a nice addition.

> It's packaged for debian 13, maybe we want to do it?

Do you have experience integrating include-what-you-use with meson ?

> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> >   Documentation/coding-style.rst | 29 +++++++++++++++++++++++++++++
> >   1 file changed, 29 insertions(+)
> > 
> > diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> > index 5b90a43a6813..2154331596ac 100644
> > --- a/Documentation/coding-style.rst
> > +++ b/Documentation/coding-style.rst
> > @@ -61,6 +61,35 @@ document:
> >     Code Style for indentation, braces, spacing, etc
> >   * Headers are guarded by the use of '#pragma once'
> > 
> > +
> > +Headers
> > +-------
> > +
> > +Every source file shall ensure that all types and symbols it uses are declared,
> > +either by including corresponding headers, or providing forward declarations.
> > +Forward declarations should be used as much as possible in header files to
> > +reduce compilation time.
> > +
> > +Relying on types and symbols being provided by indirect includes increases the
> > +risk of compilation breakages, due either to code changes altering which
> > +headers get indirectly include, or to indirect inclusions not being identical
> > +across all configurations. The latter category is particularly difficult to
> > +guard against through CI and can affect end users.
> > +
> > +For these reasons, relying on indirect includes to provide type declarations is
> > +prohibited, except when the nature of an included headers guarantees that it
> > +provides such declarations:
> > +
> > +* Code that declares a class inheriting from a base class can assume the header
> > +  for the base class provides declarations for all types used as arguments of
> > +  virtual functions.
> > +* Code that implements member functions of a class can assume the header for
> > +  the class provides declarations for all types used as arguments to the class'
> > +  member functions.
> > +
> > +In those cases, header files may skip forward declarations and source files may
> > +skip inclusion of headers.
> > +
> >   Order of Includes
> >   ~~~~~~~~~~~~~~~~~
> > 
> > 
> > base-commit: ff5cb4f17b0b9989dd853b4d4320ddb2c59a9c0e

Quoting Laurent Pinchart (2026-06-16 15:06:12)
> On Tue, Jun 16, 2026 at 09:40:24AM +0200, Barnabás Pőcze wrote:
> > 2026. 06. 15. 20:16 keltezéssel, Laurent Pinchart írta:
> > > libcamera has a coding style policy that forbids relying on indirect
> > > includes, with some exceptions. As many policies, it is currently
> > > undocumented. Fix it.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > 
> > This sounds reasonable, but I don't think it will really change much without having a job
> > that runs iwyu or similar in CI.
> 
> The main reason why I wrote this is to be able to point people to it
> during reviews :-) But CI is certainly a nice addition.
> 
> > It's packaged for debian 13, maybe we want to do it?
> 
> Do you have experience integrating include-what-you-use with meson ?

When I've used it I just run:

 iwyu_tool -p build/gcc/ -j32 > iwyu.report

where build/gcc is my configured build of course.

-
Kieran


> 
> > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > 
> > >   Documentation/coding-style.rst | 29 +++++++++++++++++++++++++++++
> > >   1 file changed, 29 insertions(+)
> > > 
> > > diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> > > index 5b90a43a6813..2154331596ac 100644
> > > --- a/Documentation/coding-style.rst
> > > +++ b/Documentation/coding-style.rst
> > > @@ -61,6 +61,35 @@ document:
> > >     Code Style for indentation, braces, spacing, etc
> > >   * Headers are guarded by the use of '#pragma once'
> > > 
> > > +
> > > +Headers
> > > +-------
> > > +
> > > +Every source file shall ensure that all types and symbols it uses are declared,
> > > +either by including corresponding headers, or providing forward declarations.
> > > +Forward declarations should be used as much as possible in header files to
> > > +reduce compilation time.
> > > +
> > > +Relying on types and symbols being provided by indirect includes increases the
> > > +risk of compilation breakages, due either to code changes altering which
> > > +headers get indirectly include, or to indirect inclusions not being identical
> > > +across all configurations. The latter category is particularly difficult to
> > > +guard against through CI and can affect end users.
> > > +
> > > +For these reasons, relying on indirect includes to provide type declarations is
> > > +prohibited, except when the nature of an included headers guarantees that it
> > > +provides such declarations:
> > > +
> > > +* Code that declares a class inheriting from a base class can assume the header
> > > +  for the base class provides declarations for all types used as arguments of
> > > +  virtual functions.
> > > +* Code that implements member functions of a class can assume the header for
> > > +  the class provides declarations for all types used as arguments to the class'
> > > +  member functions.
> > > +
> > > +In those cases, header files may skip forward declarations and source files may
> > > +skip inclusion of headers.
> > > +

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> > >   Order of Includes
> > >   ~~~~~~~~~~~~~~~~~
> > > 
> > > 
> > > base-commit: ff5cb4f17b0b9989dd853b4d4320ddb2c59a9c0e
> 
> -- 
> Regards,
> 
> Laurent Pinchart

2026. 06. 16. 16:13 keltezéssel, Kieran Bingham írta:
> Quoting Laurent Pinchart (2026-06-16 15:06:12)
>> On Tue, Jun 16, 2026 at 09:40:24AM +0200, Barnabás Pőcze wrote:
>>> 2026. 06. 15. 20:16 keltezéssel, Laurent Pinchart írta:
>>>> libcamera has a coding style policy that forbids relying on indirect
>>>> includes, with some exceptions. As many policies, it is currently
>>>> undocumented. Fix it.
>>>>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>> ---
>>>
>>> This sounds reasonable, but I don't think it will really change much without having a job
>>> that runs iwyu or similar in CI.
>>
>> The main reason why I wrote this is to be able to point people to it
>> during reviews :-) But CI is certainly a nice addition.
>>
>>> It's packaged for debian 13, maybe we want to do it?
>>
>> Do you have experience integrating include-what-you-use with meson ?
> 
> When I've used it I just run:
> 
>   iwyu_tool -p build/gcc/ -j32 > iwyu.report
> 
> where build/gcc is my configured build of course.

^

It should work with the meson generated compilation database. We can add
it at the end of one (many?) job (preferably clang based, I guess) or pass
things as artifacts. The more interesting question is probably how to run
it only for the "changed" files, but that does not seem that trivial with
header dependencies and all, so maybe it's fine to just run it on all files.


> 
> -
> Kieran
> 
> 
>>
>>> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>>
>>>>    Documentation/coding-style.rst | 29 +++++++++++++++++++++++++++++
>>>>    1 file changed, 29 insertions(+)
>>>>
>>>> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
>>>> index 5b90a43a6813..2154331596ac 100644
>>>> --- a/Documentation/coding-style.rst
>>>> +++ b/Documentation/coding-style.rst
>>>> @@ -61,6 +61,35 @@ document:
>>>>      Code Style for indentation, braces, spacing, etc
>>>>    * Headers are guarded by the use of '#pragma once'
>>>>
>>>> +
>>>> +Headers
>>>> +-------
>>>> +
>>>> +Every source file shall ensure that all types and symbols it uses are declared,
>>>> +either by including corresponding headers, or providing forward declarations.
>>>> +Forward declarations should be used as much as possible in header files to
>>>> +reduce compilation time.
>>>> +
>>>> +Relying on types and symbols being provided by indirect includes increases the
>>>> +risk of compilation breakages, due either to code changes altering which
>>>> +headers get indirectly include, or to indirect inclusions not being identical
>>>> +across all configurations. The latter category is particularly difficult to
>>>> +guard against through CI and can affect end users.
>>>> +
>>>> +For these reasons, relying on indirect includes to provide type declarations is
>>>> +prohibited, except when the nature of an included headers guarantees that it
>>>> +provides such declarations:
>>>> +
>>>> +* Code that declares a class inheriting from a base class can assume the header
>>>> +  for the base class provides declarations for all types used as arguments of
>>>> +  virtual functions.
>>>> +* Code that implements member functions of a class can assume the header for
>>>> +  the class provides declarations for all types used as arguments to the class'
>>>> +  member functions.
>>>> +
>>>> +In those cases, header files may skip forward declarations and source files may
>>>> +skip inclusion of headers.
>>>> +
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
>>>>    Order of Includes
>>>>    ~~~~~~~~~~~~~~~~~
>>>>
>>>>
>>>> base-commit: ff5cb4f17b0b9989dd853b4d4320ddb2c59a9c0e
>>
>> -- 
>> Regards,
>>
>> Laurent Pinchart

Quoting Laurent Pinchart (2026-06-16 03:16:25)
> libcamera has a coding style policy that forbids relying on indirect
> includes, with some exceptions. As many policies, it is currently
> undocumented. Fix it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  Documentation/coding-style.rst | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> index 5b90a43a6813..2154331596ac 100644
> --- a/Documentation/coding-style.rst
> +++ b/Documentation/coding-style.rst
> @@ -61,6 +61,35 @@ document:
>    Code Style for indentation, braces, spacing, etc
>  * Headers are guarded by the use of '#pragma once'
>  
> +
> +Headers
> +-------
> +
> +Every source file shall ensure that all types and symbols it uses are declared,
> +either by including corresponding headers, or providing forward declarations.
> +Forward declarations should be used as much as possible in header files to
> +reduce compilation time.
> +
> +Relying on types and symbols being provided by indirect includes increases the
> +risk of compilation breakages, due either to code changes altering which
> +headers get indirectly include, or to indirect inclusions not being identical

s/include/included/

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> +across all configurations. The latter category is particularly difficult to
> +guard against through CI and can affect end users.
> +
> +For these reasons, relying on indirect includes to provide type declarations is
> +prohibited, except when the nature of an included headers guarantees that it
> +provides such declarations:
> +
> +* Code that declares a class inheriting from a base class can assume the header
> +  for the base class provides declarations for all types used as arguments of
> +  virtual functions.
> +* Code that implements member functions of a class can assume the header for
> +  the class provides declarations for all types used as arguments to the class'
> +  member functions.
> +
> +In those cases, header files may skip forward declarations and source files may
> +skip inclusion of headers.
> +
>  Order of Includes
>  ~~~~~~~~~~~~~~~~~
>  
> 
> base-commit: ff5cb4f17b0b9989dd853b4d4320ddb2c59a9c0e
> -- 
> Regards,
> 
> Laurent Pinchart
>