public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Iain Sandoe <iain@sandoe.co.uk>
To: Richard Biener <rguenther@suse.de>
Cc: Jakub Jelinek <jakub@redhat.com>,
	FX Coudert <fxcoudert@gmail.com>,
	GCC Patches <gcc-patches@gcc.gnu.org>,
	dmalcolm@redhat.com, Eric Gallager <egall@gwmail.gwu.edu>,
	josmyers@redhat.com
Subject: Re: [PATCH] jit: Ensure ssize_t is defined.
Date: Tue, 11 Jun 2024 09:16:50 +0100	[thread overview]
Message-ID: <FF05C582-E0EB-4C09-8D12-2C1547CE3A4D@sandoe.co.uk> (raw)
In-Reply-To: <o5q2090r-5p18-q5rr-35r1-no195qrp8p1q@fhfr.qr>



> On 11 Jun 2024, at 09:06, Richard Biener <rguenther@suse.de> wrote:
> 
> 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?

well, afaict, all the code is c++ and we are building with a std >= 11, so that
presumes c99 support.  I can confirm that, at least on Darwin, we do not need
to define either _POSIX_C_SOURCE, or _XOPEN_SOURCE to get the 
correct behaviour.

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

that seems to be the case for darwin >= 11 but not before.

Iain

> 
> 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)


  reply	other threads:[~2024-06-11  8:16 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-02 19:48 FX Coudert
2024-05-11 15:16 ` FX Coudert
2024-05-26 15:35   ` FX Coudert
2024-06-01 16:44   ` FX Coudert
2024-06-11  6:06   ` FX Coudert
2024-06-11  7:27     ` Richard Biener
2024-06-11  7:44       ` Jakub Jelinek
2024-06-11  8:03         ` Iain Sandoe
2024-06-11  8:04           ` Richard Biener
2024-06-11  8:06             ` Richard Biener
2024-06-11  8:16               ` Iain Sandoe [this message]
2024-06-11  8:34                 ` Andreas Schwab
2024-06-11  8:20               ` Jakub Jelinek
2024-06-11  8:21               ` Andreas Schwab
2024-06-27 17:08         ` FX Coudert
2024-06-28  6:17           ` Richard Biener
2024-06-28  7:15             ` FX Coudert
2024-09-07 17:35           ` FX Coudert
2024-09-18 10:28             ` FX Coudert
2024-09-18 10:53               ` Richard Biener
2024-09-18 17:53                 ` FX Coudert
2024-06-11  8:21       ` FX Coudert
2024-06-11  7:20   ` Xi Ruoyao
  -- strict thread matches above, loose matches on Subject: below --
2024-01-28 11:44 Iain Sandoe
2024-01-28 21:25 ` Eric Gallager
2024-01-28 23:13   ` Iain Sandoe
2024-01-29 11:26     ` Iain Sandoe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=FF05C582-E0EB-4C09-8D12-2C1547CE3A4D@sandoe.co.uk \
    --to=iain@sandoe.co.uk \
    --cc=dmalcolm@redhat.com \
    --cc=egall@gwmail.gwu.edu \
    --cc=fxcoudert@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=josmyers@redhat.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).