bug#4643: two different definitions for catchlist in eval.c and alloc.c

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Adrian Robert]

Just out of curiosity, is this new #include definitely not needed in  
the ns*.m files?  I don't understand the actual issue, but it seems  
like every other source file has been touched.  While gcc-4.5 will  
probably never be used on OS X (moving to clang), it might be in  
GNUstep compilations.

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Adrian Robert <adrian.b.robert@gmail.com> writes:

  > Just out of curiosity, is this new #include definitely not needed in
  > the ns*.m files?  I don't understand the actual issue, but it seems

I think I grepped for *.[ch], so *.m where accidentally left out.
Fixed now.

  > like every other source file has been touched.  While gcc-4.5 will
  > probably never be used on OS X (moving to clang), it might be in
  > GNUstep compilations.

This would affect any compiler that does IPA.

bug#4643: two different definitions for handlerlist and catchlist in eval.c and alloc.c

[Dan Nicolaescu]

struct handlerlist and struct catchlist have different definitions in
eval.c and alloc.c.
This generates and error when using doing link time optimizations with
gcc. 

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Here's a patch that fixes this problem by moving the definition in a
separate file included in both places.

OK to check in?

Index: src/Makefile.in
===================================================================
RCS file: /cvsroot/emacs/emacs/src/Makefile.in,v
retrieving revision 1.450
diff -u -3 -p -r1.450 Makefile.in
--- src/Makefile.in	26 Sep 2009 19:49:17 -0000	1.450
+++ src/Makefile.in	7 Oct 2009 08:26:06 -0000
@@ -1231,14 +1231,14 @@ xsmfns.o: xsmfns.c $(config_h) systime.h
 /* The files of Lisp proper */
 
 alloc.o: alloc.c process.h frame.h window.h buffer.h  puresize.h syssignal.h keyboard.h \
- blockinput.h atimer.h systime.h character.h dispextern.h $(config_h) \
+ blockinput.h atimer.h systime.h character.h dispextern.h catchtag.h $(config_h) \
  $(INTERVALS_H)
 bytecode.o: bytecode.c buffer.h syntax.h character.h window.h dispextern.h \
   frame.h xterm.h $(config_h)
 data.o: data.c buffer.h puresize.h character.h syssignal.h keyboard.h frame.h \
    termhooks.h $(config_h)
 eval.o: eval.c commands.h keyboard.h blockinput.h atimer.h systime.h \
-  dispextern.h $(config_h)
+  dispextern.h catchtag.h $(config_h)
 floatfns.o: floatfns.c syssignal.h $(config_h)
 fns.o: fns.c commands.h $(config_h) frame.h buffer.h character.h keyboard.h \
  keymap.h frame.h window.h dispextern.h $(INTERVALS_H) coding.h md5.h \
Index: src/alloc.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/alloc.c,v
retrieving revision 1.449
diff -u -3 -p -r1.449 alloc.c
--- src/alloc.c	25 Aug 2009 06:03:09 -0000	1.449
+++ src/alloc.c	7 Oct 2009 08:26:08 -0000
@@ -56,6 +56,7 @@ along with GNU Emacs.  If not, see <http
 #include "syssignal.h"
 #include "termhooks.h"		/* For struct terminal.  */
 #include <setjmp.h>
+#include "catchtag.h"
 
 /* GC_MALLOC_CHECK defined means perform validity checks of malloc'd
    memory.  Can do this only if using gmalloc.c.  */
@@ -4939,13 +4940,6 @@ staticpro (varaddress)
     abort ();
 }
 
