Re: [PATCH v17.1 01/14] helpers: Move static_assert definition to shared

2018-07-09 Thread Pekka Paalanen
On Mon,  9 Jul 2018 15:27:46 +0100
Daniel Stone  wrote:

> Collect the fallback definitions of static_assert() from desktop-shell
> and the test shell, and move them to helpers.h. This allows code
> throughout the tree to use static_assert() for build-time assertions,
> where it is supported by the compiler.
> 
> As GCC goes out of its way to only add static_assert() when C11 has been
> explicitly requested - which we don't do - make sure to use the more
> widely available _Static_assert() if that is provided.
> 
> This will be used in future patches to ensure two array lengths don't go
> out of sync.
> 
> Signed-off-by: Daniel Stone 
> ---
>  desktop-shell/shell.c |  4 
>  shared/helpers.h  | 33 +++
>  tests/weston-test-desktop-shell.c |  4 
>  3 files changed, 33 insertions(+), 8 deletions(-)
> 
> On Mon, 9 Jul 2018 at 14:54, Pekka Paalanen  wrote:
> > I'm fairly paranoid of copying GPL code. The implementation sure is
> > trivial, but what exactly did you take from the kernel?
> > The documentation bit looks already copyrightable.  
> 
> They probably have better documentation. :) I wrote that documentation
> myself, so it's just the macro body which was taken from the kernel. My
> understanding is that this isn't copyrightable.
> 
> > Did you know of the existing uses of static_assert in Weston?
> > Oh, but there are no uses left, all we have left are the fallback empty
> > definitions... two of them. >_<  
> 
> But that's a good point. How about the following patch instead, which
> uses static_assert() where possible? It turns out static_assert() was
> actually mostly not defined, but I tried this one with:
> static_assert(1 != 0, "must not trigger");
> static_assert(1 == 0, "must trigger");
> 
> ... and that worked.
> 
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 8b7a23ade..ea3c45354 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -47,10 +47,6 @@
>  #define DEFAULT_NUM_WORKSPACES 1
>  #define DEFAULT_WORKSPACE_CHANGE_ANIMATION_LENGTH 200
>  
> -#ifndef static_assert
> -#define static_assert(cond, msg)
> -#endif
> -
>  struct focus_state {
>   struct desktop_shell *shell;
>   struct weston_seat *seat;
> diff --git a/shared/helpers.h b/shared/helpers.h
> index 46f745d1b..9e1de947e 100644
> --- a/shared/helpers.h
> +++ b/shared/helpers.h
> @@ -100,6 +100,39 @@ extern "C" {
>   (type *)( (char *)__mptr - offsetof(type,member) );})
>  #endif
>  
> +/**
> + * Build-time static assertion support
> + *
> + * A build-time equivalent to assert(), will generate a compilation error
> + * if the supplied condition does not evaluate true.
> + *
> + * The following example demonstrates use of static_assert to ensure that
> + * arrays which are supposed to mirror each other have a consistent
> + * size.
> + *
> + * This is only a fallback definition; support must be provided by the
> + * compiler itself.
> + *
> + * @code
> + * int small[4];
> + * long expanded[4];
> + *
> + * static_assert(ARRAY_LENGTH(small) != ARRAY_LENGTH(expanded));

I think this condition is inverted.


> + * for (i = 0; i < ARRAY_LENGTH(small); i++)
> + * expanded[i] = small[4];
> + * @endcode
> + *
> + * @param condition Expression to check for truth
> + * @param msg Message to print on failure
> + */
> +#ifndef static_assert
> +# ifdef _Static_assert
> +#  define static_assert(cond, msg) _Static_assert(cond, msg)
> +# else
> +#  define static_assert(cond, msg)
> +# endif
> +#endif
> +

If one wanted to be really fancy, the empty fallback definition could
be the -1 elements array trick. But this is enough for me already.

The doc fixed:
Reviewed-by: Pekka Paalanen 


Thanks,
pq

>  #ifdef  __cplusplus
>  }
>  #endif
> diff --git a/tests/weston-test-desktop-shell.c 
> b/tests/weston-test-desktop-shell.c
> index de8442512..c780316d9 100644
> --- a/tests/weston-test-desktop-shell.c
> +++ b/tests/weston-test-desktop-shell.c
> @@ -41,10 +41,6 @@
>  #include "shared/helpers.h"
>  #include "libweston-desktop/libweston-desktop.h"
>  
> -#ifndef static_assert
> -#define static_assert(cond, msg)
> -#endif
> -
>  struct desktest_shell {
>   struct wl_listener compositor_destroy_listener;
>   struct weston_desktop *desktop;



pgpyGrHRx16bN.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v17.1 01/14] helpers: Move static_assert definition to shared

