[v2,12/42] libcamera: yaml_parser: Add function to set a YamlObject value
diff mbox series

Message ID 20260407153427.1825999-13-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: Global configuration file improvements
Related show

Commit Message

Laurent Pinchart April 7, 2026, 3:33 p.m. UTC
Add a YamlObject::set() function to set the value of an object, with
specializations for scalar types.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Remove cv in addition to reference in YamlObject::set()
---
 include/libcamera/internal/yaml_parser.h |  8 ++++
 src/libcamera/yaml_parser.cpp            | 59 ++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

Comments

Laurent Pinchart April 23, 2026, 7:48 p.m. UTC | #1
On Thu, Apr 23, 2026 at 03:59:25PM +0200, Barnabás Pőcze wrote:
> 2026. 04. 07. 17:33 keltezéssel, Laurent Pinchart írta:
> > Add a YamlObject::set() function to set the value of an object, with
> > specializations for scalar types.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Remove cv in addition to reference in YamlObject::set()
> > ---
> >   include/libcamera/internal/yaml_parser.h |  8 ++++
> >   src/libcamera/yaml_parser.cpp            | 59 ++++++++++++++++++++++++
> >   2 files changed, 67 insertions(+)
> > 
> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> > index 7953befe11e2..0666762308ac 100644
> > --- a/include/libcamera/internal/yaml_parser.h
> > +++ b/include/libcamera/internal/yaml_parser.h
> > @@ -182,6 +182,13 @@ public:
> >   		return get<T>().value_or(std::forward<U>(defaultValue));
> >   	}
> >   
> > +	template<typename T>
> > +	void set(T &&value)
> > +	{
> > +		return Accessor<std::remove_cv_t<std::remove_reference_t<T>>>{}
> > +			.set(*this, std::forward<T>(value));
> > +	}
> > +
> >   	DictAdapter asDict() const { return DictAdapter{ list_ }; }
> >   	ListAdapter asList() const { return ListAdapter{ list_ }; }
> >   
> > @@ -207,6 +214,7 @@ private:
> >   	template<typename T, typename Enable = void>
> >   	struct Accessor {
> >   		std::optional<T> get(const YamlObject &obj) const;
> > +		void set(YamlObject &obj, T value);
> >   	};
> >   
> >   	Type type_;
> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> > index 399e7529385f..3119ab41d89e 100644
> > --- a/src/libcamera/yaml_parser.cpp
> > +++ b/src/libcamera/yaml_parser.cpp
> > @@ -137,6 +137,20 @@ std::size_t YamlObject::size() const
> >    * \return The YamlObject value, or \a defaultValue if parsing failed
> >    */
> >   
> > +/**
> > + * \fn template<typename T> YamlObject::set<T>(T &&value)
> > + * \brief Set the value of a YamlObject
> > + * \param[in] value The value
> > + *
> > + * This function sets the value stored in a YamlObject to \a value. The value is
> > + * converted to a string in an implementation-specific way that guarantees that
> > + * subsequent calls to get<T>() will return the same value.
> 
> Is this true for floating pointer types as well?

... no ... :-/

I would like it to be, and we could use std::to_chars() and
std::from_chars() to implement that. Despite being specified in C++17,
they're only available since gcc 11 and clang 20 for floating-point
types. On the gcc side, once we drop Debian Bullseye from CI this
summer, we'll be fine. For clang, however, even Trixie ships a too old
version, so we won't be able to offer a guarantee.

Do you have any recommendation on how to handle this ? Should I just
document that the promise doesn't hold true for floating point types ?

> > + *
> > + * Values can only be set on YamlObject of Type::Value type or empty YamlObject.
> > + * Attempting to set a value on an object of type Type::Dict or Type::List does
> > + * not modify the YamlObject.
> > + */
> > +
> >   #ifndef __DOXYGEN__
> >   
> >   template<>
> > @@ -154,6 +168,16 @@ YamlObject::Accessor<bool>::get(const YamlObject &obj) const
> >   	return std::nullopt;
> >   }
> >   
> > +template<>
> > +void YamlObject::Accessor<bool>::set(YamlObject &obj, bool value)
> > +{
> > +	if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
> > +		return;
> > +
> > +	obj.type_ = Type::Value;
> > +	obj.value_ = value ? "true" : "false";
> > +}
> > +
> >   template<typename T>
> >   struct YamlObject::Accessor<T, std::enable_if_t<
> >   	std::is_same_v<int8_t, T> ||
> > @@ -178,6 +202,15 @@ struct YamlObject::Accessor<T, std::enable_if_t<
> >   
> >   		return value;
> >   	}
> > +
> > +	void set(YamlObject &obj, T value)
> > +	{
> > +		if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
> > +			return;
> > +
> > +		obj.type_ = Type::Value;
> > +		obj.value_ = std::to_string(value);
> > +	}
> >   };
> >   
> >   template struct YamlObject::Accessor<int8_t>;
> > @@ -194,6 +227,12 @@ YamlObject::Accessor<float>::get(const YamlObject &obj) const
> >   	return obj.get<double>();
> >   }
> >   
> > +template<>
> > +void YamlObject::Accessor<float>::set(YamlObject &obj, float value)
> > +{
> > +	obj.set<double>(std::forward<float>(value));
> 
> This forwarding looks a bit unnecessary. In any case

I'll drop it.

> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> > +}
> > +
> >   template<>
> >   std::optional<double>
> >   YamlObject::Accessor<double>::get(const YamlObject &obj) const
> > @@ -215,6 +254,16 @@ YamlObject::Accessor<double>::get(const YamlObject &obj) const
> >   	return value;
> >   }
> >   
> > +template<>
> > +void YamlObject::Accessor<double>::set(YamlObject &obj, double value)
> > +{
> > +	if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
> > +		return;
> > +
> > +	obj.type_ = Type::Value;
> > +	obj.value_ = std::to_string(value);
> > +}
> > +
> >   template<>
> >   std::optional<std::string>
> >   YamlObject::Accessor<std::string>::get(const YamlObject &obj) const
> > @@ -225,6 +274,16 @@ YamlObject::Accessor<std::string>::get(const YamlObject &obj) const
> >   	return obj.value_;
> >   }
> >   
> > +template<>
> > +void YamlObject::Accessor<std::string>::set(YamlObject &obj, std::string value)
> > +{
> > +	if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
> > +		return;
> > +
> > +	obj.type_ = Type::Value;
> > +	obj.value_ = std::move(value);
> > +}
> > +
> >   template<>
> >   std::optional<Size>
> >   YamlObject::Accessor<Size>::get(const YamlObject &obj) const
Barnabás Pőcze April 24, 2026, 8:02 a.m. UTC | #2
2026. 04. 23. 21:48 keltezéssel, Laurent Pinchart írta:
> On Thu, Apr 23, 2026 at 03:59:25PM +0200, Barnabás Pőcze wrote:
>> 2026. 04. 07. 17:33 keltezéssel, Laurent Pinchart írta:
>>> Add a YamlObject::set() function to set the value of an object, with
>>> specializations for scalar types.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> ---
>>> Changes since v1:
>>>
>>> - Remove cv in addition to reference in YamlObject::set()
>>> ---
>>>    include/libcamera/internal/yaml_parser.h |  8 ++++
>>>    src/libcamera/yaml_parser.cpp            | 59 ++++++++++++++++++++++++
>>>    2 files changed, 67 insertions(+)
>>>
>>> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
>>> index 7953befe11e2..0666762308ac 100644
>>> --- a/include/libcamera/internal/yaml_parser.h
>>> +++ b/include/libcamera/internal/yaml_parser.h
>>> @@ -182,6 +182,13 @@ public:
>>>    		return get<T>().value_or(std::forward<U>(defaultValue));
>>>    	}
>>>    
>>> +	template<typename T>
>>> +	void set(T &&value)
>>> +	{
>>> +		return Accessor<std::remove_cv_t<std::remove_reference_t<T>>>{}
>>> +			.set(*this, std::forward<T>(value));
>>> +	}
>>> +
>>>    	DictAdapter asDict() const { return DictAdapter{ list_ }; }
>>>    	ListAdapter asList() const { return ListAdapter{ list_ }; }
>>>    
>>> @@ -207,6 +214,7 @@ private:
>>>    	template<typename T, typename Enable = void>
>>>    	struct Accessor {
>>>    		std::optional<T> get(const YamlObject &obj) const;
>>> +		void set(YamlObject &obj, T value);
>>>    	};
>>>    
>>>    	Type type_;
>>> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
>>> index 399e7529385f..3119ab41d89e 100644
>>> --- a/src/libcamera/yaml_parser.cpp
>>> +++ b/src/libcamera/yaml_parser.cpp
>>> @@ -137,6 +137,20 @@ std::size_t YamlObject::size() const
>>>     * \return The YamlObject value, or \a defaultValue if parsing failed
>>>     */
>>>    
>>> +/**
>>> + * \fn template<typename T> YamlObject::set<T>(T &&value)
>>> + * \brief Set the value of a YamlObject
>>> + * \param[in] value The value
>>> + *
>>> + * This function sets the value stored in a YamlObject to \a value. The value is
>>> + * converted to a string in an implementation-specific way that guarantees that
>>> + * subsequent calls to get<T>() will return the same value.
>>
>> Is this true for floating pointer types as well?
> 
> ... no ... :-/
> 
> I would like it to be, and we could use std::to_chars() and
> std::from_chars() to implement that. Despite being specified in C++17,
> they're only available since gcc 11 and clang 20 for floating-point
> types. On the gcc side, once we drop Debian Bullseye from CI this
> summer, we'll be fine. For clang, however, even Trixie ships a too old
> version, so we won't be able to offer a guarantee.
> 
> Do you have any recommendation on how to handle this ? Should I just
> document that the promise doesn't hold true for floating point types ?

I think it is fine as is for now. I'd maybe drop this whole statement
or mention that "\todo Guarantee for floating point types".


> 
>>> + *
>>> + * Values can only be set on YamlObject of Type::Value type or empty YamlObject.
>>> + * Attempting to set a value on an object of type Type::Dict or Type::List does
>>> + * not modify the YamlObject.
>>> + */
>>> +
>>>    #ifndef __DOXYGEN__
>>>    
>>>    template<>
>>> @@ -154,6 +168,16 @@ YamlObject::Accessor<bool>::get(const YamlObject &obj) const
>>>    	return std::nullopt;
>>>    }
>>>    
>>> +template<>
>>> +void YamlObject::Accessor<bool>::set(YamlObject &obj, bool value)
>>> +{
>>> +	if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
>>> +		return;
>>> +
>>> +	obj.type_ = Type::Value;
>>> +	obj.value_ = value ? "true" : "false";
>>> +}
>>> +
>>>    template<typename T>
>>>    struct YamlObject::Accessor<T, std::enable_if_t<
>>>    	std::is_same_v<int8_t, T> ||
>>> @@ -178,6 +202,15 @@ struct YamlObject::Accessor<T, std::enable_if_t<
>>>    
>>>    		return value;
>>>    	}
>>> +
>>> +	void set(YamlObject &obj, T value)
>>> +	{
>>> +		if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
>>> +			return;
>>> +
>>> +		obj.type_ = Type::Value;
>>> +		obj.value_ = std::to_string(value);
>>> +	}
>>>    };
>>>    
>>>    template struct YamlObject::Accessor<int8_t>;
>>> @@ -194,6 +227,12 @@ YamlObject::Accessor<float>::get(const YamlObject &obj) const
>>>    	return obj.get<double>();
>>>    }
>>>    
>>> +template<>
>>> +void YamlObject::Accessor<float>::set(YamlObject &obj, float value)
>>> +{
>>> +	obj.set<double>(std::forward<float>(value));
>>
>> This forwarding looks a bit unnecessary. In any case
> 
> I'll drop it.
> 
>> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>
>>> +}
>>> +
>>>    template<>
>>>    std::optional<double>
>>>    YamlObject::Accessor<double>::get(const YamlObject &obj) const
>>> @@ -215,6 +254,16 @@ YamlObject::Accessor<double>::get(const YamlObject &obj) const
>>>    	return value;
>>>    }
>>>    
>>> +template<>
>>> +void YamlObject::Accessor<double>::set(YamlObject &obj, double value)
>>> +{
>>> +	if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
>>> +		return;
>>> +
>>> +	obj.type_ = Type::Value;
>>> +	obj.value_ = std::to_string(value);
>>> +}
>>> +
>>>    template<>
>>>    std::optional<std::string>
>>>    YamlObject::Accessor<std::string>::get(const YamlObject &obj) const
>>> @@ -225,6 +274,16 @@ YamlObject::Accessor<std::string>::get(const YamlObject &obj) const
>>>    	return obj.value_;
>>>    }
>>>    
>>> +template<>
>>> +void YamlObject::Accessor<std::string>::set(YamlObject &obj, std::string value)
>>> +{
>>> +	if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
>>> +		return;
>>> +
>>> +	obj.type_ = Type::Value;
>>> +	obj.value_ = std::move(value);
>>> +}
>>> +
>>>    template<>
>>>    std::optional<Size>
>>>    YamlObject::Accessor<Size>::get(const YamlObject &obj) const
>