-struct catchtag
-{
-    Lisp_Object tag;
-    Lisp_Object val;
-    struct catchtag *next;
-};
-
 \f
 /***********************************************************************
 			  Protection from GC
Index: src/eval.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/eval.c,v
retrieving revision 1.315
diff -u -3 -p -r1.315 eval.c
--- src/eval.c	1 Oct 2009 17:47:44 -0000	1.315
+++ src/eval.c	7 Oct 2009 08:26:08 -0000
@@ -26,6 +26,7 @@ along with GNU Emacs.  If not, see <http
 #include "keyboard.h"
 #include "dispextern.h"
 #include <setjmp.h>
+#include "catchtag.h"
 
 #if HAVE_X_WINDOWS
 #include "xterm.h"
@@ -49,41 +50,6 @@ struct backtrace
 
 struct backtrace *backtrace_list;
 
-/* This structure helps implement the `catch' and `throw' control
-   structure.  A struct catchtag contains all the information needed
-   to restore the state of the interpreter after a non-local jump.
-
-   Handlers for error conditions (represented by `struct handler'
-   structures) just point to a catch tag to do the cleanup required
-   for their jumps.
-
-   catchtag structures are chained together in the C calling stack;
-   the `next' member points to the next outer catchtag.
-
-   A call like (throw TAG VAL) searches for a catchtag whose `tag'
-   member is TAG, and then unbinds to it.  The `val' member is used to
-   hold VAL while the stack is unwound; `val' is returned as the value
-   of the catch form.
-
-   All the other members are concerned with restoring the interpreter
-   state.  */
-
-struct catchtag
-{
-  Lisp_Object tag;
-  Lisp_Object val;
-  struct catchtag *next;
-  struct gcpro *gcpro;
-  jmp_buf jmp;
-  struct backtrace *backlist;
-  struct handler *handlerlist;
-  int lisp_eval_depth;
-  int pdlcount;
-  int poll_suppress_count;
-  int interrupt_input_blocked;
-  struct byte_stack *byte_stack;
-};
-
 struct catchtag *catchlist;
 
 #ifdef DEBUG_GCPRO
--- /dev/null	2009-08-21 19:23:04.524086726 -0700
+++ src/catchtag.h	2009-10-07 01:11:39.784950000 -0700
@@ -0,0 +1,57 @@
+/* Structure for implementing the `catch' and `throw' control structure.
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+This file is part of GNU Emacs.
+
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef CATCHTAG_H
+#define CATCHTAG_H
+
+/* This structure helps implement the `catch' and `throw' control
+   structure.  A struct catchtag contains all the information needed
+   to restore the state of the interpreter after a non-local jump.
+
+   Handlers for error conditions (represented by `struct handler'
+   structures) just point to a catch tag to do the cleanup required
+   for their jumps.
+
+   catchtag structures are chained together in the C calling stack;
+   the `next' member points to the next outer catchtag.
+
+   A call like (throw TAG VAL) searches for a catchtag whose `tag'
+   member is TAG, and then unbinds to it.  The `val' member is used to
+   hold VAL while the stack is unwound; `val' is returned as the value
+   of the catch form.
+
+   All the other members are concerned with restoring the interpreter
+   state.  */
+
+struct catchtag
+{
+  Lisp_Object tag;
+  Lisp_Object val;
+  struct catchtag *next;
+  struct gcpro *gcpro;
+  jmp_buf jmp;
+  struct backtrace *backlist;
+  struct handler *handlerlist;
+  int lisp_eval_depth;
+  int pdlcount;
+  int poll_suppress_count;
+  int interrupt_input_blocked;
+  struct byte_stack *byte_stack;
+};
+
+#endif /* CATCHTAG_H */

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Dan Nicolaescu <dann@ics.uci.edu> writes:

  > Here's a patch that fixes this problem by moving the definition in a
  > separate file included in both places.
  > 
  > OK to check in?

Ping.

