Re: [PATCH] jit: Ensure ssize_t is defined.

[Richard Biener <rguenther@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2530 bytes --]

On Tue, 11 Jun 2024, Richard Biener wrote:

> On Tue, 11 Jun 2024, Iain Sandoe wrote:
> 
> > 
> > 
> > > On 11 Jun 2024, at 08:44, Jakub Jelinek <jakub@redhat.com> wrote:
> > > 
> > > On Tue, Jun 11, 2024 at 09:27:37AM +0200, Richard Biener wrote:
> > >> On Tue, 11 Jun 2024, FX Coudert wrote:
> > >> 
> > >>> Hi
> > >>> 
> > >>> I can’t seem to get a review of this one-line patch. Could a global reviewer help?
> > >> 
> > >> While stdio.h can be relied on to exist I do not think you can assume
> > >> the same for sys/types.h without "configury", but libgccjit.h is an
> > >> installed API.  I would assume including stdlib.h gets you ssize_t as 
> > >> well?
> > > 
> > > If stdlib.h includes sys/types.h like often on Linux, yes, but not
> > > necessarily.  ssize_t is a POSIX type and it might be solely in sys/types.h.
> > 
> > .. and that is the case for at least some affected Darwin versions (stdlib.h
> > does not include sys/types.h).
> > 
> > > Perhaps libgccjit.h could use
> > > #ifdef __has_include
> > > #if __has_include (<sys/types.h>)
> > > #include <sys/types.h>
> > > #endif
> > > #endif
> > 
> > That seems like a good solution to me.
> > (my original patch was conditional on __APPLE__ but it seems that the
> >  issue could exist on other platforms too).
> 
> Don't you also need to add
> 
> approrpiate #define _POSIX_C_SOURCE or #define _XOPE_SOURCE befor the
> include in case somebody builds with -std=c99?

Oh, and the manpage says that <stdio.h> also defines ssize_t which
is a bit odd since we already include that ...

Richard.

> Richard.
> 
> > Iain
> > 
> > > instead of just #include <sys/types.h>.
> > > When compiled by gcc, one can use hacks like
> > > #define unsigned signed
> > > typedef __SIZE_TYPE__ gcc_jit_ssize_t;
> > > #undef unsigned
> > > but that might not work with other compilers and is perhaps
> > > just too ugly.
> > > 
> > >> In fact the C11 standard doesn't even mention ssize_t so the
> > >> API should probably avoid using it and instead use size_t for
> > >> 
> > >> /* Given type "T", get its size.
> > >>   This API entrypoint was added in LIBGCCJIT_ABI_20; you can test for its
> > >>   presence using
> > >>     #ifdef LIBGCCJIT_HAVE_SIZED_INTEGERS  */
> > >> extern ssize_t
> > >> gcc_jit_type_get_size (gcc_jit_type *type);
> > > 
> > > 	Jakub
> > 
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)