Re: [PATCH] Re: Patchset related to array functions

[Andy Wingo <wingo@pobox.com>]

On Thu 14 Jul 2016 17:41, Daniel Llorens <daniel.llorens@bluewin.ch> writes:

> On 14 Jul 2016, at 11:46, Andy Wingo <wingo@pobox.com> wrote:
>
>>  (1) Can we support C99 on all targets we care about?
>
> Emacs

http://git.savannah.gnu.org/cgit/emacs.git/tree/configure.ac#n764

"Emacs needs C99".  Sweet!  We check this point off.

>>  (2) Can we use C99 in our public interface, or just internally?  If we
>>      use it publically, what should we change?  No more scm_t_uint8 I
>>      hope, besides for back-compat?  This patch set does not have to
>>      include these changes, but we should have a plan.
>
> I think we'd want C89/C90 users to still be able to #include <libguile.h>. Dunno.

Really?  I would *love* to be able to say "just use c99, or at least
something with stdint.h".  Apparently gcc's default has been gnu11 for a
while...

>>  (3) Most importantly, what is the impact on inlining?  See the comment
>>      in __scm.h around line 165.
>
> Apparently the standard practice in C99 is to put inline definition in
> the header and extern declaration in the .c, while with ‘Guile inline’
> both SCM_INLINE and SCM_INLINE_IMPLEMENTATION are in the header.

I believe that Guile tries to do this as well.  By default the headers
define inline definitions and there is the extern inline declaration,
and then inline.c re-includes those headers with
SCM_INLINE_C_IMPLEMENTING_INLINES defined which reifies the definitions
so that the symbols end up in the .so.  But this landscape is quite
gnarly.  The specific implementation actually relies on the gnu_inline
attribute, so I guess we are using GNU extensions either way, at least
when compiled with GCC...

> I can try to fix Guile to follow the C99 practice and remove most of
> the #define guards. Would a patch doing this be accepted? I'd welcome
> advice on how to test such a patch. E.g. with both -O2 and -O0 or
> so. I'm mostly a C++ programmer and I don't want to mess anything up.

I think the concerns are:

 (1) Do inlined definitions get inlined?
 (2) Are external definitions reified as well?
 (3) Do we avoid reifying definitions in each compilation unit?
 (4) Can you dlsym() an inline function?

All these answers should be yes.  No benchmarking needed, just
inspection of the build artifacts under different configurations.

>> If you want your patch set to depend on C99 that's fine, but you have to
>> answer these questions first for the project as a whole and get some
>> consensus.
>
> That is a very reasonable viewpoint. Since C99 was just a minor
> convenience to me, I withdraw that particular patch. I have rebased
> everything to avoid the C99 requirement. Revised patchset attached.

Tx, will review separately.  In the future would you mind please
spamming the list with these patches as a thread of multiple mails, as
git-send-email would do?  That makes it easy for me to review just one
patch, say LGTM or whatever on that patch, then work on other patches on
other days.  But I will make an initial pass on this mail, later though
:)

Andy