Fix for FreeBSD with static_assert

[apraga]

On FreeBSD kitty fails to build due to "type specifier missing" in
static_assert.
We currently patch it to replace it with _Static_assert.
This pull requests aims to take the change upstream .
Discussion here https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=265393

[kovidgoyal]

static_assert is part of the C11 standard. If its not in the freebsd headers that's a bug there. It is supposed to be defined as a macro that expands to _Static_assert. kitty is compiled with -std c11 so this macro must be defined. The proper fix is to figure out why its not defined for you. And if you cant figure that out, then a better fix is to use

#ifndef static_assert
#define static_assert _Static_assert
#endif

[nunotexbsd]

Hi,

We went to a similar problem with xxHash issue and is said "use _Static_assert instead, which is directly part of the C11 language".

I'm not a programmer but indeed that fix was applied by upstream.

Thanks

[kovidgoyal]

static_assert is also part of the C11 standard and the reason it is prefered
to _Static_assert in header files is that static_assert works in both C
and C++

[nunotexbsd]

You have a point on that and it seems to be clang related because FreeBSD kitty-0.25.2 builds:

• 14 current: clang 14.0.5: OK
• 13.1 Release: clang 13.0.0: OK
• 12.3 Release: clang 10.0.1 NOT OK[1]

[1] build log:

(...)
[12/102] Compiling kitty/state.c ...
In file included from kitty/parser.c:10:
kitty/data-types.h:167:15: error: expected parameter declarator
static_assert(sizeof(GPUCell) == 20, "Fix the ordering of GPUCell");
              ^
kitty/data-types.h:167:15: error: expected ')'
kitty/data-types.h:167:14: note: to match this '('
static_assert(sizeof(GPUCell) == 20, "Fix the ordering of GPUCell");
             ^
kitty/data-types.h:167:1: error: type specifier missing, defaults to 'int' [-Werror,-Wimplicit-int]
static_assert(sizeof(GPUCell) == 20, "Fix the ordering of GPUCell");
^
kitty/data-types.h:167:14: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
static_assert(sizeof(GPUCell) == 20, "Fix the ordering of GPUCell");
             ^
                                                                  void
kitty/data-types.h:174:15: error: expected parameter declarator
static_assert(sizeof(CPUCell) == 12, "Fix the ordering of CPUCell");
              ^
kitty/data-types.h:174:15: error: expected ')'
kitty/data-types.h:174:14: note: to match this '('
static_assert(sizeof(CPUCell) == 12, "Fix the ordering of CPUCell");
             ^
kitty/data-types.h:174:1: error: type specifier missing, defaults to 'int' [-Werror,-Wimplicit-int]
static_assert(sizeof(CPUCell) == 12, "Fix the ordering of CPUCell");
^
kitty/data-types.h:174:14: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
static_assert(sizeof(CPUCell) == 12, "Fix the ordering of CPUCell");
             ^
                                                                  void
8 errors generated.
 done
Compiling kitty/parser.c ...
cc -MMD -DNDEBUG -DEPOLL_SHIM_DISABLE_WRAPPER_MACROS -I/usr/local/include/libepoll-shim -DPRIMARY_VERSION=4000 -DSECONDARY_VERSION=25 -DXT_VERSION="0.25.2" -Wextra -Wfloat-conversion -Wno-missing-field-initializers -Wall -Wstrict-prototypes -std=c11 -pedantic-errors -Werror -O3 -fwrapv -fstack-protector-strong -pipe -fvisibility=hidden -D_FORTIFY_SOURCE=2 -fPIC -O2 -pipe -fstack-protector-strong -fno-strict-aliasing -flto -I/usr/local/include/ -DKITTY_HAS_RS_SIG_ARGS -pthread -I/usr/local/include/libpng16 -I/usr/local/include -I/usr/local/include -I/usr/local/include/freetype2 -I/usr/local/include/libpng16 -I/usr/local/include/harfbuzz -I/usr/local/include/freetype2 -I/usr/local/include/libpng16 -I/usr/local/include -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include/python3.9 -c kitty/parser.c -o build/fast_data_types-parser.c.o
*** Error code 1

Stop.
make: stopped in /usr/ports/x11/kitty
build of x11/kitty | kitty-0.25.2 ended at Sun Aug 21 10:54:13 WEST 2022
build time: 00:00:28
!!! build failure encountered !!!

Any hints?

Thanks

[kovidgoyal]

I suggest simply patching locally if you are trying to build with ancient clang. The patch I posted in my previous comment should work fine. If it does, let me know and I will see about adding it to kitty's code.

[nunotexbsd]

It has already been patched on commit hash: 719265f when port was updated to latest version.

I could have applied this patch only for 12.3 Release, but when I did commit it, I applied patch for all official releases.

[kovidgoyal]

Your patch is OK as long as kitty never happens to use data-types.h in a
c++ file, if it does it will break.

[nunotexbsd]

I'm ok with that, this was the only workaround found to avoid "port broken" on 12.3.

Well I could patch it only for 12.3 or wait that upstream do a change that fix it for ancient clang using a correct way to do it.

[nunotexbsd]

static_assert is part of the C11 standard. If its not in the freebsd headers that's a bug there. It is supposed to be defined as a macro that expands to _Static_assert. kitty is compiled with -std c11 so this macro must be defined. The proper fix is to figure out why its not defined for you. And if you cant figure that out, then a better fix is to use

#ifndef static_assert
#define static_assert _Static_assert
#endif

Sorry I was a little bit lost thinking port patch was the recommended by upstream.

I've tested your recomendation and port builds in all releases (including ancient clang 10.0.0.1):

--- kitty/data-types.h.orig     2022-08-22 11:08:26 UTC
+++ kitty/data-types.h
@@ -159,6 +159,10 @@ typedef union CellAttrs {
 #define NUM_UNDERLINE_STYLES (5u)
 #define SGR_MASK (~(((CellAttrs){.width=WIDTH_MASK, .mark=MARK_MASK}).val))

+#ifndef static_assert
+#define static_assert _Static_assert
+#endif
+
 typedef struct {
     color_type fg, bg, decoration_fg;
     sprite_index sprite_x, sprite_y, sprite_z;

Now I can use an if condition to apply it only were it is needed, 12.3, ancient clang.

What you think the best option is?

Thanks

[kovidgoyal]

On Mon, Aug 22, 2022 at 05:51:45AM -0700, nunotexbsd wrote: Now I can use an if condition to apply it only were it is needed, 12.3, ancient clang. What you think the best option is?

Do that, I am not interested in upstreaming this patch given it is for one particular ancient compiler version one one OS. The patch itself should be fairly robust.

[nunotexbsd]

Thank you very much for your help @kovidgoyal.

https://cgit.freebsd.org/ports/commit/?id=adf824b09c349ee00ac25189c2f98e3fc2c48953

Cheers