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

[Philipp Stephani <p.stephani2@gmail.com>]

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

Paul Eggert <eggert@cs.ucla.edu> 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).

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