[libcamera-devel] libcamera: controls: Replace array designated initializer
diff mbox series

Message ID 20200513091255.7109-1-jacopo@jmondi.org
State Superseded, archived
Delegated to: Jacopo Mondi
Headers show
Series
  • [libcamera-devel] libcamera: controls: Replace array designated initializer
Related show

Commit Message

Jacopo Mondi May 13, 2020, 9:12 a.m. UTC 
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@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(-)

--
2.26.2

Comments

Laurent Pinchart May 13, 2020, 12:01 p.m. UTC | #1 
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@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.

> 
>  } /* namespace */
Jacopo Mondi May 13, 2020, 12:41 p.m. UTC | #2 
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@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

Patch
diff mbox series 

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) },
 };

 } /* namespace */