Patch
diff mbox series

diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
index 7953befe11e2..0666762308ac 100644
--- a/include/libcamera/internal/yaml_parser.h
+++ b/include/libcamera/internal/yaml_parser.h
@@ -182,6 +182,13 @@  public:
 		return get<T>().value_or(std::forward<U>(defaultValue));
 	}
 
+	template<typename T>
+	void set(T &&value)
+	{
+		return Accessor<std::remove_cv_t<std::remove_reference_t<T>>>{}
+			.set(*this, std::forward<T>(value));
+	}
+
 	DictAdapter asDict() const { return DictAdapter{ list_ }; }
 	ListAdapter asList() const { return ListAdapter{ list_ }; }
 
@@ -207,6 +214,7 @@  private:
 	template<typename T, typename Enable = void>
 	struct Accessor {
 		std::optional<T> get(const YamlObject &obj) const;
+		void set(YamlObject &obj, T value);
 	};
 
 	Type type_;
diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
index 399e7529385f..3119ab41d89e 100644
--- a/src/libcamera/yaml_parser.cpp
+++ b/src/libcamera/yaml_parser.cpp
@@ -137,6 +137,20 @@  std::size_t YamlObject::size() const
  * \return The YamlObject value, or \a defaultValue if parsing failed
  */
 
