unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: Andy Wingo <wingo@pobox.com>
To: Mark H Weaver <mhw@netris.org>
Cc: guile-devel <guile-devel@gnu.org>
Subject: Re: RFC: Foreign objects facility
Date: Sun, 27 Apr 2014 19:51:41 +0200	[thread overview]
Message-ID: <87oazm3bki.fsf@pobox.com> (raw)
In-Reply-To: <87a9b692ys.fsf@yeeloong.lan> (Mark H. Weaver's message of "Sun,  27 Apr 2014 12:00:59 -0400")

Hi Mark,

Thanks for the review!

On Sun 27 Apr 2014 18:00, Mark H Weaver <mhw@netris.org> writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> Here is the proposed C API:
>>
>>     SCM scm_make_foreign_object_type (SCM name, SCM slot_names,
>>                                       scm_t_struct_finalize finalizer);
>
> Shouldn't it be 'scm_t_struct_finalizer', with trailing 'r'?

Probably it should, but note that it's already defined -- see struct.h.
(My bad there, I think!)

>>     SCM scm_make_foreign_object_n (SCM type, size_t n, scm_t_bits vals[]);
>>
>>     scm_t_bits scm_foreign_object_ref (SCM obj, size_t n);
>>     void scm_foreign_object_set_x (SCM obj, size_t n, scm_t_bits val);
>
> I'm worried about the type-punning that's implied by this interface.

This issue exists already with the SMOB and struct interfaces.  I don't
think that the proposed API adds new hazards, though it does extend the
old ones.

> The C standards are now very strict about this sort of thing,

It's an incredible shame that C is making itself semantically less
capable as time goes on :-(

> For example, having recently researched this, I know that converting
> unsigned integers to signed integers is very awkward and potentially
> inefficient to do portably, so using 'scm_foreign_object_ref' to extract
> a signed integer will be a pain.  It would look something like this
> (untested):
>
> scm_t_signed_bits
> scm_foreign_object_signed_ref (SCM obj, size_t n)
> {
>   scm_t_bits bits = scm_foreign_object_ref (obj, n);
>
> /* GNU C specifies that when casting to a signed integer of width N, the
>    value is reduced modulo 2^N to be within range of the type.  Neither
>    C99 nor C11 make any guarantees about casting an out-of-range value
>    to a signed integer type.  */
>
> #ifndef __GNUC__
>   if (bits > SCM_T_SIGNED_BITS_MAX)
>     return -1 - (scm_t_signed_bits) ~bits;
> #endif
>   return (scm_t_signed_bits) bits;
> }

True sadness.

> Portability is more problematic for pointer types.  The C standards make
> no guarantees about the semantics of converting between pointers and
> integers, and it's not clear to me how future proof this will be.

Don't they make some guarantees wrt uintptr_t and intptr_t ?

> One related issue is that I'd like to leave open the possibility that
> 'scm_t_bits' might be larger than a pointer.

You prefer to have three interface then: one for uintptr_t, one for
intptr_t, and one for void*?

> For constructors, I think we should provide a macro that handles the
> conversion from pointer to scm_t_bits.  (Converting from signed to
> unsigned is portable, so there's no issue there).

This is really gross...

>> The overhead of a foreign object is two words -- the same as the
>> overhead on any struct.  (Compare to SMOBs, which have a half-word
>> overhead.)
>
> It would be good to think about how we might someday reduce this
> overhead in the future, and to make sure we don't make decisions that
> would prevent us from doing that.

This is an orthogonal issue, I think, and would like to punt on that for
now.

>> +  scm_t_bits vals[2] = { val0, val1 };
>
> This non-constant initializer depends on C99.

Will fix.

>> +    if (scm_i_symbol_ref (layout, i * 2) != 'u')
>> +      scm_wrong_type_arg_msg (FUNC_NAME, 0, layout, "'u' field");
>
> This looks inefficient.  How about using 'scm_i_symbol_chars'?

OK.

Cheers,

Andy
-- 
http://wingolog.org/



  parent reply	other threads:[~2014-04-27 17:51 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-27 13:17 RFC: Foreign objects facility Andy Wingo
2014-04-27 16:00 ` Mark H Weaver
2014-04-27 16:46   ` Stefan Israelsson Tampe
2014-04-28 17:47     ` Andy Wingo
2014-04-27 17:51   ` Andy Wingo [this message]
2014-05-03  4:57     ` Doug Evans
2014-04-28 16:08   ` Andy Wingo
2014-04-29 16:08     ` Doug Evans
2014-04-29 18:27       ` Andy Wingo
2014-04-28  8:24 ` Ludovic Courtès
2014-04-28 18:05   ` Andy Wingo
2014-04-29 15:56 ` Doug Evans
2014-04-29 18:25   ` Andy Wingo
2014-05-03  5:45     ` Doug Evans

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

  List information: https://www.gnu.org/software/guile/

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

  git send-email \
    --in-reply-to=87oazm3bki.fsf@pobox.com \
    --to=wingo@pobox.com \
    --cc=guile-devel@gnu.org \
    --cc=mhw@netris.org \
    /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).