[Top][All Lists]

[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: 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

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.

Attachment: pseudovec-2.txt
Description: Text document

reply via email to

[Prev in Thread] Current Thread [Next in Thread]