+/**
+ * \fn template<typename T> YamlObject::set<T>(T &&value)
+ * \brief Set the value of a YamlObject
+ * \param[in] value The value
+ *
+ * This function sets the value stored in a YamlObject to \a value. The value is
+ * converted to a string in an implementation-specific way that guarantees that
+ * subsequent calls to get<T>() will return the same value.
+ *
+ * Values can only be set on YamlObject of Type::Value type or empty YamlObject.
+ * Attempting to set a value on an object of type Type::Dict or Type::List does
+ * not modify the YamlObject.
+ */
+
 #ifndef __DOXYGEN__
 
 template<>
@@ -154,6 +168,16 @@  YamlObject::Accessor<bool>::get(const YamlObject &obj) const
 	return std::nullopt;
 }
 
+template<>
+void YamlObject::Accessor<bool>::set(YamlObject &obj, bool value)
+{
+	if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
+		return;
+
+	obj.type_ = Type::Value;
+	obj.value_ = value ? "true" : "false";
+}
+
 template<typename T>
 struct YamlObject::Accessor<T, std::enable_if_t<
 	std::is_same_v<int8_t, T> ||
@@ -178,6 +202,15 @@  struct YamlObject::Accessor<T, std::enable_if_t<
 
 		return value;
 	}
+
+	void set(YamlObject &obj, T value)
+	{
+		if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
+			return;
+
+		obj.type_ = Type::Value;
+		obj.value_ = std::to_string(value);
+	}
 };
 
 template struct YamlObject::Accessor<int8_t>;
