Re: Old topic(s) again [was: Re: Proposal: immediate strings]

[Stefan Monnier <monnier@iro.umontreal.ca>]

> This feature is not thrown away, so I would like to get more comments
> around it.

I'm still not sure it's worth the trouble (although I've been running
here with a similar patch for the last several years).

A few comments below.

> +dnl Determine the basic type of ptrdiff_t.
> +AC_CHECK_SIZEOF([int])
> +AC_CHECK_SIZEOF([long])
> +AC_CHECK_SIZEOF([long long])
> +AC_CHECK_SIZEOF([ptrdiff_t])
> +AC_MSG_CHECKING([for the basic type of ptrdiff_t])
> +if test $ac_cv_sizeof_int = $ac_cv_sizeof_ptrdiff_t; then
> +   emacs_cv_type_ptrdiff_t="int"
> +elif test $ac_cv_sizeof_long = $ac_cv_sizeof_ptrdiff_t; then
> +   emacs_cv_type_ptrdiff_t="long"
> +elif test $ac_cv_sizeof_long_long = $ac_cv_sizeof_ptrdiff_t; then
> +   emacs_cv_type_ptrdiff_t="long long"
> +else
> +   emacs_cv_type_ptrdiff_t="unknown"
> +fi
> +AC_MSG_RESULT([$emacs_cv_type_ptrdiff_t])
> +if test $emacs_cv_type_ptrdiff_t != "unknown"; then
> +   AC_DEFINE_UNQUOTED([TYPE_PTRDIFF_T], [$emacs_cv_type_ptrdiff_t],
> +                      [Define to the basic type of ptrdiff_t])
> +else
> +   AC_MSG_ERROR([Unable to find the basic type of ptrdiff_t.])
> +fi

I really would much rather avoid such things.  Why do we need it?

> -	      && ((struct Lisp_String *) p)->data != NULL);
> +	      && live_string_data_p ((struct Lisp_String *) p));

Why can't we keep using the same simple test (combined with testing
immbit, obviously)?

> -/* Convenience macros for dealing with Lisp strings.  */

Why do you move this block of definitions?

> +{
> +  /* Text properties in this string.  Should be the first
> +     member since NEXT_FREE_LISP_STRING from alloc.c uses it.  */
> +  INTERVAL intervals;

I don't understand this comment (you haven't changed
NEXT_FREE_LISP_STRING AFAICT, but the field is not first in the current
code).

> +  union {
> +    /* GC mark bit and subtype bit are in IMM just by convention - when
> +       IMMBIT is 0, the DAT field is used except it's UNUSED field.  */

This comment seems out of date: gcmarkbit seems to be in both `imm' and
`dat', and I don't see any "subtype" bit, especially not one that's only
in `imm'.  Finally, I can't find any `unused' field either.

> +    struct {
> +      unsigned immbit : 1;
> +      unsigned size : BITS_PER_CHAR - 1;
> +      unsigned char data[STRING_IMM_SIZE];
> +      unsigned size_byte : BITS_PER_CHAR - 1;
> +      unsigned gcmarkbit : 1;
> +    } imm;
> +
> +    struct {
> +      unsigned immbit : 1;
> +      unsigned TYPE_PTRDIFF_T size : BITS_PER_PTRDIFF_T - 1;
> +      unsigned char *data;
> +      unsigned TYPE_PTRDIFF_T size_byte : BITS_PER_PTRDIFF_T - 1;
> +      unsigned gcmarkbit : 1;
> +    } dat;
> +  } u;
> +};

The comment should explain here the use of STRING_UNIBYTE_IMM/DAT_MARK.

One more thing: while I'm quite willing to believe that this placement
of gcmarkbit at the end of both structs (placed after differently-sized
bit fields) works fine in practice, I'd be interested to know to what
extent the C language guarantees that it will work.


        Stefan