Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-1-18-g904a78f

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-1-18-g904a78f

[Ludovic Courtès]

"Michael Gran" <spk121@yahoo.com> writes:

> commit 904a78f11d2d11a58d5df365a44c4fbbd4c96df3
> Author: Michael Gran <spk121@yahoo.com>
> Date:   Wed Jul 29 06:38:32 2009 -0700
>
>     Add 32-bit characters
>     
>     This adds the 32-bit standalone characters.  Strings are still
>     8-bit.  Characters larger than 8-bit can only be entered or
>     displayed in octal format at this point.  At this point, the
>     terminal's display encoding is expected to be Latin-1.

It looks like Unicode is approaching, good news!  :-)

My remark about user-visibility was actually regarding this commit, not
the previous one.

> +#ifndef SCM_WCHAR_DEFINED
> +typedef scm_t_int32 scm_t_wchar;
> +#define SCM_WCHAR_DEFINED
> +#endif

Why is this #ifdef hack needed?

> +#define SCM_CHAR(x) ((scm_t_wchar)SCM_ITAG8_DATA(x))

Please, use GCS style.

> +#define SCM_MAKE_CHAR(x) ({scm_t_int32 _x = (x);                        \
> +      _x < 0                                                            \
> +        ? SCM_MAKE_ITAG8((scm_t_bits)(unsigned char)_x, scm_tc8_char)   \
> +        : SCM_MAKE_ITAG8((scm_t_bits)_x, scm_tc8_char);})

This macro uses a GCC extension, which is not acceptable for Guile.  Can
you please rewrite it in standard C?  (The only risk is multiple
expansion of X, but that's OK.)

Does X < 0 mean ASCII?  And why is it truncated to 8 bits?  A comment
just above indicating the encoding trick would be handy IMO.

(And style isn't OK.)

> +#define SCM_CODEPOINT_MAX (0x10ffff)
> +#define SCM_IS_UNICODE_CHAR(c)                                          \
> +  ((scm_t_wchar)(c)<=0xd7ff ||                                          \
> +   ((scm_t_wchar)(c)>=0xe000 && (scm_t_wchar)(c)<=SCM_CODEPOINT_MAX))

Style.

> +                  if (i<256)
> +                    {
> +                      /* Character is graphic.  Print it.  */
> +                      scm_putc (i, port);
> +                    }

Style (extraneous braces).

> +VM_DEFINE_INSTRUCTION (42, make_char32, "make-char32", 4, 0, 1)
> +{
> +  scm_t_wchar v = 0;
> +  v += FETCH ();
> +  v <<= 8; v += FETCH ();
> +  v <<= 8; v += FETCH ();
> +  v <<= 8; v += FETCH ();
> +  PUSH (SCM_MAKE_CHAR (v));
> +  NEXT;
> +}

The doc will need to be augmented.

> +	((char? x)
> +         (cond ((<= (char->integer x) #xff)
> +                `(make-char8 ,(char->integer x)))
> +               (else
> +                `(make-char32 ,(char->integer x)))))

Sounds cool!  :-)

Thanks,
Ludo'.

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-1-18-g904a78f

[Mike Gran]

On Fri, 2009-07-31 at 01:21 +0200, Ludovic Courtès wrote:
> "Michael Gran" <spk121@yahoo.com> writes:
> My remark about user-visibility was actually regarding this commit, not
> the previous one.
> 
> > +#ifndef SCM_WCHAR_DEFINED
> > +typedef scm_t_int32 scm_t_wchar;
> > +#define SCM_WCHAR_DEFINED
> > +#endif
> 
> Why is this #ifdef hack needed?
> 

It was to work around a problem, which, apparently, I can no longer
reproduce.  So, it isn't needed.

> > +#define SCM_MAKE_CHAR(x) ({scm_t_int32 _x = (x);                        \
> > +      _x < 0                                                            \
> > +        ? SCM_MAKE_ITAG8((scm_t_bits)(unsigned char)_x, scm_tc8_char)   \
> > +        : SCM_MAKE_ITAG8((scm_t_bits)_x, scm_tc8_char);})
> 
> This macro uses a GCC extension, which is not acceptable for Guile.  Can
> you please rewrite it in standard C?  (The only risk is multiple
> expansion of X, but that's OK.)

OK.  There was one case of multiple expansion causing side effects, but,
I fixed that.

> Does X < 0 mean ASCII?  And why is it truncated to 8 bits?  A comment
> just above indicating the encoding trick would be handy IMO.

OK.  Wide chars are always positive, but, the upper 128 of signed 8-bit
C chars are negative, which is the reason for that logic.

>> +                  if (i<256)
>> +                    {
>> +                      /* Character is graphic.  Print it.  */
>> +                      scm_putc (i, port);
>> +                    }

> Style (extraneous braces).

Noted.  If that's the standard then so be it.   But, for this case, I
declare, in classic flamewar fashion, that the standard is nonsense.

Thanks,

Mike

Re: [Guile-commits] GNU Guile branch, master, updated. release_1-9-1-18-g904a78f

[Andy Wingo]

Hi Mike,

Another niggle:

On Sat 01 Aug 2009 19:58, Mike Gran <spk121@yahoo.com> writes:

> On Fri, 2009-07-31 at 01:21 +0200, Ludovic Courtès wrote:
>> "Michael Gran" <spk121@yahoo.com> writes:
>> > +#define SCM_MAKE_CHAR(x) ({scm_t_int32 _x = (x);                        \
>> > +      _x < 0                                                            \
>> > +        ? SCM_MAKE_ITAG8((scm_t_bits)(unsigned char)_x, scm_tc8_char)   \
>> > +        : SCM_MAKE_ITAG8((scm_t_bits)_x, scm_tc8_char);})
>> 
>> This macro uses a GCC extension, which is not acceptable for Guile.  Can
>> you please rewrite it in standard C?  (The only risk is multiple
>> expansion of X, but that's OK.)
>
> OK.  There was one case of multiple expansion causing side effects, but,
> I fixed that.

Could we not just fix invokers of SCM_MAKE_CHAR() with negative values?
Are there instances of this outside Guile's source tree whose behavior
we need to preserve? That would avoid the multiple expansion problem
neatly, which the future would appreciate.

>> Does X < 0 mean ASCII?  And why is it truncated to 8 bits?  A comment
>> just above indicating the encoding trick would be handy IMO.
>
> OK.  Wide chars are always positive, but, the upper 128 of signed 8-bit
> C chars are negative, which is the reason for that logic.

I see. How irritating. Well, I guess that's fine, then -- barring a
requirement for SCM_LITERAL_CHAR or something.

>> Style (extraneous braces).
>
> Noted.  If that's the standard then so be it.   But, for this case, I
> declare, in classic flamewar fashion, that the standard is nonsense.

:)

It is better that we follow the standard, though.

Cheers,

Andy
-- 
http://wingolog.org/