Without a change like this emacs cannot be compiled using gcc-4.5 -flto 


  > Index: src/Makefile.in
  > ===================================================================
  > RCS file: /cvsroot/emacs/emacs/src/Makefile.in,v
  > retrieving revision 1.450
  > diff -u -3 -p -r1.450 Makefile.in
  > --- src/Makefile.in	26 Sep 2009 19:49:17 -0000	1.450
  > +++ src/Makefile.in	7 Oct 2009 08:26:06 -0000
  > @@ -1231,14 +1231,14 @@ xsmfns.o: xsmfns.c $(config_h) systime.h
  >  /* The files of Lisp proper */
  >  
  >  alloc.o: alloc.c process.h frame.h window.h buffer.h  puresize.h syssignal.h keyboard.h \
  > - blockinput.h atimer.h systime.h character.h dispextern.h $(config_h) \
  > + blockinput.h atimer.h systime.h character.h dispextern.h catchtag.h $(config_h) \
  >   $(INTERVALS_H)
  >  bytecode.o: bytecode.c buffer.h syntax.h character.h window.h dispextern.h \
  >    frame.h xterm.h $(config_h)
  >  data.o: data.c buffer.h puresize.h character.h syssignal.h keyboard.h frame.h \
  >     termhooks.h $(config_h)
  >  eval.o: eval.c commands.h keyboard.h blockinput.h atimer.h systime.h \
  > -  dispextern.h $(config_h)
  > +  dispextern.h catchtag.h $(config_h)
  >  floatfns.o: floatfns.c syssignal.h $(config_h)
  >  fns.o: fns.c commands.h $(config_h) frame.h buffer.h character.h keyboard.h \
  >   keymap.h frame.h window.h dispextern.h $(INTERVALS_H) coding.h md5.h \
  > Index: src/alloc.c
  > ===================================================================
  > RCS file: /cvsroot/emacs/emacs/src/alloc.c,v
  > retrieving revision 1.449
  > diff -u -3 -p -r1.449 alloc.c
  > --- src/alloc.c	25 Aug 2009 06:03:09 -0000	1.449
  > +++ src/alloc.c	7 Oct 2009 08:26:08 -0000
  > @@ -56,6 +56,7 @@ along with GNU Emacs.  If not, see <http
  >  #include "syssignal.h"
  >  #include "termhooks.h"		/* For struct terminal.  */
  >  #include <setjmp.h>
  > +#include "catchtag.h"
  >  
  >  /* GC_MALLOC_CHECK defined means perform validity checks of malloc'd
  >     memory.  Can do this only if using gmalloc.c.  */
  > @@ -4939,13 +4940,6 @@ staticpro (varaddress)
  >      abort ();
  >  }
  >  
  > -struct catchtag
  > -{
  > -    Lisp_Object tag;
  > -    Lisp_Object val;
  > -    struct catchtag *next;
  > -};
  > -
  >  \f
  >  /***********************************************************************
  >  			  Protection from GC
  > Index: src/eval.c
  > ===================================================================
  > RCS file: /cvsroot/emacs/emacs/src/eval.c,v
  > retrieving revision 1.315
  > diff -u -3 -p -r1.315 eval.c
  > --- src/eval.c	1 Oct 2009 17:47:44 -0000	1.315
  > +++ src/eval.c	7 Oct 2009 08:26:08 -0000
  > @@ -26,6 +26,7 @@ along with GNU Emacs.  If not, see <http
  >  #include "keyboard.h"
  >  #include "dispextern.h"
  >  #include <setjmp.h>
  > +#include "catchtag.h"
  >  
  >  #if HAVE_X_WINDOWS
  >  #include "xterm.h"
  > @@ -49,41 +50,6 @@ struct backtrace
  >  
  >  struct backtrace *backtrace_list;
  >  
  > -/* This structure helps implement the `catch' and `throw' control
  > -   structure.  A struct catchtag contains all the information needed
  > -   to restore the state of the interpreter after a non-local jump.
  > -
  > -   Handlers for error conditions (represented by `struct handler'
  > -   structures) just point to a catch tag to do the cleanup required
  > -   for their jumps.
  > -
  > -   catchtag structures are chained together in the C calling stack;
  > -   the `next' member points to the next outer catchtag.
  > -
  > -   A call like (throw TAG VAL) searches for a catchtag whose `tag'
  > -   member is TAG, and then unbinds to it.  The `val' member is used to
  > -   hold VAL while the stack is unwound; `val' is returned as the value
  > -   of the catch form.
  > -
  > -   All the other members are concerned with restoring the interpreter
  > -   state.  */
  > -
  > -struct catchtag
  > -{
  > -  Lisp_Object tag;
  > -  Lisp_Object val;
  > -  struct catchtag *next;
  > -  struct gcpro *gcpro;
  > -  jmp_buf jmp;
  > -  struct backtrace *backlist;
  > -  struct handler *handlerlist;
  > -  int lisp_eval_depth;
  > -  int pdlcount;
  > -  int poll_suppress_count;
  > -  int interrupt_input_blocked;
  > -  struct byte_stack *byte_stack;
  > -};
  > -
  >  struct catchtag *catchlist;
  >  
  >  #ifdef DEBUG_GCPRO
  > --- /dev/null	2009-08-21 19:23:04.524086726 -0700
  > +++ src/catchtag.h	2009-10-07 01:11:39.784950000 -0700
  > @@ -0,0 +1,57 @@
  > +/* Structure for implementing the `catch' and `throw' control structure.
  > +   Copyright (C) 2009 Free Software Foundation, Inc.
  > +
  > +This file is part of GNU Emacs.
  > +
  > +GNU Emacs is free software: you can redistribute it and/or modify
  > +it under the terms of the GNU General Public License as published by
  > +the Free Software Foundation, either version 3 of the License, or
  > +(at your option) any later version.
  > +
  > +GNU Emacs is distributed in the hope that it will be useful,
  > +but WITHOUT ANY WARRANTY; without even the implied warranty of
  > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  > +GNU General Public License for more details.
  > +
  > +You should have received a copy of the GNU General Public License
  > +along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
  > +
  > +#ifndef CATCHTAG_H
  > +#define CATCHTAG_H
  > +
  > +/* This structure helps implement the `catch' and `throw' control
  > +   structure.  A struct catchtag contains all the information needed
  > +   to restore the state of the interpreter after a non-local jump.
  > +
  > +   Handlers for error conditions (represented by `struct handler'
  > +   structures) just point to a catch tag to do the cleanup required
  > +   for their jumps.
  > +
  > +   catchtag structures are chained together in the C calling stack;
  > +   the `next' member points to the next outer catchtag.
  > +
  > +   A call like (throw TAG VAL) searches for a catchtag whose `tag'
  > +   member is TAG, and then unbinds to it.  The `val' member is used to
  > +   hold VAL while the stack is unwound; `val' is returned as the value
  > +   of the catch form.
  > +
  > +   All the other members are concerned with restoring the interpreter
  > +   state.  */
  > +
  > +struct catchtag
  > +{
  > +  Lisp_Object tag;
  > +  Lisp_Object val;
  > +  struct catchtag *next;
  > +  struct gcpro *gcpro;
  > +  jmp_buf jmp;
  > +  struct backtrace *backlist;
  > +  struct handler *handlerlist;
  > +  int lisp_eval_depth;
  > +  int pdlcount;
  > +  int poll_suppress_count;
  > +  int interrupt_input_blocked;
  > +  struct byte_stack *byte_stack;
  > +};
  > +
  > +#endif /* CATCHTAG_H */

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Stefan Monnier]

