From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Philipp Stephani Newsgroups: gmane.emacs.devel Subject: Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 Date: Thu, 15 Jun 2017 12:54:12 +0000 Message-ID: References: NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="001a11407a7a0df1fb0551ff2bde" X-Trace: blaine.gmane.org 1497531307 30248 195.159.176.226 (15 Jun 2017 12:55:07 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 15 Jun 2017 12:55:07 +0000 (UTC) To: Paul Eggert , Emacs developers Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Jun 15 14:55:03 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dLUIg-0007cj-By for ged-emacs-devel@m.gmane.org; Thu, 15 Jun 2017 14:55:02 +0200 Original-Received: from localhost ([::1]:53920 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLUIl-0002ak-QL for ged-emacs-devel@m.gmane.org; Thu, 15 Jun 2017 08:55:07 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:42270) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLUI6-0002ZY-RD for emacs-devel@gnu.org; Thu, 15 Jun 2017 08:54:28 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dLUI5-0005D5-E5 for emacs-devel@gnu.org; Thu, 15 Jun 2017 08:54:26 -0400 Original-Received: from mail-ot0-x236.google.com ([2607:f8b0:4003:c0f::236]:33612) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dLUI5-0005Cw-72 for emacs-devel@gnu.org; Thu, 15 Jun 2017 08:54:25 -0400 Original-Received: by mail-ot0-x236.google.com with SMTP id k4so9058241otd.0 for ; Thu, 15 Jun 2017 05:54:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=iO1zuqoOEPTWvh8g+3hy2CXfyo5HpO3+ULUCSAidPLM=; b=E1lLzHp5PgW9di42dOU8uW6SjYVJN7vf8fVtCYY6uRlwzGPB0RmY3j6Hbo1VUlICQy NYp+c3/l7vDRZG1BouSMqOZAFWiAZtpp9PdsUK6F04rxUXXWGyGZma3W7JwIEOOzf8ZX NnxyNA+PLT/2saudH58n3TQrBVcrTMQEhK8bH0CH8G5fjrMJ+huzlkL7f0ttQMJMuNv+ Vod/iOnHa28LpypHWFUUWpIOdmverCeyfQNxoA8oWVvJS2vWDzgrj/eWNmu1XCM+DbCz MxqHUtkLwhnkkggaQ6p3PSGvw8HMyIslFXns5UHe+0FMWrFb5Ddp0ttrox+G4QG3EG98 XA0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=iO1zuqoOEPTWvh8g+3hy2CXfyo5HpO3+ULUCSAidPLM=; b=lii9RUe7F2dAnigddTJhSq7su14qTV0Uzhig3wTESed1svQqFzqgMHASRuMTUat+Qr dRo1D0cWmE/3FtQ+qeXcVa5Bxa3z5ns7c21Iqaa0CEH5tFiiT/VU0EDfwWqMgQAKaxen RrJqP0HSzjEaKl+9LOnlDE60lvH6A8vGQtZvYhseBCDlMH9wMWSfRSYODGNo9ylyYXEB JKVNeG1fP1EdRNuR0dBHrRJqCJOjdbzhJCpP10MDB4+tb/nJ3dlGZ8deObAUAkbrDA4R v3JlcoDpJkBV8ysoakVu/guj4rs32GLHpEsHFsY/0O0IZwoXTdzj4xNdu5pP5H3nc14O kd9w== X-Gm-Message-State: AKS2vOzNgy1cJc+0vd4OlyXrSYVAbM8va2MS1h31rIX/bh1xJcDWwxp1 o5e0qAJQN6hZbo7ayYpWT8eu425UFw3W X-Received: by 10.157.16.8 with SMTP id h8mr3306308ote.80.1497531264394; Thu, 15 Jun 2017 05:54:24 -0700 (PDT) In-Reply-To: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4003:c0f::236 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:215637 Archived-At: --001a11407a7a0df1fb0551ff2bde Content-Type: text/plain; charset="UTF-8" Paul Eggert schrieb am Mi., 14. Juni 2017 um 20:26 Uhr: > On 06/14/2017 03:29 AM, Philipp Stephani wrote: > > > > the commit message says "The C compiler should check the cleanup > > attribute in the next line anyway." But that's not the case: unknown > > attributes are silently ignored, at least in Clang. The > > verify(__has_attribute(cleanup)) or equivalent is absolutely required > > here. Can we revert that commit? > > No, because the 'verify' breaks the build with Oracle Studio 12.5, where > __has_attribute works only inside preprocessor conditionals. I installed > the attached patch, which checks for __attribute__ (cleanup) in a > different way. Thanks. > But while we're on the subject, wouldn't it be better if > emacs-module.c were made to work (albeit perhaps less efficiently) even > on compilers that do not support this nonstandard C extension? > I've considered a couple of options. I think the simplest and most portable one would be to compile as C++, which has destructors built into the language. If that's not possible for whatever reason, we could introduce a macro that would create an entire wrapper function, which could implement the cleanup as normal C function. That would require some preprocessor metaprogramming, though, or more boilerplate. Alternatively we could inline most of the macros and require discipline from the developers editing that code. > > Another thing. Have you had a chance to think about related questions I > asked about recently installed portability changes? Here are the URLs: > Yes, sorry for the slow response, I'm working through these now. > > http://lists.gnu.org/archive/html/emacs-devel/2017-06/msg00225.html > > http://lists.gnu.org/archive/html/emacs-devel/2017-06/msg00226.html > > Since then I see that you installed another patch (commit > 32d8dba625fc50ccbe28e35afcf1f0529d611e00) to pacify Clang on macOS; this > patch unfortunately could cause trouble on non-POSIX platforms where > rlim_t is signed. Pacifying Clang shouldn't be at the cost of > portability or unnecessary complexity. > Agreed. I wasn't aware of such platforms. How about using #pragma clang diagnostic push/pop/ignore to ignore the warnings in the specific statements where they arise and we know that they are false positives? I'd much prefer that over disabling them globally in configure, because most of the time the warnings are useful. > > And I'm still puzzled as to why you're getting the Clang warnings but I > am not. Are you using an older Clang? Are you passing it extra warning > options? > > I'm using the Apple fork on macOS. It's mostly identical to upstream Clang and compiles Emacs just fine, but it is a fork and not 100% identical. I also get some of the warnings only when building with -O3 (haven't checked other optimization levels). --001a11407a7a0df1fb0551ff2bde Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


Paul E= ggert <eggert@cs.ucla.edu> = schrieb am Mi., 14. Juni 2017 um 20:26=C2=A0Uhr:
On 06/14/2017 03:29 AM, Philipp Stephani wrote:
>
> the commit message says "The C compiler should check the cleanup<= br> > attribute in the next line anyway." But that's not the case: = unknown
> attributes are silently ignored, at least in Clang. The
> verify(__has_attribute(cleanup)) or equivalent is absolutely required<= br> > here. Can we revert that commit?

No, because the 'verify' breaks the build with Oracle Studio 12.5, = where
__has_attribute works only inside preprocessor conditionals. I installed the attached patch, which checks for __attribute__ (cleanup) in a
different way.

Thanks.
=C2=A0
But while we're on the subject, would= n't it be better if
emacs-module.c were made to work (albeit perhaps less efficiently) even
on compilers that do not support this nonstandard C extension?

I've considered a couple of options. I think th= e simplest and most portable one would be to compile as C++, which has dest= ructors built into the language.
If that's not possible for w= hatever reason, we could introduce a macro that would create an entire wrap= per function, which could implement the cleanup as normal C function. That = would require some preprocessor metaprogramming, though, or more boilerplat= e.
Alternatively we could inline most of the macros and require d= iscipline from the developers editing that code.
=C2=A0

Another thing. Have you had a chance to think about related questions I
asked about recently installed portability changes? Here are the URLs:
<= /blockquote>

Yes, sorry for the slow response, I'm w= orking through these now.
=C2=A0

http://lists.gnu.org/archive/html= /emacs-devel/2017-06/msg00225.html

http://lists.gnu.org/archive/html= /emacs-devel/2017-06/msg00226.html

Since then I see that you installed another patch (commit
32d8dba625fc50ccbe28e35afcf1f0529d611e00) to pacify Clang on macOS; this patch unfortunately could cause trouble on non-POSIX platforms where
rlim_t is signed. Pacifying Clang shouldn't be at the cost of
portability or unnecessary complexity.

= Agreed. I wasn't aware of such platforms.
How about using #pr= agma clang diagnostic push/pop/ignore to ignore the warnings in the specifi= c statements where they arise and we know that they are false positives? I&= #39;d much prefer that over disabling them globally in configure, because m= ost of the time the warnings are useful.
=C2=A0

And I'm still puzzled as to why you're getting the Clang warnings b= ut I
am not. Are you using an older Clang? Are you passing it extra warning
options?


I'm using the Apple fork on macOS.= It's mostly identical to upstream Clang and compiles Emacs just fine, = but it is a fork and not 100% identical. I also get some of the warnings on= ly when building with -O3 (haven't checked other optimization levels).= =C2=A0
--001a11407a7a0df1fb0551ff2bde--