External inline functions

External inline functions

[Eli Zaretskii]

Several functions are declared `inline', but used from files other
than where they are defined.  Example: window_box_right.

Is this portable enough?  The Microsoft compiler errors out during
linking due to unresolved externals, but is this a problem specific to
that compiler?

Re: External inline functions

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> Is this portable enough?  The Microsoft compiler errors out during
> linking due to unresolved externals, but is this a problem specific to
> that compiler?

Apparently this is not a C compiler.  The inline specifier does not
influence the linkage.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: External inline functions

[Óscar Fuentes]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Is this portable enough?  The Microsoft compiler errors out during
>> linking due to unresolved externals, but is this a problem specific to
>> that compiler?
>
> Apparently this is not a C compiler.  The inline specifier does not
> influence the linkage.

There are several issues here. First, `inline' was introduced in the
1999 standard. There is no mention to it in the 1990 standard. So, if
the Microsoft compiler defaults to the 1990 standard it must be
implementing `inline' in a vendor-dependent way.

Second, my reading of the 1999 standard says that `inline' *does* affect
linkage:

6.7.4 p6:

... If all of the file scope declarations for a function in a
translation unit include the inline function specifier without extern,
then the definition in that translation unit is an inline definition. An
inline definition does not provide an external definition for the
function, and does not forbid an external definition in another
translation unit. ...

but OTOH xdisp.c includes dispextern.h, which declares the
function. Decorating the declaration of window_box_right with `extern'
should work.

Re: External inline functions

[Paul Eggert]

Yes, that's right: pre-C99 'inline' is a portability minefield,
and GNU Emacs isn't portable even to C99's tricky semantics
for extern inline (among other things, extern inline functions
cannot call static functions).

I installed the following to work around the problems in this area that
I found.  But as this issue keeps coming up, how about if we
simply stop using extern inline functions?  They do not buy us
significant performance, and they are a hassle that is causing
portability problems, so this should be an easy call.

====

Fix some portability problems with 'inline'.
* dispextern.h (window_box, window_box_height, window_text_bottom_y)
(window_box_width, window_box_left, window_box_left_offset)
(window_box_right, window_box_right_offset): Declare extern.
Otherwise, these inline functions do not conform to C99 and
are miscompiled by Microsoft compilers.  Reported by Eli Zaretskii in
<http://lists.gnu.org/archive/html/emacs-devel/2011-11/msg00084.html>.
* intervals.c (adjust_intervals_for_insertion)
(adjust_intervals_for_deletion): Now extern, because otherwise the
extern inline functions 'offset_intervals' couldn't refer to it.
(static_offset_intervals): Remove.
(offset_intervals): Rewrite using the old contents of
static_offset_intervals.  The old version didn't conform to C99
because an extern inline function contained a reference to an
identifier with static linkage.
=== modified file 'src/dispextern.h'
--- src/dispextern.h	2011-10-25 16:36:20 +0000
+++ src/dispextern.h	2011-11-06 21:03:15 +0000
@@ -3006,14 +3006,14 @@
 void set_vertical_scroll_bar (struct window *);
 #endif
 int try_window (Lisp_Object, struct text_pos, int);
-void window_box (struct window *, int, int *, int *, int *, int *);
-int window_box_height (struct window *);
-int window_text_bottom_y (struct window *);
-int window_box_width (struct window *, int);
-int window_box_left (struct window *, int);
-int window_box_left_offset (struct window *, int);
-int window_box_right (struct window *, int);
-int window_box_right_offset (struct window *, int);
+extern void window_box (struct window *, int, int *, int *, int *, int *);
+extern int window_box_height (struct window *);
+extern int window_text_bottom_y (struct window *);
+extern int window_box_width (struct window *, int);
+extern int window_box_left (struct window *, int);
+extern int window_box_left_offset (struct window *, int);
+extern int window_box_right (struct window *, int);
+extern int window_box_right_offset (struct window *, int);
 int estimate_mode_line_height (struct frame *, enum face_id);
 void pixel_to_glyph_coords (struct frame *, int, int, int *, int *,
                             NativeRectangle *, int);

=== modified file 'src/intervals.c'
--- src/intervals.c	2011-10-26 01:18:13 +0000
+++ src/intervals.c	2011-11-06 21:03:14 +0000
@@ -52,6 +52,11 @@

 #define TMEM(sym, set) (CONSP (set) ? ! NILP (Fmemq (sym, set)) : ! NILP (set))

+extern INTERVAL adjust_intervals_for_insertion (INTERVAL,
+						EMACS_INT, EMACS_INT);
+extern void adjust_intervals_for_deletion (struct buffer *,
+					   EMACS_INT, EMACS_INT);
+
 static Lisp_Object merge_properties_sticky (Lisp_Object, Lisp_Object);
 static INTERVAL merge_interval_right (INTERVAL);
 static INTERVAL reproduce_tree (INTERVAL, INTERVAL);
@@ -798,7 +803,7 @@
    and check the hungry bits of both.  Then add the length going back up
    to the root.  */

-static INTERVAL
+INTERVAL
 adjust_intervals_for_insertion (INTERVAL tree, EMACS_INT position,
 				EMACS_INT length)
 {
@@ -859,7 +864,7 @@
    interval.  Another possibility would be to create a new interval for
    this text, and make it have the merged properties of both ends.  */

-static INTERVAL
+INTERVAL
 adjust_intervals_for_insertion (INTERVAL tree,
 				EMACS_INT position, EMACS_INT length)
 {
@@ -1369,7 +1374,7 @@
    text.  The deletion is effected at position START (which is a
    buffer position, i.e. origin 1).  */

-static void
+void
 adjust_intervals_for_deletion (struct buffer *buffer,
 			       EMACS_INT start, EMACS_INT length)
 {
@@ -1425,9 +1430,8 @@
    compiler that does not allow calling a static function (here,
    adjust_intervals_for_deletion) from a non-static inline function.  */

-static inline void
-static_offset_intervals (struct buffer *buffer, EMACS_INT start,
-			 EMACS_INT length)
+inline void
+offset_intervals (struct buffer *buffer, EMACS_INT start, EMACS_INT length)
 {
   if (NULL_INTERVAL_P (BUF_INTERVALS (buffer)) || length == 0)
     return;
@@ -1440,12 +1444,6 @@
       adjust_intervals_for_deletion (buffer, start, -length);
     }
 }
-
-inline void
-offset_intervals (struct buffer *buffer, EMACS_INT start, EMACS_INT length)
-{
-  static_offset_intervals (buffer, start, length);
-}
 \f
 /* Merge interval I with its lexicographic successor. The resulting
    interval is returned, and has the properties of the original

Re: External inline functions

[Eli Zaretskii]

> Date: Sun, 06 Nov 2011 13:20:07 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: Andreas Schwab <schwab@linux-m68k.org>, Eli Zaretskii <eliz@gnu.org>, 
>  emacs-devel@gnu.org
> 
> how about if we simply stop using extern inline functions?

That'd be my suggestion as well, if no one objects.

Re: External inline functions

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> how about if we simply stop using extern inline functions?
>
> That'd be my suggestion as well, if no one objects.

Fine by me.

Re: External inline functions

[Paul Eggert]

On 11/08/11 01:59, Chong Yidong wrote:
> Fine by me.

OK, done, as bzr 106330.