>> Here's a patch that fixes this problem by moving the definition in a
>> separate file included in both places.
>> 
>> OK to check in?

> Ping.

I agree on the idea, but I don't want a new file just for that
declaration, so please move it to some existing file instead.


        Stefan

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > >> Here's a patch that fixes this problem by moving the definition in a
  > >> separate file included in both places.
  > >> 
  > >> OK to check in?
  > 
  > > Ping.
  > 
  > I agree on the idea, but I don't want a new file just for that
  > declaration, so please move it to some existing file instead.

The catchtag structure definition needs a #include <setjmp.h> to precede it.
We don't include setjmp.h in many places, so it will need to be
added... 
I don't see any existing header that would be a good match (other than lisp.h).
So please suggest where to add it.

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Stefan Monnier]

>> >> Here's a patch that fixes this problem by moving the definition in a
>> >> separate file included in both places.
>> >> 
>> >> OK to check in?
>> 
>> > Ping.
>> 
>> I agree on the idea, but I don't want a new file just for that
>> declaration, so please move it to some existing file instead.

> The catchtag structure definition needs a #include <setjmp.h> to
> precede it.  We don't include setjmp.h in many places, so it will need
> to be added...  I don't see any existing header that would be a good
> match (other than lisp.h).  So please suggest where to add it.

Huh.... lisp.h?


        Stefan

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > >> >> Here's a patch that fixes this problem by moving the definition in a
  > >> >> separate file included in both places.
  > >> >> 
  > >> >> OK to check in?
  > >> 
  > >> > Ping.
  > >> 
  > >> I agree on the idea, but I don't want a new file just for that
  > >> declaration, so please move it to some existing file instead.
  > 
  > > The catchtag structure definition needs a #include <setjmp.h> to
  > > precede it.  We don't include setjmp.h in many places, so it will need
  > > to be added...  I don't see any existing header that would be a good
  > > match (other than lisp.h).  So please suggest where to add it.
  > 
  > Huh.... lisp.h?

If you don't mind setjmp.h being included everywhere, fine with me.

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Stefan Monnier]

> If you don't mind setjmp.h being included everywhere, fine with me.

Indeed, I don't mind.


        Stefan