[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#8546: fix for Emacs pseudovector incompatibility with GCC 4.6.0
From: |
Paul Eggert |
Subject: |
bug#8546: fix for Emacs pseudovector incompatibility with GCC 4.6.0 |
Date: |
Mon, 25 Apr 2011 16:12:51 -0700 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.15) Gecko/20110307 Fedora/3.1.9-0.39.b3pre.fc14 Thunderbird/3.1.9 |
On 04/25/11 07:05, Stefan Monnier wrote:
> +struct vector_header
>
> I'd call it vectorlike_header.
OK, I'll do that.
> +#define XVECTOR_SIZE(a) (XVECTOR (a)->header.size + 0)
>
> why not use ASIZE?
No good reason. Thanks, I'll do that too.
> + {
> + EMACS_UINT size;
> + union {
> + struct buffer *buffer;
> + struct Lisp_Vector *vector;
> + } next;
> + };
>
> Why do you need to handle buffers specially here? That sounds wrong.
Purely as a convenience. The code always uses the 'next' pointer as a
struct buffer * (in alloc.c, buffer.c, data.c), or as a struct
Lisp_Vector * (in alloc.c, fns.c). As an alternative, we could
replace the above with
{
EMACS_UINT size;
struct vectorlike_header *next;
};
and then replace uses like this:
for (b = all_buffers; b && b != po; b = b->header.next.buffer)
with uses like this:
for (b = all_buffers; b && b != po; b = (struct buffer *) b->header.next)
I thought that the union made the code clearer and I know you
normally dislike casts, but if you prefer the style with casts it'd be
easy to do that too.
> +#define XVECTOR_HEADER_SIZE(a) (((struct vector_header *) XPNTR
> (a))->size + 0)
>
> why do we need this variant with this weird set of type casts?
I'll remove it. It is used in only one place, in
XSETTYPED_PSEUDOVECTOR, where the idea is a key part of the
antialiasing fix. But there's no need to break it out as a separate
macro, so I'll fold it into XSETTYPED_PSEUDOVECTOR.
> + * lread.c (defsubr): Use XSETTYPED_PVECTYPE, since Lisp_Subr is a
> + special case.
>
> Why does Lisp_Subr need to be a special case (IIUC this applies to
> XSETTYPED_PSEUDOVECTOR and TYPED_PSEUDOVECTORP as well).
struct Lisp_Subr has a "size" field but no "next" field. I didn't
change its layout to contain a struct vectorlike_header field, as that
would have added an extra word that isn't needed. It would be safer,
from a C standards point of view, to spend the extra word and make
struct Lisp_Subr be like the other vector-like objects, but in
practice I doubt whether any practical optimizing compiler would break
that part of the code; so I kept it as a special case.
If you prefer the simpler and cleaner (but less space-efficient)
variant, where struct Lisp_Subr has a "next" field like all the other
vector-like data structures, that would be easy to do.
Attached is a revised patch with the above comments taken into account.
pseudovec-2.txt
Description: Text document