[PATCH v1] libcamera: yaml_parser: Take string keys in `std::string_view`

[Kieran Bingham]

Quoting Barnabás Pőcze (2024-10-01 17:03:32)
> In many cases a static string literal is used as key. Thus
> having the argument type be `const std::string&` is suboptimal
> since an `std::string` object needs to be constructed before
> the call.
> 
> C++17 introduced `std::string_view`, using which the call
> can be done with less overhead, as the `std::string_view`
> is non-owning and may be passed in registers entirely.
> 
> So make `YamlObject::{contains,operator[]}` take the string keys
> in `std::string_view`s.
> 
> Unfortunately, that is not sufficient yet, because `std::map::find()`
> takes an reference to `const key_type`, which would be `const std::string&`
> in the case of `YamlParser`. However, with a transparent comparator
> such as `std::less<>` `std::map::find()` is able to accept any
> object as the argument, and it forwards it to the comparator.
> 
> So make `YamlParser::dictionary_` use `std::less<>` as the comparator
> to enable the use of `std::map::find()` with any type of argument.
> 
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>

This passes the CI and unit tests at
https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1281981
which includes tests covering the yaml-parser.

I don't think I would have spotted this ... but it looks and sounds
reasonable to me ...

Makes me wonder if sometimes we should just use a raw 'const char *'
(</me ducks>) ;-) but I think this gives us that with type/length safety
on top so seems like a win to me.

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

> ---
>  include/libcamera/internal/yaml_parser.h |  7 ++++---
>  src/libcamera/yaml_parser.cpp            | 11 ++++-------
>  2 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> index 6211ff4a..de452844 100644
> --- a/include/libcamera/internal/yaml_parser.h
> +++ b/include/libcamera/internal/yaml_parser.h
> @@ -12,6 +12,7 @@
>  #include <optional>
>  #include <stdint.h>
>  #include <string>
> +#include <string_view>
>  #include <vector>
>  
>  #include <libcamera/base/class.h>
> @@ -206,8 +207,8 @@ public:
>  
>         const YamlObject &operator[](std::size_t index) const;
>  
> -       bool contains(const std::string &key) const;
> -       const YamlObject &operator[](const std::string &key) const;
> +       bool contains(std::string_view key) const;
> +       const YamlObject &operator[](std::string_view key) const;
>  
>  private:
>         LIBCAMERA_DISABLE_COPY_AND_MOVE(YamlObject)
> @@ -232,7 +233,7 @@ private:
>  
>         std::string value_;
>         Container list_;
> -       std::map<std::string, YamlObject *> dictionary_;
> +       std::map<std::string, YamlObject *, std::less<>> dictionary_;
>  };
>  
>  class YamlParser final
> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> index 4784f2dc..7c0b341a 100644
> --- a/src/libcamera/yaml_parser.cpp
> +++ b/src/libcamera/yaml_parser.cpp
> @@ -481,16 +481,13 @@ const YamlObject &YamlObject::operator[](std::size_t index) const
>   *
>   * \return True if an element exists, false otherwise
>   */
> -bool YamlObject::contains(const std::string &key) const
> +bool YamlObject::contains(std::string_view key) const
>  {
> -       if (dictionary_.find(std::ref(key)) == dictionary_.end())
> -               return false;
> -
> -       return true;
> +       return dictionary_.find(key) != dictionary_.end();
>  }
>  
>  /**
> - * \fn YamlObject::operator[](const std::string &key) const
> + * \fn YamlObject::operator[](std::string_view key) const
>   * \brief Retrieve a member by name from the dictionary
>   *
>   * This function retrieve a member of a YamlObject by name. Only YamlObject
> @@ -500,7 +497,7 @@ bool YamlObject::contains(const std::string &key) const
>   *
>   * \return The YamlObject corresponding to the \a key member
>   */
> -const YamlObject &YamlObject::operator[](const std::string &key) const
> +const YamlObject &YamlObject::operator[](std::string_view key) const
>  {
>         if (type_ != Type::Dictionary)
>                 return empty;
> -- 
> 2.46.2
> 
>