GNU bug report logs - #25128
Reorder lisp.h to declare types before using them

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25128 in the body.
You can then email your comments to 25128 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Emacs bug reports and feature requests <bug-gnu-emacs <at> gnu.org>
Subject: Reorder lisp.h to declare types before using them
Date: Tue, 6 Dec 2016 23:19:19 -0800

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

Tags: patch

Attached is a patch to reorder src/lisp.h so that more types are declared before 
use. The main motivation is to eventually add support for 
-fcheck-pointer-bounds; future changes will need access to type sizes (e.g., 
XCONS might need sizeof (struct Lisp_Cons)), which should be easy after this 
change. Also, the change shortens lisp.h a bit and (if you ask me) makes it 
easier to read, since the basic functions for a type are now declared next to 
the type.

I would like to install this soon, and am posting this as a bug report to give 
Eli and others a heads-up, in case some MS-Windows module now needs to include 
process.h. This may be needed because XPROCESS is now defined in process.h 
instead of lisp.h; similarly for buffer.h, window.h, termhooks.h.

[0001-Reorder-lisp.h-to-declare-types-before-using-them.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 25128 <at> debbugs.gnu.org
Subject: Re: bug#25128: Reorder lisp.h to declare types before using them
Date: Wed, 07 Dec 2016 18:09:15 +0200

> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Tue, 6 Dec 2016 23:19:19 -0800
> 
> Attached is a patch to reorder src/lisp.h so that more types are declared before 
> use. The main motivation is to eventually add support for 
> -fcheck-pointer-bounds; future changes will need access to type sizes (e.g., 
> XCONS might need sizeof (struct Lisp_Cons)), which should be easy after this 
> change. Also, the change shortens lisp.h a bit and (if you ask me) makes it 
> easier to read, since the basic functions for a type are now declared next to 
> the type.
> 
> I would like to install this soon, and am posting this as a bug report to give 
> Eli and others a heads-up, in case some MS-Windows module now needs to include 
> process.h. This may be needed because XPROCESS is now defined in process.h 
> instead of lisp.h; similarly for buffer.h, window.h, termhooks.h.

Thanks.  I'd appreciate if someone could beat me to it, as I'm busy
merging and testing the concurrency branch, hopefully soon to land on
master.

In any case, could you please wait with pushing this for a few days
until concurrency actually lands?  I'm in the last stages of testing
the merge, and it would minimize the impact of the final merging.  The
changes in lisp.h due to concurrency are very few, FWIW, so I don't
believe it will require you to make changes in your patch due to the
merge.

TIA

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 25128 <at> debbugs.gnu.org
Subject: Re: bug#25128: Reorder lisp.h to declare types before using them
Date: Wed, 7 Dec 2016 08:30:49 -0800

Eli Zaretskii wrote:
> could you please wait with pushing this for a few days
> until concurrency actually lands?

That shouldn't be a problem. It should give me more time to polish the 
-fcheck-pointer-bounds stuff.

Message #16 received at 25128-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 25128-done <at> debbugs.gnu.org
Subject: Re: bug#25128: Reorder lisp.h to declare types before using them
Date: Sun, 25 Dec 2016 09:38:14 -0800

I installed this patch, after fixing it up to work with the concurrency branch.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 25128 <at> debbugs.gnu.org
Subject: Re: bug#25128: Reorder lisp.h to declare types before using them
Date: Sun, 25 Dec 2016 19:59:38 +0200

> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Sun, 25 Dec 2016 09:38:14 -0800
> 
> I installed this patch, after fixing it up to work with the concurrency branch.

Thanks.

However, it breaks the MS-Windows 32-bit build --with-wide-int:

  In file included from scroll.c:25:0:
  lisp.h:1001:1: error: redefinition of 'XUNTAG'
   XUNTAG (Lisp_Object a, int type)
   ^
  In file included from scroll.c:25:0:
  lisp.h:630:2: note: previous definition of 'XUNTAG' was here
   (XUNTAG) (Lisp_Object a, int type)
    ^

and similarly in all the other files in src.

In case this matters, the compiler switches are

  CFLAGS = -O0 -gdwarf-4 -g3

Let me know if I can help by providing further information.

(A 64-bit GNU/Linux build succeeds, not surprisingly.)

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 25128 <at> debbugs.gnu.org
Subject: Re: bug#25128: Reorder lisp.h to declare types before using them
Date: Sun, 25 Dec 2016 10:15:27 -0800

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

Eli Zaretskii wrote:
> However, it breaks the MS-Windows 32-bit build --with-wide-int:

Sorry about that. Should be fixed now, with the attached:

[0001-Fix-typo-in-lisp.h-reordering-patch.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 25128 <at> debbugs.gnu.org
Subject: Re: bug#25128: Reorder lisp.h to declare types before using them
Date: Sun, 25 Dec 2016 20:19:49 +0200

> Cc: 25128 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Sun, 25 Dec 2016 10:15:27 -0800
> 
> Eli Zaretskii wrote:
> > However, it breaks the MS-Windows 32-bit build --with-wide-int:
> 
> Sorry about that. Should be fixed now, with the attached:

Thanks, builds cleanly now.

Previous Next

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