emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5


From: Philipp Stephani
Subject: Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5
Date: Thu, 15 Jun 2017 12:54:12 +0000



Paul Eggert <address@hidden> 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). 

reply via email to

[Prev in Thread] Current Thread [Next in Thread]