Unbreak build on macOS

Unbreak build on macOS

[Mattias Engdegård]

On macOS, lib/fcntl.h does

#   define open rpl_open

which now (f8ab90839f) gets included in src/font.c and breaks the line

      font_object = driver_list->driver->open (f, entity, psize);

As an immediate remedy, I added

#undef open

to src/font.c.

In general, '#define open' is a rather unpleasant thing to do. Can we at least assume enough of C99 to do

#define open(...) rpl_open (__VA_ARGS__)

instead? That would permit font.c to use bracketing as an alternative countermeasure.

Re: Unbreak build on macOS

[Richard Copley]

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

On Tue, 9 Jul 2019 at 12:44, Mattias Engdegård <mattiase@acm.org> wrote:

> On macOS, lib/fcntl.h does
>
> #   define open rpl_open
>
> which now (f8ab90839f) gets included in src/font.c and breaks the line
>
>       font_object = driver_list->driver->open (f, entity, psize);
>
> As an immediate remedy, I added
>
> #undef open
>
> to src/font.c.
>
> In general, '#define open' is a rather unpleasant thing to do. Can we at
> least assume enough of C99 to do
>
> #define open(...) rpl_open (__VA_ARGS__)
>
> instead? That would permit font.c to use bracketing as an alternative
> countermeasure.
>