2018-07-09 Thread Daniel Stone
Collect the fallback definitions of static_assert() from desktop-shell
and the test shell, and move them to helpers.h. This allows code
throughout the tree to use static_assert() for build-time assertions,
where it is supported by the compiler.

As GCC goes out of its way to only add static_assert() when C11 has been
explicitly requested - which we don't do - make sure to use the more
widely available _Static_assert() if that is provided.

This will be used in future patches to ensure two array lengths don't go
out of sync.

Signed-off-by: Daniel Stone 
---
 desktop-shell/shell.c |  4 
 shared/helpers.h  | 33 +++
 tests/weston-test-desktop-shell.c |  4 
 3 files changed, 33 insertions(+), 8 deletions(-)

On Mon, 9 Jul 2018 at 14:54, Pekka Paalanen  wrote:
> I'm fairly paranoid of copying GPL code. The implementation sure is
> trivial, but what exactly did you take from the kernel?
> The documentation bit looks already copyrightable.

They probably have better documentation. :) I wrote that documentation
myself, so it's just the macro body which was taken from the kernel. My
understanding is that this isn't copyrightable.

> Did you know of the existing uses of static_assert in Weston?
> Oh, but there are no uses left, all we have left are the fallback empty
> definitions... two of them. >_<

But that's a good point. How about the following patch instead, which
uses static_assert() where possible? It turns out static_assert() was
actually mostly not defined, but I tried this one with:
static_assert(1 != 0, "must not trigger");
static_assert(1 == 0, "must trigger");

... and that worked.


diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 8b7a23ade..ea3c45354 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -47,10 +47,6 @@
 #define DEFAULT_NUM_WORKSPACES 1
 #define DEFAULT_WORKSPACE_CHANGE_ANIMATION_LENGTH 200
 
-#ifndef static_assert
-#define static_assert(cond, msg)
-#endif
-
 struct focus_state {
struct desktop_shell *shell;
struct weston_seat *seat;
diff --git a/shared/helpers.h b/shared/helpers.h
index 46f745d1b..9e1de947e 100644
--- a/shared/helpers.h
+++ b/shared/helpers.h
@@ -100,6 +100,39 @@ extern "C" {
(type *)( (char *)__mptr - offsetof(type,member) );})
 #endif
 
+/**
+ * Build-time static assertion support
+ *
+ * A build-time equivalent to assert(), will generate a compilation error
+ * if the supplied condition does not evaluate true.
+ *
+ * The following example demonstrates use of static_assert to ensure that
+ * arrays which are supposed to mirror each other have a consistent
+ * size.
+ *
+ * This is only a fallback definition; support must be provided by the
+ * compiler itself.
+ *
+ * @code
+ * int small[4];
+ * long expanded[4];
+ *
+ * static_assert(ARRAY_LENGTH(small) != ARRAY_LENGTH(expanded));
+ * for (i = 0; i < ARRAY_LENGTH(small); i++)
+ * expanded[i] = small[4];
+ * @endcode
+ *
+ * @param condition Expression to check for truth
+ * @param msg Message to print on failure
+ */
+#ifndef static_assert
+# ifdef _Static_assert
+#  define static_assert(cond, msg) _Static_assert(cond, msg)
+# else
+#  define static_assert(cond, msg)
+# endif
+#endif
+
 #ifdef  __cplusplus
 }
 #endif
diff --git a/tests/weston-test-desktop-shell.c 
b/tests/weston-test-desktop-shell.c
index de8442512..c780316d9 100644
--- a/tests/weston-test-desktop-shell.c
+++ b/tests/weston-test-desktop-shell.c
@@ -41,10 +41,6 @@
 #include "shared/helpers.h"
 #include "libweston-desktop/libweston-desktop.h"
 
-#ifndef static_assert
-#define static_assert(cond, msg)
-#endif
-
 struct desktest_shell {
struct wl_listener compositor_destroy_listener;
struct weston_desktop *desktop;
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel