[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Old topic(s) again [was: Re: Proposal: immediate strings]
From: |
Stefan Monnier |
Subject: |
Re: Old topic(s) again [was: Re: Proposal: immediate strings] |
Date: |
Wed, 04 Jul 2012 09:08:30 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1.50 (gnu/linux) |
> 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