@@ -194,6 +227,12 @@  YamlObject::Accessor<float>::get(const YamlObject &obj) const
 	return obj.get<double>();
 }
 
+template<>
+void YamlObject::Accessor<float>::set(YamlObject &obj, float value)
+{
+	obj.set<double>(std::forward<float>(value));
+}
+
 template<>
 std::optional<double>
 YamlObject::Accessor<double>::get(const YamlObject &obj) const
@@ -215,6 +254,16 @@  YamlObject::Accessor<double>::get(const YamlObject &obj) const
 	return value;
 }
 
+template<>
+void YamlObject::Accessor<double>::set(YamlObject &obj, double value)
+{
+	if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
+		return;
+
+	obj.type_ = Type::Value;
+	obj.value_ = std::to_string(value);
+}
+
 template<>
 std::optional<std::string>
 YamlObject::Accessor<std::string>::get(const YamlObject &obj) const
@@ -225,6 +274,16 @@  YamlObject::Accessor<std::string>::get(const YamlObject &obj) const
 	return obj.value_;
 }
 
+template<>
+void YamlObject::Accessor<std::string>::set(YamlObject &obj, std::string value)
+{
+	if (obj.type_ != Type::Empty && obj.type_ != Type::Value)
+		return;
+
+	obj.type_ = Type::Value;
+	obj.value_ = std::move(value);
+}
+
 template<>
 std::optional<Size>
 YamlObject::Accessor<Size>::get(const YamlObject &obj) const