Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.

From: Eli Zaretskii <[email-masked]>
Date: Sat, 02 Jun 2012 10:31:19 +0300

> Date: Fri, 01 Jun 2012 16:37:15 -0700
> From: Paul Eggert <[email-masked]>
> 
> Tags: patch
> 
> Here's a patch I'd like to install into the trunk soon, after testing
> it on a few more platforms.  It follows up on a recent discussion in
> emacs-devel.

Could you please explain this part:

> +	* w32heap.c (allocate_heap, init_heap):
> +	Pay no attention to USE_LISP_UNION_TYPE, as it's irrelevant here.

In what way is it "irrelevant here"?  Your changes don't remove
USE_LISP_UNION_TYPE, do they?  And using the union still limits the
address space, doesn't it?

Also, why did you change DECL_ALIGN to DCL_ALIGN? was something wrong
with the previous name?  The log entry doesn't mention this change,
and the rest of lisp.h that uses DECL_ALIGN was not modified.  Will
this even compile?

Re this:

> +	(union Lisp_Object): Don't worry about WORDS_BIGENDIAN; the
> +	code works fine either way, and efficiency is not a concern here.

why isn't efficiency a concern?

Re this:

> +	(LISP_MAKE_RVALUE, make_number) [USE_LISP_UNION_TYPE]:
> +	Use an inline function on all platforms, since this is simpler and
> +	'static inline' (via gnulib) is portable now.

I still see one instance of LISP_MAKE_RVALUE as a macro in lisp.h:

> @@ -404,6 +373,7 @@
>  
>  typedef EMACS_INT Lisp_Object;
>  #define LISP_MAKE_RVALUE(o) (0+(o))
> +#define LISP_INITIALLY_ZERO 0
>  #endif /* USE_LISP_UNION_TYPE */

Also, which parts of gnulib make 'static inline' portable?  If it
isn't in a part that is used by the MS-Windows port, the MSVC build is
still being screwed by these changes.

The following changes are not mentioned in the log entry:

>  # define XSET(var, vartype, ptr) \
> -  (eassert ((((uintptr_t) (ptr)) & ((1 << GCTYPEBITS) - 1)) == 0),	\
> -   (var).u.val = ((uintptr_t) (ptr)) >> GCTYPEBITS,			\
> -   (var).u.type = ((char) (vartype)))
> +  (eassert (((uintptr_t) (ptr) & ((1 << GCTYPEBITS) - 1)) == 0),	\
> +   (var).u.val = (uintptr_t) (ptr) >> GCTYPEBITS,			\
> +   (var).u.type = (vartype))
>  
>  /* Some versions of gcc seem to consider the bitfield width when issuing
>     the "cast to pointer from integer of different size" warning, so the
> @@ -563,7 +533,7 @@
>  # define XSETFASTINT(a, b) ((a).i = (b))
>  
>  # define XSET(var, vartype, ptr) \
> -   (((var).s.val = ((intptr_t) (ptr))), ((var).s.type = ((char) (vartype))))
> +   ((var).s.val = (intptr_t) (ptr), (var).s.type = (vartype))

What was the reason for removing this comment:

> --- src/frame.c	2012-06-01 03:41:03 +0000
> +++ src/frame.c	2012-06-01 23:34:07 +0000
> @@ -1152,10 +1152,6 @@
>    described for Fdelete_frame.  */
>  Lisp_Object
>  delete_frame (Lisp_Object frame, Lisp_Object force)
> -     /* If we use `register' here, gcc-4.0.2 on amd64 using
> -	-DUSE_LISP_UNION_TYPE complains further down that we're getting the
> -	address of `force'.  Go figure.  */
> -
>  {
>    struct frame *f;
>    struct frame *sf = SELECTED_FRAME ();

?  It is again not mentioned in the log entry.

Thanks.