GNU bug report logs - #8546
fix for Emacs pseudovector incompatibility with GCC 4.6.0

Previous Next

Full log

Message #17 received at 8546 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 8546 <at> debbugs.gnu.org
Subject: Re: bug#8546: fix for Emacs pseudovector incompatibility with GCC
	4.6.0
Date: Mon, 25 Apr 2011 16:12:51 -0700

[Message part 1 (text/plain, inline)]

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 (text/plain, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.