Is INLINE_HEADER_BEGIN still useful?

Is INLINE_HEADER_BEGIN still useful?

[Oleh Krehel]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]


Hi all,

This macro, encountered in most headers, seems to work around some
compilers not supporting C99.  A quick internet search shows that no
other software except Emacs uses this (any more, I assume some did in
the past).

Is it still useful? If not, it unnecessarily complicates the header
structure and should be removed.

I attach an example patch for only buffer.h that compiles and works
well.

Also, what's the stance on C11? I don't know a lot of C outside of what
C++ includes, but I'm considering to learn more of it just for Emacs.
Is C11 encouraged / allowed / discouraged / disallowed?

I assume that C99 is at least allowed since #17487. Is it encouraged?
In that case, surely INLINE_HEADER_BEGIN should be removed.

Oleh


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-INLINE_HEADER_BEGIN-in-buffer.h.patch --]
[-- Type: text/x-diff, Size: 7669 bytes --]

From 6da130ba276a9c7f0f13b5993cace8f943cb7334 Mon Sep 17 00:00:00 2001
From: Oleh Krehel <ohwoeowho@gmail.com>
Date: Wed, 29 Apr 2015 13:11:18 +0200
Subject: [PATCH] Remove INLINE_HEADER_BEGIN in buffer.h

---
 src/buffer.h | 80 +++++++++++++++++++++++++++++-------------------------------
 1 file changed, 38 insertions(+), 42 deletions(-)

diff --git a/src/buffer.h b/src/buffer.h
index a0410d4..0a72154 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -21,8 +21,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <sys/types.h>
 #include <time.h>
 
-INLINE_HEADER_BEGIN
-
 /* Accessing the parameters of the current buffer.  */
 
 /* These macros come in pairs, one for the char position
@@ -878,102 +876,102 @@ struct buffer
 /* Most code should use these functions to set Lisp fields in struct
    buffer.  (Some setters that are private to a single .c file are
    defined as static in those files.)  */
-INLINE void
+inline void
 bset_bidi_paragraph_direction (struct buffer *b, Lisp_Object val)
 {
   b->bidi_paragraph_direction_ = val;
 }
-INLINE void
+inline void
 bset_cache_long_scans (struct buffer *b, Lisp_Object val)
 {
   b->cache_long_scans_ = val;
 }
-INLINE void
+inline void
 bset_case_canon_table (struct buffer *b, Lisp_Object val)
 {
   b->case_canon_table_ = val;
 }
-INLINE void
+inline void
 bset_case_eqv_table (struct buffer *b, Lisp_Object val)
 {
   b->case_eqv_table_ = val;
 }
-INLINE void
+inline void
 bset_directory (struct buffer *b, Lisp_Object val)
 {
   b->directory_ = val;
 }
-INLINE void
+inline void
 bset_display_count (struct buffer *b, Lisp_Object val)
 {
   b->display_count_ = val;
 }
-INLINE void
+inline void
 bset_display_time (struct buffer *b, Lisp_Object val)
 {
   b->display_time_ = val;
 }
-INLINE void
+inline void
 bset_downcase_table (struct buffer *b, Lisp_Object val)
 {
   b->downcase_table_ = val;
 }
-INLINE void
+inline void
 bset_enable_multibyte_characters (struct buffer *b, Lisp_Object val)
 {
   b->enable_multibyte_characters_ = val;
 }
-INLINE void
+inline void
 bset_filename (struct buffer *b, Lisp_Object val)
 {
   b->filename_ = val;
 }
-INLINE void
+inline void
 bset_keymap (struct buffer *b, Lisp_Object val)
 {
   b->keymap_ = val;
 }
-INLINE void
+inline void
 bset_last_selected_window (struct buffer *b, Lisp_Object val)
 {
   b->last_selected_window_ = val;
 }
-INLINE void
+inline void
 bset_local_var_alist (struct buffer *b, Lisp_Object val)
 {
   b->local_var_alist_ = val;
 }
