Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Glenn Morris]

> branch: master
> commit b99ec5d5b11154bafb193ceaaac6976daafe3f82

I can provide no more information than this, but it seems this causes
hydra.nixos darwin builds to fail.

Ref https://hydra.nixos.org/build/135418161

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Paul Eggert]

On 1/22/21 2:49 PM, Glenn Morris wrote:
> I can provide no more information than this, but it seems this causes
> hydra.nixos darwin builds to fail.
> 
> https://hydra.nixos.org/build/135418161

When I click on that web page's line "Failed build produced output. 
Click here to inspect the output." 
<https://hydra.nixos.org/build/135418161/contents/1>, the result is a 
web page that says "404 Not Found Product 
/nix/store/3ib8nmn8wy3sqk4rk7n64j2hcvglkd6l-emacs-unknown has 
disappeared." If I click on the same web page's "Help" button it 
suggests running "curl 
https://hydra.nixos.org/build/135418161/nix/closure/emacs-unknown-none.closure.gz 
| gunzip | nix-store --import", but curl reports a 404 not found for 
that as well.

Perhaps someone who can build on macOS directly can provide more info 
about the problem. I had no problem building with clang 11.0 on Ubuntu 
but Apple's clang is special.

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Glenn Morris]

Paul Eggert wrote:

>> https://hydra.nixos.org/build/135418161
>
> When I click on that web page's line "Failed build produced output.
> Click here to inspect the output."

Use the "logfile" links, eg

https://hydra.nixos.org/build/135418161/log/raw

Or see bug#46046.

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Mattias Engdegård]

23 jan. 2021 kl. 01.46 skrev Paul Eggert <eggert@cs.ucla.edu>:

> Perhaps someone who can build on macOS directly can provide more info about the problem. I had no problem building with clang 11.0 on Ubuntu but Apple's clang is special.

Apple's clang seems to come with a builtin

#define __nonnull _Nonnull

which clashes with the the one in lib/cdefs.h. Perhaps we should call it something else.

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Philipp Stephani]

Am Sa., 23. Jan. 2021 um 11:14 Uhr schrieb Mattias Engdegård <mattiase@acm.org>:
>
> 23 jan. 2021 kl. 01.46 skrev Paul Eggert <eggert@cs.ucla.edu>:
>
> > Perhaps someone who can build on macOS directly can provide more info about the problem. I had no problem building with clang 11.0 on Ubuntu but Apple's clang is special.
>
> Apple's clang seems to come with a builtin
>
> #define __nonnull _Nonnull
>
> which clashes with the the one in lib/cdefs.h. Perhaps we should call it something else.


Gnulib should avoid identifiers starting with an underscore, as they
are reserved for the implementation.

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 23 Jan 2021 17:32:24 +0100
> Cc: Glenn Morris <rgm@gnu.org>, Paul Eggert <eggert@cs.ucla.edu>,
>  Emacs developers <emacs-devel@gnu.org>
> 
> > Apple's clang seems to come with a builtin
> >
> > #define __nonnull _Nonnull
> >
> > which clashes with the the one in lib/cdefs.h. Perhaps we should call it something else.
> 
> Gnulib should avoid identifiers starting with an underscore, as they
> are reserved for the implementation.

I don't think it can, at least not in general, because Gnulib replaces
some implementations, and this is a kind-of implementation itself.

The rule that you cite is for applications, not for system libraries.

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Paul Eggert]

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

On 1/23/21 2:11 AM, Mattias Engdegård wrote:
> 23 jan. 2021 kl. 01.46 skrev Paul Eggert <eggert@cs.ucla.edu>:
> 
>> Perhaps someone who can build on macOS directly can provide more info about the problem. I had no problem building with clang 11.0 on Ubuntu but Apple's clang is special.
> 
> Apple's clang seems to come with a builtin
> 
> #define __nonnull _Nonnull
> 
> which clashes with the the one in lib/cdefs.h. Perhaps we should call it something else.

Thanks for the diagnosis. Although calling it something else would 
obviously work, it would require renaming __nonnull everywhere within 
Gnulib (and eventually Glibc), and that would be entirely contrary to 
the intent of the Apple macro - which was to encourage compatibility 
with GNU code. As I understand things Apple's __nonnull keyword used to 
mean something else but collided with GNU's __nonnull macro, so they 
renamed the keyword and put in a backward-compatibility macro for old 
Applish code, but we don't need that macro and can just use __nonnull 
for our own use.

I attempted to fix the problem by installing the attached patch into 
Gnulib, and then merged Gnulib into Emacs master. Please give it a try, 
as I don't use macOS and so did not check the patch directly.

Phillip is correct in theory that Gnulib (application code) should avoid 
messing with reserved identifiers, but theory doesn't exactly equal 
practice here because we also want to minimize differences between 
Gnulib and Glibc.

