[libcamera-devel,RFC,01/17] libcamera: base: class: Document Extensible::_d() functions
diff mbox series

Message ID 20210723040036.32346-2-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Replace CameraData with Camera::Private
Related show

Commit Message

Laurent Pinchart July 23, 2021, 4 a.m. UTC
The Extensible::_d() functions are meant to be called by users of the
class. Document them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/base/class.h |  2 --
 src/libcamera/base/class.cpp   | 17 +++++++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

Niklas Söderlund July 24, 2021, 6:48 a.m. UTC | #1
Hi Laurent,

Thanks for your work.

On 2021-07-23 07:00:20 +0300, Laurent Pinchart wrote:
> The Extensible::_d() functions are meant to be called by users of the
> class. Document them.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/base/class.h |  2 --
>  src/libcamera/base/class.cpp   | 17 +++++++++++++++++
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/base/class.h b/include/libcamera/base/class.h
> index c2e1d3534ed1..9c7f0f2e6e27 100644
> --- a/include/libcamera/base/class.h
> +++ b/include/libcamera/base/class.h
> @@ -88,7 +88,6 @@ public:
>  	Extensible(Private *d);
>  
>  protected:
> -#ifndef __DOXYGEN__
>  	template<typename T>
>  	const T *_d() const
>  	{
> @@ -100,7 +99,6 @@ protected:
>  	{
>  		return static_cast<T *>(d_.get());
>  	}
> -#endif
>  
>  private:
>  	const std::unique_ptr<Private> d_;
> diff --git a/src/libcamera/base/class.cpp b/src/libcamera/base/class.cpp
> index 26b4967726d6..d0899671ca11 100644
> --- a/src/libcamera/base/class.cpp
> +++ b/src/libcamera/base/class.cpp
> @@ -150,6 +150,23 @@ Extensible::Extensible(Extensible::Private *d)
>  {
>  }
>  
> +/**
> + * \fn Extensible::_d() const
> + * \brief Retrieve the private data instance
> + *
> + * This template function isn't meant to be called directly. Instead, classes
> + * derived from Extensible get, through the LIBCAMERA_DECLARE_PRIVATE() macro,
> + * overriden _d() functions that return the correct pointer type to the
> + * corresponding derived Private class.
> + *
> + * \return A pointer to the private data instance
> + */
> +
> +/**
> + * \fn Extensible::_d()
> + * \copydoc Extensible::_d() const
> + */
> +
>  /**
>   * \var Extensible::d_
>   * \brief Pointer to the private data instance
> -- 
> Regards,
> 
> Laurent Pinchart
>
Jacopo Mondi July 29, 2021, 8:05 p.m. UTC | #2
Hi Laurent,

On Fri, Jul 23, 2021 at 07:00:20AM +0300, Laurent Pinchart wrote:
> The Extensible::_d() functions are meant to be called by users of the
> class. Document them.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/base/class.h |  2 --
>  src/libcamera/base/class.cpp   | 17 +++++++++++++++++
>  2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/include/libcamera/base/class.h b/include/libcamera/base/class.h
> index c2e1d3534ed1..9c7f0f2e6e27 100644
> --- a/include/libcamera/base/class.h
> +++ b/include/libcamera/base/class.h
> @@ -88,7 +88,6 @@ public:
>  	Extensible(Private *d);
>
>  protected:
> -#ifndef __DOXYGEN__
>  	template<typename T>
>  	const T *_d() const
>  	{
> @@ -100,7 +99,6 @@ protected:
>  	{
>  		return static_cast<T *>(d_.get());
>  	}
> -#endif
>
>  private:
>  	const std::unique_ptr<Private> d_;
> diff --git a/src/libcamera/base/class.cpp b/src/libcamera/base/class.cpp
> index 26b4967726d6..d0899671ca11 100644
> --- a/src/libcamera/base/class.cpp
> +++ b/src/libcamera/base/class.cpp
> @@ -150,6 +150,23 @@ Extensible::Extensible(Extensible::Private *d)
>  {
>  }
>
> +/**
> + * \fn Extensible::_d() const
> + * \brief Retrieve the private data instance
> + *
> + * This template function isn't meant to be called directly. Instead, classes
> + * derived from Extensible get, through the LIBCAMERA_DECLARE_PRIVATE() macro,
> + * overriden _d() functions that return the correct pointer type to the
> + * corresponding derived Private class.

It really took me a while to understand that we have Extensible::_d()
and the overriden versions, and it's written just here :)

Probably because the commit message says "are meant to be called by
users" and this starts with "isn't meant to be called directly" :)

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> + *
> + * \return A pointer to the private data instance
> + */
> +
> +/**
> + * \fn Extensible::_d()
> + * \copydoc Extensible::_d() const
> + */
> +
>  /**
>   * \var Extensible::d_
>   * \brief Pointer to the private data instance
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart July 30, 2021, 2:28 a.m. UTC | #3
Hi Jacopo,

On Thu, Jul 29, 2021 at 10:05:42PM +0200, Jacopo Mondi wrote:
> On Fri, Jul 23, 2021 at 07:00:20AM +0300, Laurent Pinchart wrote:
> > The Extensible::_d() functions are meant to be called by users of the
> > class. Document them.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/libcamera/base/class.h |  2 --
> >  src/libcamera/base/class.cpp   | 17 +++++++++++++++++
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libcamera/base/class.h b/include/libcamera/base/class.h
> > index c2e1d3534ed1..9c7f0f2e6e27 100644
> > --- a/include/libcamera/base/class.h
> > +++ b/include/libcamera/base/class.h
> > @@ -88,7 +88,6 @@ public:
> >  	Extensible(Private *d);
> >
> >  protected:
> > -#ifndef __DOXYGEN__
> >  	template<typename T>
> >  	const T *_d() const
> >  	{
> > @@ -100,7 +99,6 @@ protected:
> >  	{
> >  		return static_cast<T *>(d_.get());
> >  	}
> > -#endif
> >
> >  private:
> >  	const std::unique_ptr<Private> d_;
> > diff --git a/src/libcamera/base/class.cpp b/src/libcamera/base/class.cpp
> > index 26b4967726d6..d0899671ca11 100644
> > --- a/src/libcamera/base/class.cpp
> > +++ b/src/libcamera/base/class.cpp
> > @@ -150,6 +150,23 @@ Extensible::Extensible(Extensible::Private *d)
> >  {
> >  }
> >
> > +/**
> > + * \fn Extensible::_d() const
> > + * \brief Retrieve the private data instance
> > + *
> > + * This template function isn't meant to be called directly. Instead, classes
> > + * derived from Extensible get, through the LIBCAMERA_DECLARE_PRIVATE() macro,
> > + * overriden _d() functions that return the correct pointer type to the
> > + * corresponding derived Private class.
> 
> It really took me a while to understand that we have Extensible::_d()
> and the overriden versions, and it's written just here :)
> 
> Probably because the commit message says "are meant to be called by
> users" and this starts with "isn't meant to be called directly" :)

The issue is that the Extensible::_d() function isn't meant to be called
directly, but I haven't found a way to get doxygen to document the
overridden functions generated by LIBCAMERA_DECLARE_PRIVATE(). I've thus
decided to document this in the base Extensible::_d() function as the
best workaround I could find :-S.

> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > + *
> > + * \return A pointer to the private data instance
> > + */
> > +
> > +/**
> > + * \fn Extensible::_d()
> > + * \copydoc Extensible::_d() const
> > + */
> > +
> >  /**
> >   * \var Extensible::d_
> >   * \brief Pointer to the private data instance

Patch
diff mbox series

diff --git a/include/libcamera/base/class.h b/include/libcamera/base/class.h
index c2e1d3534ed1..9c7f0f2e6e27 100644
--- a/include/libcamera/base/class.h
+++ b/include/libcamera/base/class.h
@@ -88,7 +88,6 @@  public:
 	Extensible(Private *d);
 
 protected:
-#ifndef __DOXYGEN__
 	template<typename T>
 	const T *_d() const
 	{
@@ -100,7 +99,6 @@  protected:
 	{
 		return static_cast<T *>(d_.get());
 	}
-#endif
 
 private:
 	const std::unique_ptr<Private> d_;
diff --git a/src/libcamera/base/class.cpp b/src/libcamera/base/class.cpp
index 26b4967726d6..d0899671ca11 100644
--- a/src/libcamera/base/class.cpp
+++ b/src/libcamera/base/class.cpp
@@ -150,6 +150,23 @@  Extensible::Extensible(Extensible::Private *d)
 {
 }
 
+/**
+ * \fn Extensible::_d() const
+ * \brief Retrieve the private data instance
+ *
+ * This template function isn't meant to be called directly. Instead, classes
+ * derived from Extensible get, through the LIBCAMERA_DECLARE_PRIVATE() macro,
+ * overriden _d() functions that return the correct pointer type to the
+ * corresponding derived Private class.
+ *
+ * \return A pointer to the private data instance
+ */
+
+/**
+ * \fn Extensible::_d()
+ * \copydoc Extensible::_d() const
+ */
+
 /**
  * \var Extensible::d_
  * \brief Pointer to the private data instance