-INLINE void
+inline void
 bset_mark_active (struct buffer *b, Lisp_Object val)
 {
   b->mark_active_ = val;
 }
-INLINE void
+inline void
 bset_point_before_scroll (struct buffer *b, Lisp_Object val)
 {
   b->point_before_scroll_ = val;
 }
-INLINE void
+inline void
 bset_read_only (struct buffer *b, Lisp_Object val)
 {
   b->read_only_ = val;
 }
-INLINE void
+inline void
 bset_truncate_lines (struct buffer *b, Lisp_Object val)
 {
   b->truncate_lines_ = val;
 }
-INLINE void
+inline void
 bset_undo_list (struct buffer *b, Lisp_Object val)
 {
   b->undo_list_ = val;
 }
-INLINE void
+inline void
 bset_upcase_table (struct buffer *b, Lisp_Object val)
 {
   b->upcase_table_ = val;
 }
-INLINE void
+inline void
 bset_width_table (struct buffer *b, Lisp_Object val)
 {
   b->width_table_ = val;
@@ -1090,7 +1088,7 @@ extern void set_buffer_if_live (Lisp_Object);
 
 /* Return B as a struct buffer pointer, defaulting to the current buffer.  */
 
-INLINE struct buffer *
+inline struct buffer *
 decode_buffer (Lisp_Object b)
 {
   return NILP (b) ? current_buffer : (CHECK_BUFFER (b), XBUFFER (b));
@@ -1105,7 +1103,7 @@ decode_buffer (Lisp_Object b)
    windows than the selected one requires a select_window at some
    time, and that increments windows_or_buffers_changed.  */
 
-INLINE void
+inline void
 set_buffer_internal (struct buffer *b)
 {
   if (current_buffer != b)
@@ -1115,7 +1113,7 @@ set_buffer_internal (struct buffer *b)
 /* Arrange to go back to the original buffer after the next
    call to unbind_to if the original buffer is still alive.  */
 
-INLINE void
+inline void
 record_unwind_current_buffer (void)
 {
   record_unwind_protect (set_buffer_if_live, Fcurrent_buffer ());
@@ -1150,7 +1148,7 @@ extern Lisp_Object Vbuffer_alist;
 
 /* Get text properties of B.  */
 
-INLINE INTERVAL
+inline INTERVAL
 buffer_intervals (struct buffer *b)
 {
   eassert (b->text != NULL);
@@ -1159,7 +1157,7 @@ buffer_intervals (struct buffer *b)
 
 /* Set text properties of B to I.  */
 
-INLINE void
+inline void
 set_buffer_intervals (struct buffer *b, INTERVAL i)
 {
   eassert (b->text != NULL);
@@ -1168,7 +1166,7 @@ set_buffer_intervals (struct buffer *b, INTERVAL i)
 
 /* Non-zero if current buffer has overlays.  */
 
-INLINE bool
+inline bool
 buffer_has_overlays (void)
 {
   return current_buffer->overlays_before || current_buffer->overlays_after;
@@ -1188,7 +1186,7 @@ buffer_has_overlays (void)
    the buffer to the next character after fetching this one.  Instead,
    use either FETCH_CHAR_ADVANCE or STRING_CHAR_AND_LENGTH.  */
 
-INLINE int
+inline int
 FETCH_MULTIBYTE_CHAR (ptrdiff_t pos)
 {
   unsigned char *p = ((pos >= GPT_BYTE ? GAP_SIZE : 0)
@@ -1200,7 +1198,7 @@ FETCH_MULTIBYTE_CHAR (ptrdiff_t pos)
    If POS doesn't point the head of valid multi-byte form, only the byte at
    POS is returned.  No range checking.  */
 
-INLINE int
+inline int
 BUF_FETCH_MULTIBYTE_CHAR (struct buffer *buf, ptrdiff_t pos)
 {
   unsigned char *p
@@ -1211,7 +1209,7 @@ BUF_FETCH_MULTIBYTE_CHAR (struct buffer *buf, ptrdiff_t pos)
 
 /* Return number of windows showing B.  */
 
-INLINE int
+inline int
 buffer_window_count (struct buffer *b)
 {
   if (b->base_buffer)
@@ -1318,13 +1316,13 @@ extern int last_per_buffer_idx;
 /* Functions to get and set default value of the per-buffer
    variable at offset OFFSET in the buffer structure.  */
 
-INLINE Lisp_Object
+inline Lisp_Object
 per_buffer_default (int offset)
 {
   return *(Lisp_Object *)(offset + (char *) &buffer_defaults);
 }
 
-INLINE void
+inline void
 set_per_buffer_default (int offset, Lisp_Object value)
 {
   *(Lisp_Object *)(offset + (char *) &buffer_defaults) = value;
@@ -1333,20 +1331,20 @@ set_per_buffer_default (int offset, Lisp_Object value)
 /* Functions to get and set buffer-local value of the per-buffer
    variable at offset OFFSET in the buffer structure.  */
 
-INLINE Lisp_Object
+inline Lisp_Object
 per_buffer_value (struct buffer *b, int offset)
 {
   return *(Lisp_Object *)(offset + (char *) b);
 }
 
-INLINE void
+inline void
 set_per_buffer_value (struct buffer *b, int offset, Lisp_Object value)
 {
   *(Lisp_Object *)(offset + (char *) b) = value;
 }
 
 /* Downcase a character C, or make no change if that cannot be done.  */
-INLINE int
+inline int
 downcase (int c)
 {
   Lisp_Object downcase_table = BVAR (current_buffer, downcase_table);
@@ -1355,10 +1353,10 @@ downcase (int c)
 }
 
 /* True if C is upper case.  */
-INLINE bool uppercasep (int c) { return downcase (c) != c; }
+inline bool uppercasep (int c) { return downcase (c) != c; }
 
 /* Upcase a character C known to be not upper case.  */
-INLINE int
+inline int
 upcase1 (int c)
 {
   Lisp_Object upcase_table = BVAR (current_buffer, upcase_table);
@@ -1367,13 +1365,11 @@ upcase1 (int c)
 }
 
 /* True if C is lower case.  */
-INLINE bool
+inline bool
 lowercasep (int c)
 {
   return !uppercasep (c) && upcase1 (c) != c;
 }
 
 /* Upcase a character C, or make no change if that cannot be done.  */
-INLINE int upcase (int c) { return uppercasep (c) ? c : upcase1 (c); }
-
-INLINE_HEADER_END
+inline int upcase (int c) { return uppercasep (c) ? c : upcase1 (c); }
-- 
1.8.4

Re: Is INLINE_HEADER_BEGIN still useful?

[Paul Eggert]

Oleh Krehel wrote:
> This macro, encountered in most headers, seems to work around some
> compilers not supporting C99.  A quick internet search shows that no
> other software except Emacs uses this (any more, I assume some did in
> the past).

INLINE_HEADER_BEGIN is defined in src/conf_post.h.  The command "git blame 
src/conf_post.h" shows it was added in 2012, so it's not that old.  (For 
software archaeology questions like this, "git blame" Is Your Friend.)

> Is it still useful?

It's defined in terms of _GL_INLINE_HEADER_BEGIN.  The comment for the 
definition of _GL_INLINE_HEADER_BEGIN says it's a workaround for GCC bugs 54133 
(fixed in December 2013 on the GCC trunk) and 63877 (fixed November 2014).  So I 
guess these macros are useful until we can assume that everybody is building 
Emacs with GCC 5.1.0 or later.  (Though someone should test this guess.)

If my guess is right, then given the slow pace at which GCC gets out into the 
field, we'll need this macro for another decade or two.

> Is C11 encouraged / allowed / discouraged / disallowed?
>
> I assume that C99 is at least allowed since #17487. Is it encouraged?

It depends on what C99 features you're talking about.  Emacs can use ordinary 
C99 features such as declaration after statements and variadic macros.  The 
Emacs source does not assume conformance to C99 in every detail, as not every 
implementation supports every C99 feature (even GCC doesn't do that).

Although C11 features can be used on systems where they're known to work, Emacs 
needs to be portable to pre-C11 systems, as people still regularly build Emacs 
with GCC 3.x.  For example, the Emacs source uses the 'alignof' syntax of C11, 
but Emacs supplies its own implementation of 'alignof' on pre-C11 systems that 
lack it.

There are also some style issues.  The Emacs comment style prefers commenting /* 
like this */, not // like this (introduced in C99).  The Emacs source code does 
not ever use trigraphs, even though C99 allows them.  That sort of thing.

Re: Is INLINE_HEADER_BEGIN still useful?

[Oleh Krehel]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Oleh Krehel wrote:
>> This macro, encountered in most headers, seems to work around some
>> compilers not supporting C99.  A quick internet search shows that no
>> other software except Emacs uses this (any more, I assume some did in
>> the past).
>
> INLINE_HEADER_BEGIN is defined in src/conf_post.h.  The command "git
> blame src/conf_post.h" shows it was added in 2012, so it's not that
> old.  (For software archaeology questions like this, "git blame" Is
> Your Friend.)

Thanks for the tip.

>> Is it still useful?
>
> It's defined in terms of _GL_INLINE_HEADER_BEGIN.  The comment for the
> definition of _GL_INLINE_HEADER_BEGIN says it's a workaround for GCC
> bugs 54133 (fixed in December 2013 on the GCC trunk) and 63877 (fixed
> November 2014).  So I guess these macros are useful until we can
> assume that everybody is building Emacs with GCC 5.1.0 or later.
> (Though someone should test this guess.)

I don't see how 54133 can be related to anything with the "inline"
declaration.  And 63877 is just a bogus warning for a specific case, not
a big deal. I got no warnings when I tested my buffer.h patch.

> If my guess is right, then given the slow pace at which GCC gets out
> into the field, we'll need this macro for another decade or two.

I'm getting no problems so far with my default 4.8.2.  I can try to
patch the whole thing, not just buffer.h. Unless it's useless, because
4.8.2 is considered too new.

>> Is C11 encouraged / allowed / discouraged / disallowed?
>>
>> I assume that C99 is at least allowed since #17487. Is it encouraged?
>
> It depends on what C99 features you're talking about.  Emacs can use
> ordinary C99 features such as declaration after statements and
> variadic macros.  The Emacs source does not assume conformance to C99
> in every detail, as not every implementation supports every C99
> feature (even GCC doesn't do that).
>
> Although C11 features can be used on systems where they're known to
> work, Emacs needs to be portable to pre-C11 systems, as people still
> regularly build Emacs with GCC 3.x.  For example, the Emacs source
> uses the 'alignof' syntax of C11, but Emacs supplies its own
> implementation of 'alignof' on pre-C11 systems that lack it.
>
> There are also some style issues.  The Emacs comment style prefers
> commenting /* like this */, not // like this (introduced in C99).  The
> Emacs source code does not ever use trigraphs, even though C99 allows
> them.  That sort of thing.

Thanks for the comments, they're very helpful. Maybe someone on the list
could respond if they use a compiler that doesn't support the C99
"inline" declaration?

Oleh

Re: Is INLINE_HEADER_BEGIN still useful?

[Paul Eggert]

Oleh Krehel wrote:
> I got no warnings when I tested my buffer.h patch.

Try './configure --enable-gcc-warnings'.  Certainly the warnings do appear on 
some modern GCC implementations -- otherwise we wouldn't have gone to all that 
work to suppress them so recently.

> Maybe someone on the list
> could respond if they use a compiler that doesn't support the C99
> "inline" declaration?

It's not as simple as that.  Although most compilers have grokked the 'inline' 
keyword for some time, this does not imply full support for C99 'inline' 
everywhere.  For example, OS X 10.8's standard headers do not work when used by 
extern inline functions, so Emacs can't simply assume C99 inline will work on 
this relatively recent platform (released July 2012 and still in wide use).

Re: Is INLINE_HEADER_BEGIN still useful?

[Oleh Krehel]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Oleh Krehel wrote:
>> I got no warnings when I tested my buffer.h patch.
>
> Try './configure --enable-gcc-warnings'.  Certainly the warnings do
> appear on some modern GCC implementations -- otherwise we wouldn't
> have gone to all that work to suppress them so recently.

OK, but that just makes each warning into an error.  My build terminates
for unused variable "spool_name", which is unused because I don't have
the right configuration.

>> Maybe someone on the list
>> could respond if they use a compiler that doesn't support the C99
>> "inline" declaration?
>
> It's not as simple as that.  Although most compilers have grokked the
> 'inline' keyword for some time, this does not imply full support for
> C99 'inline' everywhere.  For example, OS X 10.8's standard headers do
> not work when used by extern inline functions, so Emacs can't simply
> assume C99 inline will work on this relatively recent platform
> (released July 2012 and still in wide use).

Not all inline are extern inline, buffer.h doesn't need to be extern
inline, if I understand correctly. But anyway, if you say it's needed,
it's needed.

One more question, is there a list of platforms on which Emacs is known
to build fine, by version? Maybe 25.1 doesn't build on OS X 10.8 for
reasons other than C99 compatibility.

Oleh

Re: Is INLINE_HEADER_BEGIN still useful?

[Paul Eggert]

On 04/29/2015 08:33 AM, Oleh Krehel wrote:
>
>> Try './configure --enable-gcc-warnings'.  Certainly the warnings do
>> appear on some modern GCC implementations -- otherwise we wouldn't
>> have gone to all that work to suppress them so recently.
> OK, but that just makes each warning into an error.

Actually, --enable-gcc-warnings does two things.  First, it turns 
warnings into errors.  Second, it enables many warnings that are 
normally disabled.  It's intended for developers who want to apply many 
static checks to programs, partly to increase the probability that the 
programs will be portable to random platforms the developers don't have 
easy access to.

These warnings are not the default, because the default is intended for 
ordinary installation builds where we would rather have the build work 
than to have it be derailed by a false alarm.

> My build terminates
> for unused variable "spool_name", which is unused because I don't have
> the right configuration.
>

I regularly test --enable-gcc-warnings on a fairly-fully-loaded Fedora 
21 x86-64 host, and it should be clean there.  It's not clean on 
arbitrary configurations, and isn't intended to be clean everywhere 
because that would be a of hassle for not-that-much payoff.

If you regularly use --enable-gcc-warnings for development (which is a 
good idea, if you're editing the C code), and if it has false alarms for 
you, please feel free to fix them.  If it's just a matter of missing 
libraries, though, I expect it would be better to install the libraries, 
if only so that you can check that your changes don't hurt calls to 
those libraries.

>>> Maybe someone on the list
>>> could respond if they use a compiler that doesn't support the C99
>>> "inline" declaration?
>> It's not as simple as that.  Although most compilers have grokked the
>> 'inline' keyword for some time, this does not imply full support for
>> C99 'inline' everywhere.  For example, OS X 10.8's standard headers do
>> not work when used by extern inline functions, so Emacs can't simply
>> assume C99 inline will work on this relatively recent platform
>> (released July 2012 and still in wide use).
> Not all inline are extern inline, buffer.h doesn't need to be extern
> inline, if I understand correctly.

Why not?  You can't just make those functions plain 'inline', as that 
would not conform to the C99 rules and would not be portable.

> One more question, is there a list of platforms on which Emacs is known
> to build fine, by version?

Sorry, not that I know of.  The list of platforms would be pretty large, 
as Emacs is portable to a lot of platforms that are constantly mutating.

> Maybe 25.1 doesn't build on OS X 10.8 for
> reasons other than C99 compatibility.
>

No, Emacs builds on OS X 10.8.  The C99 issues aren't a problem, because 
Emacs macros like INLINE_HEADER_BEGIN work around the OS X 10.8 bugs 
with inline functions.  For more, please see m4/extern-inline.m4 (this 
file is taken from gnulib).