The build is currently (as of Mattias' fdea0e602b) broken on MS Windows. I
didn't
check which recent revision introduced the breakage; possibly also
f8ab90839f.

make[1]: Entering directory '/c/projects/emacs/src'
  CC       font.o
font.c: In function 'font_open_entity':
font.c:2909:40: error: 'const struct font_driver' has no member named 'open'
       font_object = driver_list->driver->open (f, entity, psize);
                                        ^~

[-- Attachment #2: Type: text/html, Size: 1577 bytes --]

Re: Unbreak build on macOS

[Mattias Engdegård]

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

9 juli 2019 kl. 15.51 skrev Richard Copley <rcopley@gmail.com>:
> 
> The build is currently (as of Mattias' fdea0e602b) broken on MS Windows. I didn't
> check which recent revision introduced the breakage; possibly also f8ab90839f.

Sorry to hear that --- apparently font.h is being preprocessed with '#define open #rpl_open' on Windows.
Would you try this smelly hack instead?


[-- Attachment #2: 0001-Fix-Windows-build-by-renaming-open-open_.patch --]
[-- Type: application/octet-stream, Size: 6572 bytes --]

From 8fddeef12f517379f2fe8c80f4c3bd08ebb4a13c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Tue, 9 Jul 2019 16:44:24 +0200
Subject: [PATCH] Fix Windows build by renaming open -> open_

* src/xftfont.c (xftfont_driver)
* src/xfont.c (xfont_driver)
* src/nsfont.m (nsfont_driver)
* src/macfont.m (macfont_driver)
* src/ftxfont.c (ftxfont_driver)
* src/ftfont.c (ftfont_driver)
* src/ftcrfont.c (ftcrfont_driver)
* src/font.h (struct font_driver)
* src/font.c (font_open_entity):
Rename `open' member to `open_', to avoid clash with preprocessor define
of `open' in lib/fcntl.h.  Remove earlier #undef hack.
---
 src/font.c     |  6 +-----
 src/font.h     | 10 +++++-----
 src/ftcrfont.c |  2 +-
 src/ftfont.c   |  2 +-
 src/ftxfont.c  |  2 +-
 src/macfont.m  |  2 +-
 src/nsfont.m   |  2 +-
 src/xfont.c    |  2 +-
 src/xftfont.c  |  2 +-
 9 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/font.c b/src/font.c
index 457f3f9958..dd77b18cc1 100644
--- a/src/font.c
+++ b/src/font.c
@@ -44,10 +44,6 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include TERM_HEADER
 #endif /* HAVE_WINDOW_SYSTEM */
 
-/* Avoid macro definition of `open' in generated lib/fcntl.h to mess up
-   use of it as a struct member.  */
-#undef open
-
 #define DEFAULT_ENCODING Qiso8859_1
 
 /* Vector of Vfont_weight_table, Vfont_slant_table, and Vfont_width_table. */
@@ -2906,7 +2902,7 @@ font_open_entity (struct frame *f, Lisp_Object entity, int pixel_size)
      width and height.  */
   for (psize = pixel_size; ; psize++)
     {
-      font_object = driver_list->driver->open (f, entity, psize);
+      font_object = driver_list->driver->open_ (f, entity, psize);
       if (NILP (font_object))
 	return Qnil;
       font = XFONT_OBJECT (font_object);
diff --git a/src/font.h b/src/font.h
index 3387878ad3..35cc865b28 100644
--- a/src/font.h
+++ b/src/font.h
@@ -58,7 +58,7 @@ INLINE_HEADER_BEGIN
 	Lisp object encapsulating "struct font".  This corresponds to
 	an opened font.
 
-	Note: Only the method `open' of a font-driver can create this
+	Note: Only the method `open_' of a font-driver can create this
 	object, and it should never be modified by Lisp.  */
 
 
@@ -594,9 +594,9 @@ struct font_driver
      :weight, :slant, :width, :size, :dpi, :spacing, :avgwidth.  If
      the font is scalable, :size and :avgwidth must be 0.
 
-     The `open' method of the same font-backend is called with one of
+     The `open_' method of the same font-backend is called with one of
      the returned font-entities.  If the backend needs additional
-     information to be used in `open' method, this method can add any
+     information to be used in `open_' method, this method can add any
      Lispy value using the property :font-entity to the entities.
 
      This and the following `match' are the only APIs that allocate
@@ -623,8 +623,8 @@ struct font_driver
 
   /* Open a font specified by FONT_ENTITY on frame F.  If the font is
      scalable, open it with PIXEL_SIZE.  */
-  Lisp_Object (*open) (struct frame *f, Lisp_Object font_entity,
-                       int pixel_size);
+  Lisp_Object (*open_) (struct frame *f, Lisp_Object font_entity,
+                        int pixel_size);
 
   /* Close FONT.  NOTE: this can be called by GC.  */
   void (*close) (struct font *font);
diff --git a/src/ftcrfont.c b/src/ftcrfont.c
index 9378621216..ef8af1f6b3 100644
--- a/src/ftcrfont.c
+++ b/src/ftcrfont.c
@@ -576,7 +576,7 @@ struct font_driver const ftcrfont_driver =
   .list = ftcrfont_list,
   .match = ftcrfont_match,
   .list_family = ftfont_list_family,
-  .open = ftcrfont_open,
+  .open_ = ftcrfont_open,
   .close = ftcrfont_close,
   .has_char = ftcrfont_has_char,
   .encode_char = ftcrfont_encode_char,
diff --git a/src/ftfont.c b/src/ftfont.c
index a80e2fb5c4..8c619acace 100644
--- a/src/ftfont.c
+++ b/src/ftfont.c
@@ -3039,7 +3039,7 @@ static struct font_driver const ftfont_driver =
   .list = ftfont_list,
   .match = ftfont_match,
   .list_family = ftfont_list_family,
-  .open = ftfont_open,
+  .open_ = ftfont_open,
   .close = ftfont_close,
   .has_char = ftfont_has_char,
   .encode_char = ftfont_encode_char,
diff --git a/src/ftxfont.c b/src/ftxfont.c
index ae7d1a5a9b..a4818801d9 100644
--- a/src/ftxfont.c
+++ b/src/ftxfont.c
@@ -335,7 +335,7 @@ struct font_driver const ftxfont_driver =
   .list = ftxfont_list,
   .match = ftxfont_match,
   .list_family = ftfont_list_family,
-  .open = ftxfont_open,
+  .open_ = ftxfont_open,
   .close = ftxfont_close,
   .has_char = ftfont_has_char,
   .encode_char = ftfont_encode_char,
diff --git a/src/macfont.m b/src/macfont.m
index 2b7f963fd6..51d503b8f4 100644
--- a/src/macfont.m
+++ b/src/macfont.m
@@ -1663,7 +1663,7 @@ static int macfont_variation_glyphs (struct font *, int c,
   .match = macfont_match,
   .list_family = macfont_list_family,
   .free_entity = macfont_free_entity,
-  .open = macfont_open,
+  .open_ = macfont_open,
   .close = macfont_close,
   .has_char = macfont_has_char,
   .encode_char = macfont_encode_char,
diff --git a/src/nsfont.m b/src/nsfont.m
index e22a954e63..a4d2415f7a 100644
--- a/src/nsfont.m
+++ b/src/nsfont.m
@@ -1491,7 +1491,7 @@ - (void)setIntAttribute: (NSInteger)attributeTag value: (NSInteger)val
   .list = nsfont_list,
   .match = nsfont_match,
   .list_family = nsfont_list_family,
-  .open = nsfont_open,
+  .open_ = nsfont_open,
   .close = nsfont_close,
   .has_char = nsfont_has_char,
   .encode_char = nsfont_encode_char,
diff --git a/src/xfont.c b/src/xfont.c
index 9a8417b12d..c4770d19e4 100644
--- a/src/xfont.c
+++ b/src/xfont.c
@@ -1106,7 +1106,7 @@ struct font_driver const xfont_driver =
   .list = xfont_list,
   .match = xfont_match,
   .list_family = xfont_list_family,
-  .open = xfont_open,
+  .open_ = xfont_open,
   .close = xfont_close,
   .prepare_face = xfont_prepare_face,
   .has_char = xfont_has_char,
diff --git a/src/xftfont.c b/src/xftfont.c
index 74add58007..e917859a49 100644
--- a/src/xftfont.c
+++ b/src/xftfont.c
@@ -643,7 +643,7 @@ struct font_driver const xftfont_driver =
   .list = xftfont_list,
   .match = xftfont_match,
   .list_family = ftfont_list_family,
-  .open = xftfont_open,
+  .open_ = xftfont_open,
   .close = xftfont_close,
   .prepare_face = xftfont_prepare_face,
   .done_face = xftfont_done_face,
-- 
2.20.1 (Apple Git-117)

Re: Unbreak build on macOS

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Tue, 9 Jul 2019 16:46:30 +0200
> Cc: Paul Eggert <eggert@cs.ucla.edu>, Emacs developers <emacs-devel@gnu.org>
> 
> Sorry to hear that --- apparently font.h is being preprocessed with '#define open #rpl_open' on Windows.

Not rpl_open, but sys_open, see nt/inc/ms-w32.h.

> Would you try this smelly hack instead?

I'd prefer open_font to open_.

Thanks.

Re: Unbreak build on macOS

[Mattias Engdegård]

9 juli 2019 kl. 17.29 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> I'd prefer open_font to open_.

Then open_font it is -- pushed. Thank you.

Re: Unbreak build on macOS

[Eli Zaretskii]

> Date: Tue, 09 Jul 2019 18:29:29 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: rcopley@gmail.com, eggert@cs.ucla.edu, emacs-devel@gnu.org
> 
> > From: Mattias Engdegård <mattiase@acm.org>
> > Date: Tue, 9 Jul 2019 16:46:30 +0200
> > Cc: Paul Eggert <eggert@cs.ucla.edu>, Emacs developers <emacs-devel@gnu.org>
> > 
> > Sorry to hear that --- apparently font.h is being preprocessed with '#define open #rpl_open' on Windows.
> 
> Not rpl_open, but sys_open, see nt/inc/ms-w32.h.
> 
> > Would you try this smelly hack instead?
> 
> I'd prefer open_font to open_.

And then we should probably rename close to close_font as well.

Thanks.

Re: Unbreak build on macOS

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Tue, 9 Jul 2019 14:51:16 +0100
> Cc: Paul Eggert <eggert@cs.ucla.edu>, Emacs developers <emacs-devel@gnu.org>
> 
> The build is currently (as of Mattias' fdea0e602b) broken on MS Windows. I didn't
> check which recent revision introduced the breakage; possibly also f8ab90839f.

No, it's Mattias's fix that broke MS-Windows.

Please try with the current master.

Re: Unbreak build on macOS

[Mattias Engdegård]

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

9 juli 2019 kl. 17.17 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> No, it's Mattias's fix that broke MS-Windows.

In some sense it was already broken on Windows (and with your fix, still is), but happens to work by chance: font.h and font.c are both compiled with the same unintended redefinition of `open'. Not quite satisfactory.
Here is the rebased renaming patch.


[-- Attachment #2: 0001-Fix-Windows-build-by-renaming-open-open_.patch --]
[-- Type: application/octet-stream, Size: 6631 bytes --]

From 90e44011ee51eb6bd046d69be45a4441172c2571 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Tue, 9 Jul 2019 16:44:24 +0200
Subject: [PATCH] Fix Windows build by renaming open -> open_

* src/xftfont.c (xftfont_driver):
* src/xfont.c (xfont_driver):
* src/nsfont.m (nsfont_driver):
* src/macfont.m (macfont_driver):
* src/ftxfont.c (ftxfont_driver):
* src/ftfont.c (ftfont_driver):
* src/ftcrfont.c (ftcrfont_driver):
* src/font.h (struct font_driver):
* src/font.c (font_open_entity):
Rename `open' member to `open_', to avoid clash with preprocessor define
of `open' in lib/fcntl.h and nt/inc/ms-w32.h.  Remove earlier #undef hack.
---
 src/font.c     |  8 +-------
 src/font.h     | 10 +++++-----
 src/ftcrfont.c |  2 +-
 src/ftfont.c   |  2 +-
 src/ftxfont.c  |  2 +-
 src/macfont.m  |  2 +-
 src/nsfont.m   |  2 +-
 src/xfont.c    |  2 +-
 src/xftfont.c  |  2 +-
 9 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/src/font.c b/src/font.c
index ffd5064493..dd77b18cc1 100644
--- a/src/font.c
+++ b/src/font.c
@@ -44,12 +44,6 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include TERM_HEADER
 #endif /* HAVE_WINDOW_SYSTEM */
 
-/* Avoid macro definition of `open' in generated lib/fcntl.h to mess up
-   use of it as a struct member.  */
-#ifndef WINDOWSNT
-#undef open
-#endif
-
 #define DEFAULT_ENCODING Qiso8859_1
 
 /* Vector of Vfont_weight_table, Vfont_slant_table, and Vfont_width_table. */
@@ -2908,7 +2902,7 @@ font_open_entity (struct frame *f, Lisp_Object entity, int pixel_size)
      width and height.  */
   for (psize = pixel_size; ; psize++)
     {
-      font_object = driver_list->driver->open (f, entity, psize);
+      font_object = driver_list->driver->open_ (f, entity, psize);
       if (NILP (font_object))
 	return Qnil;
       font = XFONT_OBJECT (font_object);
diff --git a/src/font.h b/src/font.h
index 3387878ad3..35cc865b28 100644
--- a/src/font.h
+++ b/src/font.h
@@ -58,7 +58,7 @@ INLINE_HEADER_BEGIN
 	Lisp object encapsulating "struct font".  This corresponds to
 	an opened font.
 
-	Note: Only the method `open' of a font-driver can create this
+	Note: Only the method `open_' of a font-driver can create this
 	object, and it should never be modified by Lisp.  */
 
 
@@ -594,9 +594,9 @@ struct font_driver
      :weight, :slant, :width, :size, :dpi, :spacing, :avgwidth.  If
      the font is scalable, :size and :avgwidth must be 0.
 
-     The `open' method of the same font-backend is called with one of
+     The `open_' method of the same font-backend is called with one of
      the returned font-entities.  If the backend needs additional
-     information to be used in `open' method, this method can add any
+     information to be used in `open_' method, this method can add any
      Lispy value using the property :font-entity to the entities.
 
      This and the following `match' are the only APIs that allocate
@@ -623,8 +623,8 @@ struct font_driver
 
   /* Open a font specified by FONT_ENTITY on frame F.  If the font is
      scalable, open it with PIXEL_SIZE.  */
-  Lisp_Object (*open) (struct frame *f, Lisp_Object font_entity,
-                       int pixel_size);
+  Lisp_Object (*open_) (struct frame *f, Lisp_Object font_entity,
+                        int pixel_size);
 
   /* Close FONT.  NOTE: this can be called by GC.  */
   void (*close) (struct font *font);
diff --git a/src/ftcrfont.c b/src/ftcrfont.c
index 9378621216..ef8af1f6b3 100644
--- a/src/ftcrfont.c
+++ b/src/ftcrfont.c
@@ -576,7 +576,7 @@ struct font_driver const ftcrfont_driver =
   .list = ftcrfont_list,
   .match = ftcrfont_match,
   .list_family = ftfont_list_family,
-  .open = ftcrfont_open,
+  .open_ = ftcrfont_open,
   .close = ftcrfont_close,
   .has_char = ftcrfont_has_char,
   .encode_char = ftcrfont_encode_char,
diff --git a/src/ftfont.c b/src/ftfont.c
index a80e2fb5c4..8c619acace 100644
--- a/src/ftfont.c
+++ b/src/ftfont.c
@@ -3039,7 +3039,7 @@ static struct font_driver const ftfont_driver =
   .list = ftfont_list,
   .match = ftfont_match,
   .list_family = ftfont_list_family,
-  .open = ftfont_open,
+  .open_ = ftfont_open,
   .close = ftfont_close,
   .has_char = ftfont_has_char,
   .encode_char = ftfont_encode_char,
diff --git a/src/ftxfont.c b/src/ftxfont.c
index ae7d1a5a9b..a4818801d9 100644
--- a/src/ftxfont.c
+++ b/src/ftxfont.c
@@ -335,7 +335,7 @@ struct font_driver const ftxfont_driver =
   .list = ftxfont_list,
   .match = ftxfont_match,
   .list_family = ftfont_list_family,
-  .open = ftxfont_open,
+  .open_ = ftxfont_open,
   .close = ftxfont_close,
   .has_char = ftfont_has_char,
   .encode_char = ftfont_encode_char,
diff --git a/src/macfont.m b/src/macfont.m
index 2b7f963fd6..51d503b8f4 100644
--- a/src/macfont.m
+++ b/src/macfont.m
@@ -1663,7 +1663,7 @@ static int macfont_variation_glyphs (struct font *, int c,
   .match = macfont_match,
   .list_family = macfont_list_family,
   .free_entity = macfont_free_entity,
-  .open = macfont_open,
+  .open_ = macfont_open,
   .close = macfont_close,
   .has_char = macfont_has_char,
   .encode_char = macfont_encode_char,
diff --git a/src/nsfont.m b/src/nsfont.m
index e22a954e63..a4d2415f7a 100644
--- a/src/nsfont.m
+++ b/src/nsfont.m
@@ -1491,7 +1491,7 @@ - (void)setIntAttribute: (NSInteger)attributeTag value: (NSInteger)val
   .list = nsfont_list,
   .match = nsfont_match,
   .list_family = nsfont_list_family,
-  .open = nsfont_open,
+  .open_ = nsfont_open,
   .close = nsfont_close,
   .has_char = nsfont_has_char,
   .encode_char = nsfont_encode_char,
diff --git a/src/xfont.c b/src/xfont.c
index 9a8417b12d..c4770d19e4 100644
--- a/src/xfont.c
+++ b/src/xfont.c
@@ -1106,7 +1106,7 @@ struct font_driver const xfont_driver =
   .list = xfont_list,
   .match = xfont_match,
   .list_family = xfont_list_family,
-  .open = xfont_open,
+  .open_ = xfont_open,
   .close = xfont_close,
   .prepare_face = xfont_prepare_face,
   .has_char = xfont_has_char,
diff --git a/src/xftfont.c b/src/xftfont.c
index 74add58007..e917859a49 100644
--- a/src/xftfont.c
+++ b/src/xftfont.c
@@ -643,7 +643,7 @@ struct font_driver const xftfont_driver =
   .list = xftfont_list,
   .match = xftfont_match,
   .list_family = ftfont_list_family,
-  .open = xftfont_open,
+  .open_ = xftfont_open,
   .close = xftfont_close,
   .prepare_face = xftfont_prepare_face,
   .done_face = xftfont_done_face,
-- 
2.20.1 (Apple Git-117)

Re: Unbreak build on macOS

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Tue, 9 Jul 2019 17:30:22 +0200
> Cc: Richard Copley <rcopley@gmail.com>, eggert@cs.ucla.edu,
>         emacs-devel@gnu.org
> 
> In some sense it was already broken on Windows (and with your fix, still is), but happens to work by chance: font.h and font.c are both compiled with the same unintended redefinition of `open'. Not quite satisfactory.

"Broken" as in "inelegant", maybe.  But not "broken" as in 'doesn't
build" or "doesn't work", right?

Re: Unbreak build on macOS

[Mattias Engdegård]

9 juli 2019 kl. 18.19 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> "Broken" as in "inelegant", maybe.  But not "broken" as in 'doesn't
> build" or "doesn't work", right?

Heavens, no. Just "broken" as in "brittle", and possibly "undefined behaviour in C", depending on how pervasive the #define is.

> And then we should probably rename close to close_font as well.

Good suggestion --- done.

I didn't bother with struct sound_device, however.

Re: Unbreak build on macOS

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Tue, 9 Jul 2019 18:49:45 +0200
> Cc: Richard Copley <rcopley@gmail.com>, eggert@cs.ucla.edu,
>         emacs-devel@gnu.org
> 
> > And then we should probably rename close to close_font as well.
> 
> Good suggestion --- done.

Thanks.

> I didn't bother with struct sound_device, however.

For another rainy day, I guess.

Re: Unbreak build on macOS

[Richard Copley]

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

 Mattias wrote
    Would you try this [...] instead?
and Eli wrote
    Please try with the current master.

Current master builds OK. Thanks.

[-- Attachment #2: Type: text/html, Size: 277 bytes --]

Re: Unbreak build on macOS

[Eli Zaretskii]

> From: Mattias Engdegård <mattiase@acm.org>
> Date: Tue, 9 Jul 2019 13:41:57 +0200
> 
> As an immediate remedy, I added
> 
> #undef open
> 
> to src/font.c.

Which broke the MS-Windows build...