[-- Attachment #2: 0001-libc-config-port-to-Xcode-7.patch --]
[-- Type: text/x-patch, Size: 2333 bytes --]

From 195a10b512067f18c79da0b2184689bc5520517c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 23 Jan 2021 10:19:24 -0800
Subject: [PATCH] libc-config: port to Xcode 7
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Mattias Engdegård in:
https://lists.gnu.org/r/emacs-devel/2021-01/msg01089.html
* lib/cdefs.h (__nonnull): If already defined but glibc is not in
use, override the definition with Gnulib’s _GL_ATTRIBUTE_NONNULL.
This is needed for Xcode 7, which has a ‘#define __nonnull
_Nonnull’ builtin for backwards-compatibility with an older Xcode
syntax that GNUish code never uses.
---
 ChangeLog   | 11 +++++++++++
 lib/cdefs.h |  6 ++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b1b81353e..43d1a1671 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2021-01-23  Paul Eggert  <eggert@cs.ucla.edu>
+
+	libc-config: port to Xcode 7
+	Problem reported by Mattias Engdegård in:
+	https://lists.gnu.org/r/emacs-devel/2021-01/msg01089.html
+	* lib/cdefs.h (__nonnull): If already defined but glibc is not in
+	use, override the definition with Gnulib’s _GL_ATTRIBUTE_NONNULL.
+	This is needed for Xcode 7, which has a ‘#define __nonnull
+	_Nonnull’ builtin for backwards-compatibility with an older Xcode
+	syntax that GNUish code never uses.
+
 2021-01-23  Bastien Roucariès  <rouca@debian.org>
 
 	explicit_bzero: Add fallback for other compilers.
diff --git a/lib/cdefs.h b/lib/cdefs.h
index 060a3d068..6f12c79da 100644
--- a/lib/cdefs.h
+++ b/lib/cdefs.h
@@ -320,14 +320,16 @@
 #endif
 
 /* The nonnull function attribute marks pointer parameters that
-   must not be NULL.  Do not define __nonnull if it is already defined,
-   for portability when this file is used in Gnulib.  */
+   must not be NULL.  */
 #ifndef __nonnull
 # if __GNUC_PREREQ (3,3) || __glibc_has_attribute (__nonnull__)
 #  define __nonnull(params) __attribute__ ((__nonnull__ params))
 # else
 #  define __nonnull(params)
 # endif
+#elif !defined __GLIBC__
+# undef __nonnull
+# define __nonnull(params) _GL_ATTRIBUTE_NONNULL (params)
 #endif
 
 /* If fortification mode, we warn about unused results of certain
-- 
2.27.0

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Doug Davis]

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

> On 1/23/21 2:11 AM, Mattias Engdegård wrote:
>> 23 jan. 2021 kl. 01.46 skrev Paul Eggert <eggert@cs.ucla.edu>:
>> 
>>> Perhaps someone who can build on macOS directly can provide more info about the problem. I had no problem building with clang 11.0 on Ubuntu but Apple's clang is special.
>> Apple's clang seems to come with a builtin
>> #define __nonnull _Nonnull
>> which clashes with the the one in lib/cdefs.h. Perhaps we should
>> call it something else.
>
> Thanks for the diagnosis. Although calling it something else would
> obviously work, it would require renaming __nonnull everywhere within
> Gnulib (and eventually Glibc), and that would be entirely contrary to
> the intent of the Apple macro - which was to encourage compatibility
> with GNU code. As I understand things Apple's __nonnull keyword used
> to mean something else but collided with GNU's __nonnull macro, so
> they renamed the keyword and put in a backward-compatibility macro for
> old Applish code, but we don't need that macro and can just use
> __nonnull for our own use.
>
> I attempted to fix the problem by installing the attached patch into
> Gnulib, and then merged Gnulib into Emacs master. Please give it a
> try, as I don't use macOS and so did not check the patch directly.
>

I just gave it a test and successfully built from scratch (latest commit
at b7068be5c4 and on macOS Big Sur with Apple clang 12).

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Mattias Engdegård]

23 jan. 2021 kl. 20.42 skrev Paul Eggert <eggert@cs.ucla.edu>:

> As I understand things Apple's __nonnull keyword used to mean something else but collided with GNU's __nonnull macro, so they renamed the keyword and put in a backward-compatibility macro for old Applish code, but we don't need that macro and can just use __nonnull for our own use.

That is fine as long as the Gnulib cdefs.h is not included in translation units also using Apple's framework headers, some of which do use __nonnull instead of _Nonnull. I suppose it's a later worry since it works for Emacs at the moment.

> I attempted to fix the problem by installing the attached patch into Gnulib, and then merged Gnulib into Emacs master. Please give it a try, as I don't use macOS and so did not check the patch directly.

Thank you! I can confirm that it works on macOS 10.14 (Mojave), Apple clang version 11.0.0 (clang-1100.0.33.17).

Re: master b99ec5d 3/3: Work around __has_attribute bug in clang 3.4

[Philipp Stephani]

Am Sa., 23. Jan. 2021 um 20:42 Uhr schrieb Paul Eggert <eggert@cs.ucla.edu>:
>
> On 1/23/21 2:11 AM, Mattias Engdegård wrote:
> > 23 jan. 2021 kl. 01.46 skrev Paul Eggert <eggert@cs.ucla.edu>:
> >
> >> Perhaps someone who can build on macOS directly can provide more info about the problem. I had no problem building with clang 11.0 on Ubuntu but Apple's clang is special.
> >
> > Apple's clang seems to come with a builtin
> >
> > #define __nonnull _Nonnull
> >
> > which clashes with the the one in lib/cdefs.h. Perhaps we should call it something else.
>
> Thanks for the diagnosis. Although calling it something else would
> obviously work, it would require renaming __nonnull everywhere within
> Gnulib (and eventually Glibc), and that would be entirely contrary to
> the intent of the Apple macro - which was to encourage compatibility
> with GNU code. As I understand things Apple's __nonnull keyword used to
> mean something else but collided with GNU's __nonnull macro, so they
> renamed the keyword and put in a backward-compatibility macro for old
> Applish code, but we don't need that macro and can just use __nonnull
> for our own use.
>
> I attempted to fix the problem by installing the attached patch into
> Gnulib, and then merged Gnulib into Emacs master. Please give it a try,
> as I don't use macOS and so did not check the patch directly.

Thanks, it compiles fine for me now.