[libcamera-devel] [PATCH] libcamera: controls: Replace array designated initializer

[Jacopo Mondi]

Hi Laurent,

On Wed, May 13, 2020 at 03:01:28PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, May 13, 2020 at 11:12:55AM +0200, Jacopo Mondi wrote:
> > Since version 10.0 the LLVM clang++ compiler emits warning to report
> > usage of array designated initializer, a C99 feature that was only
> > supported through an extension of the compiler and will be not valid
> > anymore in C++20.
> >
> > From https://releases.llvm.org/10.0.0/tools/clang/docs/ReleaseNotes.html
> > The new warnings -Wc99-designator and -Wreorder-init-list warn about
> > uses of C99 initializers in C++ mode for cases that are valid in C99 but
> > not in C++20.
> >
> > This generates the following build error in the controls.cpp file:
> > ../src/libcamera/controls.cpp:54:2: error: array designators are a C99 extension [-Werror,-Wc99-designator]
> >         [ControlTypeNone]               = 0,
> >         ^~~~~~~~~~~~~~~~~
> >
> > Fix this by replacing the plain C array with an std:map<>.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >
> > ---
> > I would have liked the map to be immutable, but I can't due to missing
> > overloading for operator[]:
> > error: no viable overloaded operator[] for type 'const std::map<unsigned int, size_t>'
> >
> > Tested with clang++ 10.0.0 and g++ 9.3.0.1
> > ---
> >  src/libcamera/controls.cpp | 21 +++++++++++----------
> >  1 file changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 08df7f29e938..eaef45a5f62c 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -8,6 +8,7 @@
> >  #include <libcamera/controls.h>
> >
> >  #include <iomanip>
> > +#include <map>
> >  #include <sstream>
> >  #include <string>
> >  #include <string.h>
> > @@ -50,16 +51,16 @@ LOG_DEFINE_CATEGORY(Controls)
> >
> >  namespace {
> >
> > -static constexpr size_t ControlValueSize[] = {
> > -	[ControlTypeNone]		= 0,
> > -	[ControlTypeBool]		= sizeof(bool),
> > -	[ControlTypeByte]		= sizeof(uint8_t),
> > -	[ControlTypeInteger32]		= sizeof(int32_t),
> > -	[ControlTypeInteger64]		= sizeof(int64_t),
> > -	[ControlTypeFloat]		= sizeof(float),
> > -	[ControlTypeString]		= sizeof(char),
> > -	[ControlTypeRectangle]		= sizeof(Rectangle),
> > -	[ControlTypeSize]		= sizeof(Size),
> > +std::map<unsigned int, size_t> ControlValueSize = {
> > +	{ ControlTypeNone,	0  },
> > +	{ ControlTypeBool,	sizeof(bool) },
> > +	{ ControlTypeByte,	sizeof(uint8_t) },
> > +	{ ControlTypeInteger32,	sizeof(int32_t) },
> > +	{ ControlTypeInteger64,	sizeof(int64_t) },
> > +	{ ControlTypeFloat,	sizeof(float) },
> > +	{ ControlTypeString,	sizeof(char) },
> > +	{ ControlTypeRectangle,	sizeof(Rectangle) },
> > +	{ ControlTypeSize,	sizeof(Size) },
> >  };
>
> Efficiency will take a hit :-S And making it non-const is asking for
> trouble. I think we need a better solution.
>
> First of all I'd like to know where the warning comes from, as I don't
> hit it when compiling with clang++ 10.0.0.

Please ignore this. I have gone through a compiler version update and
the build directory had to be nuked and re-created otherwise
-Wno-c99-designator does not get collected.

False alarm :)

>
> >
> >  } /* namespace */
>
> --
> Regards,
>
> Laurent Pinchart