Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

[Juanma Barranquero]

On Mon, Jun 3, 2013 at 11:01 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> ------------------------------------------------------------
> revno: 112828
> committer: Stefan Monnier <monnier@iro.umontreal.ca>
> branch nick: trunk
> timestamp: Mon 2013-06-03 05:01:53 -0400
> message:
>   Merge the specpdl and backtrace stacks.  Make the structure of the
>   specpdl entries more obvious via a tagged union of structs.

>   * src/eval.c (backtrace_list): Remove.
>   (set_specpdl_symbol, set_specpdl_old_value): Remove.
>   (set_backtrace_args, set_backtrace_nargs)
>   (set_backtrace_debug_on_exit, backtrace_p, backtrace_top)
>   (backtrace_next): New functions.

On Windows:

eval.o: In function `Fsignal':
c:\Devel\emacs\repo\debug\src/eval.c:1409: undefined reference to
`backtrace_top'
c:\Devel\emacs\repo\debug\src/eval.c:1409: undefined reference to
`backtrace_next'
c:\Devel\emacs\repo\debug\src/eval.c:1410: undefined reference to `backtrace_p'
c:\Devel\emacs\repo\debug\src/eval.c:1411: undefined reference to
`backtrace_next'
c:\Devel\emacs\repo\debug\src/eval.c:1412: undefined reference to `backtrace_p'
eval.o: In function `Fbacktrace_debug':
c:\Devel\emacs\repo\debug\src/eval.c:3188: undefined reference to
`backtrace_top'
c:\Devel\emacs\repo\debug\src/eval.c:3194: undefined reference to
`backtrace_next'
c:\Devel\emacs\repo\debug\src/eval.c:3193: undefined reference to `backtrace_p'
c:\Devel\emacs\repo\debug\src/eval.c:3196: undefined reference to `backtrace_p'
eval.o: In function `Fbacktrace':
c:\Devel\emacs\repo\debug\src/eval.c:3207: undefined reference to
`backtrace_top'
c:\Devel\emacs\repo\debug\src/eval.c:3238: undefined reference to
`backtrace_next'
c:\Devel\emacs\repo\debug\src/eval.c:3214: undefined reference to `backtrace_p'
eval.o: In function `Fbacktrace_frame':
c:\Devel\emacs\repo\debug\src/eval.c:3257: undefined reference to
`backtrace_top'
c:\Devel\emacs\repo\debug\src/eval.c:3264: undefined reference to
`backtrace_next'
c:\Devel\emacs\repo\debug\src/eval.c:3263: undefined reference to `backtrace_p'
c:\Devel\emacs\repo\debug\src/eval.c:3266: undefined reference to `backtrace_p'
eval.o: In function `get_backtrace':
c:\Devel\emacs\repo\debug\src/eval.c:3314: undefined reference to
`backtrace_top'
c:\Devel\emacs\repo\debug\src/eval.c:3314: undefined reference to
`backtrace_next'
c:\Devel\emacs\repo\debug\src/eval.c:3320: undefined reference to `backtrace_p'
c:\Devel\emacs\repo\debug\src/eval.c:3323: undefined reference to
`backtrace_next'
eval.o: In function `backtrace_top_function':
c:\Devel\emacs\repo\debug\src/eval.c:3332: undefined reference to
`backtrace_top'
c:\Devel\emacs\repo\debug\src/eval.c:3333: undefined reference to `backtrace_p'
collect2.exe: error: ld returned 1 exit status
Makefile:655: recipe for target `temacs.exe' failed

Disappears if these functions are INLINE instead of LISP_INLINE.

   Juanma

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

[Juanma Barranquero]

On Mon, Jun 3, 2013 at 3:59 PM, Juanma Barranquero <lekktu@gmail.com> wrote:

> Disappears if these functions are INLINE instead of LISP_INLINE.

Sorry, I meant "if these functions are not (LISP_)?INLINE at all."

    Juanma

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

[Stefan Monnier]

> On Windows:

> eval.o: In function `Fsignal':
> c:\Devel\emacs\repo\debug\src/eval.c:1409: undefined reference to
> `backtrace_top'

Hmm... for some other reasons I just installed a patch that turns the
LISP_INLINE into EXTERN_INLINE.  With some luck, it may fix this
problem, but if it doesn't, we'll have to ask Paul, because I really
don't have enough of clue about what is the detailed semantics of those
INLINE annotations.

This said, I'm really surprised to see such problems, since
backtrace_(top|p|next) are defined in eval.c, before any use, so if code
from eval.c after the definition can't use those functions, it begs the
question: where can such <foo>INLINE functions be used?


        Stefan

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 03 Jun 2013 14:25:47 -0400
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> > On Windows:
> 
> > eval.o: In function `Fsignal':
> > c:\Devel\emacs\repo\debug\src/eval.c:1409: undefined reference to
> > `backtrace_top'
> 
> Hmm... for some other reasons I just installed a patch that turns the
> LISP_INLINE into EXTERN_INLINE.  With some luck, it may fix this
> problem

It seems to fix it, indeed.  Thanks.

> but if it doesn't, we'll have to ask Paul, because I really
> don't have enough of clue about what is the detailed semantics of those
> INLINE annotations.

Perhaps Paul could describe that nonetheless.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

[Paul Eggert]

On 06/03/2013 12:23 PM, Eli Zaretskii wrote:
> Perhaps Paul could describe that nonetheless.

Where's a good place to put the description?  Here's a quick first cut:

* C code ordinarily shouldn't use 'inline'.  Ordinarily, we let
  the compiler figure out whether to inline.  Compilers are
  pretty good about it these days.

* There's an exception for functions defined (not merely declared)
  in .h files.  Here, if we avoid inline we'll suffer from code bloat.
  Suppose aaa.h defines the function foo, and aaa.c, bbb.c and ccc.c
  all include aaa.h.  foo can't be an ordinary extern function, as it'd
  be defined three times and the definitions would clash.  foo could be
  a static function, but with separate compilation if foo's not inlined
  then foo's code will appear in the executable three times.  To avoid
  this code bloat, aaa.h can do this:

     INLINE_HEADER_BEGIN
     #ifndef AAA_INLINE
     # define AAA_INLINE INLINE
     #endif

     AAA_INLINE int foo (int i) { return i + 1; }

     INLINE_HEADER_END

  On C99 hosts this expands to C99-style 'extern inline' usage that avoids
  the code bloat.  (Pre-C99 hosts still suffer from code bloat, but that's
  acceptable; they're not mainline platforms and they'll die out eventually.)

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

[Eli Zaretskii]

> Date: Mon, 03 Jun 2013 22:55:18 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: Stefan Monnier <monnier@iro.umontreal.ca>, lekktu@gmail.com, 
>  emacs-devel@gnu.org
> 
> On 06/03/2013 12:23 PM, Eli Zaretskii wrote:
> > Perhaps Paul could describe that nonetheless.
> 
> Where's a good place to put the description?

My guess would be in lisp.h, right before we use this for the first
time.

>      INLINE_HEADER_BEGIN
>      #ifndef AAA_INLINE
>      # define AAA_INLINE INLINE
>      #endif
> 
>      AAA_INLINE int foo (int i) { return i + 1; }
> 
>      INLINE_HEADER_END
> 
>   On C99 hosts this expands to C99-style 'extern inline' usage that avoids
>   the code bloat.

Thanks.  That's what I thought these did, but then how to explain the
fact that revision 112828 produced unresolved externals for such
functions that were defined in the same file (eval.c)?

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

[Paul Eggert]

On 06/04/13 08:13, Eli Zaretskii wrote:

> My guess would be in lisp.h

Since the pattern is used in many .h files, I put the
documentation in src/conf_post.h, before the definition
of INLINE, EXTERN_INLINE, etc., as trunk bzr 113054
(patch at end of this message).

> how to explain the
> fact that revision 112828 produced unresolved externals for such
> functions that were defined in the same file (eval.c)?

112828 declared LISP_INLINE functions in eval.c.
That wasn't right for C99 platforms, because
in eval.c LISP_INLINE expands to plain 'inline',
and C99 requires that if any module defines a plain
inline function FOO (perhaps via a .h file), then
FOO must be defined as 'extern inline' in exactly one
other module.

Eventually this problem got fixed by declaring
these as ordinary functions, not as inline functions.

I'll add some more documentation to Gnulib about this.
The short version is that C code should ordinarily
not use 'inline'.  The only exception is for functions
defined in a .h file, which should be defined using the
pattern documented below.

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2013-06-18 14:54:48 +0000
+++ src/ChangeLog	2013-06-18 18:36:13 +0000
@@ -1,3 +1,7 @@
+2013-06-18  Paul Eggert  <eggert@cs.ucla.edu>
+
+	* conf_post.h: Add comments for INLINE, EXTERN_INLINE, etc.
+
 2013-06-18  Kenichi Handa  <handa@gnu.org>
 
 	* font.c (Ffont_spec): Signal an error for an invalid font name

=== modified file 'src/conf_post.h'
--- src/conf_post.h	2013-03-13 18:42:22 +0000
+++ src/conf_post.h	2013-06-18 18:36:13 +0000
@@ -207,6 +207,37 @@
 #undef noinline
 #endif
 
+/* Use Gnulib's extern-inline module for extern inline functions.
+   An include file foo.h should prepend FOO_INLINE to function
+   definitions, with the following overall pattern:
+
+      [#include any other .h files first.]
+      ...
+      INLINE_HEADER_BEGIN
+      #ifndef FOO_INLINE
+      # define FOO_INLINE INLINE
+      #endif
+      ...
+      FOO_INLINE int
+      incr (int i)
+      {
+        return i + 1;
+      }
+      ...
+      INLINE_HEADER_END
+
+   The corresponding foo.c file should do this:
+
+      #define FOO_INLINE EXTERN_INLINE
+
+   before including any .h file other than config.h.
+   Other .c files should not define FOO_INILNE.
+
+   C99 compilers compile functions like 'incr' as C99-style extern
+   inline functions.  Pre-C99 GCCs do something similar with
+   GNU-specific keywords.  Pre-C99 non-GCC compilers use static
+   functions, which bloats the code but is good enough.  */
+
 #define INLINE _GL_INLINE
 #define EXTERN_INLINE _GL_EXTERN_INLINE
 #define INLINE_HEADER_BEGIN _GL_INLINE_HEADER_BEGIN

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

[Eli Zaretskii]

> Date: Tue, 18 Jun 2013 11:50:54 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> > how to explain the
> > fact that revision 112828 produced unresolved externals for such
> > functions that were defined in the same file (eval.c)?
> 
> 112828 declared LISP_INLINE functions in eval.c.
> That wasn't right for C99 platforms, because
> in eval.c LISP_INLINE expands to plain 'inline',
> and C99 requires that if any module defines a plain
> inline function FOO (perhaps via a .h file), then
> FOO must be defined as 'extern inline' in exactly one
> other module.

So you are saying that the following is not valid C99, unless another
module defines 'foo' as 'extern inline'?

 inline int foo (void) { return 42; }
 int bar (void) { return foo (); }

That sounds strangely un-C-ish to me.

Thanks.

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r112828: Merge the specpdl and backtrace stacks. Make the structure of the

[Paul Eggert]

On 06/18/13 12:05, Eli Zaretskii wrote:
> So you are saying that the following is not valid C99, unless another
> module defines 'foo' as 'extern inline'?
> 
>  inline int foo (void) { return 42; }
>  int bar (void) { return foo (); }

Yes, that's right.  See ISO/IEC 9899:TC3 section 6.7.4
paragraphs 6 and 8.  C11 is the same, unfortunately.

Here's where you can get a copy of the C99 standard
(actually, the last freely readable draft, but that's good enough):

http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf