[v3,03/17] ipa: libipa: vector: Add scalar constructor
diff mbox series

Message ID 20241118221618.13953-4-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • Improve linear algebra helpers in libipa
Related show

Commit Message

Laurent Pinchart Nov. 18, 2024, 10:16 p.m. UTC
The default constructor leaves the vector data uninitialized. Add a
constructor to fill the vector with copies of a scalar value, and fix
the documentation of the default constructor.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/libipa/vector.cpp | 8 +++++++-
 src/ipa/libipa/vector.h   | 5 +++++
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Barnabás Pőcze Nov. 18, 2024, 11:49 p.m. UTC | #1
Hi


2024. november 18., hétfő 23:16 keltezéssel, Laurent Pinchart <laurent.pinchart@ideasonboard.com> írta:

> The default constructor leaves the vector data uninitialized. Add a
> constructor to fill the vector with copies of a scalar value, and fix
> the documentation of the default constructor.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/ipa/libipa/vector.cpp | 8 +++++++-
>  src/ipa/libipa/vector.h   | 5 +++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp
> index f14f155216f3..d414ba97e41e 100644
> --- a/src/ipa/libipa/vector.cpp
> +++ b/src/ipa/libipa/vector.cpp
> @@ -29,7 +29,13 @@ namespace ipa {
> 
>  /**
>   * \fn Vector::Vector()
> - * \brief Construct a zero vector
> + * \brief Construct an uninitialized vector
> + */
> +
> +/**
> + * \fn Vector::Vector(T scalar)
> + * \brief Construct a vector filled with a \a scalar value
> + * \param[in] scalar The scalar value
>   */
> 
>  /**
> diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h
> index b72ab9851aa3..be568eadfeac 100644
> --- a/src/ipa/libipa/vector.h
> +++ b/src/ipa/libipa/vector.h
> @@ -35,6 +35,11 @@ class Vector
>  public:
>  	constexpr Vector() = default;
> 
> +	constexpr Vector(T scalar)
> +	{
> +		data_.fill(scalar);
> +	}
> +

Is there an expectation that e.g. `Vector<int, 3> x = 1` works? If not, then
I think adding `explicit` would be something to consider to avoid erroneous
construction from a scalar, especially in function calls.


Regards,
Barnabás Pőcze


>  	constexpr Vector(const std::array<T, Rows> &data)
>  	{
>  		for (unsigned int i = 0; i < Rows; i++)
> --
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart Nov. 19, 2024, 8:36 a.m. UTC | #2
On Mon, Nov 18, 2024 at 11:49:16PM +0000, Barnabás Pőcze wrote:
> Hi
> 
> 
> 2024. november 18., hétfő 23:16 keltezéssel, Laurent Pinchart <laurent.pinchart@ideasonboard.com> írta:
> 
> > The default constructor leaves the vector data uninitialized. Add a
> > constructor to fill the vector with copies of a scalar value, and fix
> > the documentation of the default constructor.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/ipa/libipa/vector.cpp | 8 +++++++-
> >  src/ipa/libipa/vector.h   | 5 +++++
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp
> > index f14f155216f3..d414ba97e41e 100644
> > --- a/src/ipa/libipa/vector.cpp
> > +++ b/src/ipa/libipa/vector.cpp
> > @@ -29,7 +29,13 @@ namespace ipa {
> > 
> >  /**
> >   * \fn Vector::Vector()
> > - * \brief Construct a zero vector
> > + * \brief Construct an uninitialized vector
> > + */
> > +
> > +/**
> > + * \fn Vector::Vector(T scalar)
> > + * \brief Construct a vector filled with a \a scalar value
> > + * \param[in] scalar The scalar value
> >   */
> > 
> >  /**
> > diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h
> > index b72ab9851aa3..be568eadfeac 100644
> > --- a/src/ipa/libipa/vector.h
> > +++ b/src/ipa/libipa/vector.h
> > @@ -35,6 +35,11 @@ class Vector
> >  public:
> >  	constexpr Vector() = default;
> > 
> > +	constexpr Vector(T scalar)
> > +	{
> > +		data_.fill(scalar);
> > +	}
> > +
> 
> Is there an expectation that e.g. `Vector<int, 3> x = 1` works? If not, then
> I think adding `explicit` would be something to consider to avoid erroneous
> construction from a scalar, especially in function calls.

Good point. Will fix in v4.

> >  	constexpr Vector(const std::array<T, Rows> &data)
> >  	{
> >  		for (unsigned int i = 0; i < Rows; i++)

Patch
diff mbox series

diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp
index f14f155216f3..d414ba97e41e 100644
--- a/src/ipa/libipa/vector.cpp
+++ b/src/ipa/libipa/vector.cpp
@@ -29,7 +29,13 @@  namespace ipa {
 
 /**
  * \fn Vector::Vector()
- * \brief Construct a zero vector
+ * \brief Construct an uninitialized vector
+ */
+
+/**
+ * \fn Vector::Vector(T scalar)
+ * \brief Construct a vector filled with a \a scalar value
+ * \param[in] scalar The scalar value
  */
 
 /**
diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h
index b72ab9851aa3..be568eadfeac 100644
--- a/src/ipa/libipa/vector.h
+++ b/src/ipa/libipa/vector.h
@@ -35,6 +35,11 @@  class Vector
 public:
 	constexpr Vector() = default;
 
+	constexpr Vector(T scalar)
+	{
+		data_.fill(scalar);
+	}
+
 	constexpr Vector(const std::array<T, Rows> &data)
 	{
 		for (unsigned int i = 0; i < Rows; i++)