[vlc-devel] [PATCH 0/6] Fix warnings related to unknown attributes by introducing VLC_ATTR

[Filip Roséen]

Hi `vlc-devel`,

As per the information posted by *Rémi Denis-Courmount* in the below
linked message, relying solely on information available during
*configure* is not sufficient as *VLC*-headers might be used by
entities differently than that used during that stage.

 - https://mailman.videolan.org/pipermail/vlc-devel/2017-February/111752.html

With the above in mind I see it as we have two approaches to solving
issues related to attribute uses using compilers that may not support
them (and where the attribute itself is not mandatory for compilation,
but certainly helpful if available).

 1. Maintain a `#if`-tree that properly defines helper macros for each
    attribute used within the codebase, and conditionally defines
    these to either that which is supported by the relevant compiler -
    or empty (in order to prevent unsupported attributes from
    resulting in diagnostics).
    
 2. Use the information available from *configure* **if** available
    (i.e. when we are building VLC), otherwise fall back to a more
    easily maintainable list of macros where we do not care about the
    exact compiler being used. Instead we would maintain in a way that
    corresponds to the current implementation. `#ifdef __GNUC__` would
    unconditionally define the macros to what the *"compiler of our
    dream"* would implement.
    
As I do not believe alternative `#1` will be easy to maintain, I would
go with alternative `#2`. Below is a link to a branch where such is
implemented (based on the patch-batch in which this email is a reply).

 - https://github.com/FilipRoseen-refp/vlc/commits/0000_attributes

Where the relevant commit with changes compared to the proposed
implementation is available in the following commit:

 - https://github.com/FilipRoseen-refp/vlc/commit/68ad551238b0b70de1c48bd977ac37dfe5de5f28

Best Regards,\
Filip

On 2017-02-26 20:13, Filip Roséen wrote:

> After the more than welcomed commit linked below we currently
> experience tons of diagnostics related to unknown attributes when
> things are compiled with clang.
> 
>  - http://git.videolan.org/?p=vlc.git;a=commit;h=97d669a8c704a19f9bd4cd4b8f995dcc9e9edbb7
> 
> Example:
> 
>     include/vlc_threads.h:837:16: warning: unknown attribute 'error' ignored [-Wunknown-attributes]
>     __attribute__((error("sorry, cannot sleep for such short a time")))
>                    ^
>     include/vlc_threads.h:847:16: warning: unknown attribute 'warning' ignored [-Wunknown-attributes]
>     __attribute__((warning("use proper event handling instead of short delay")))
>                    ^
> Rationale for changes
> ----------------------------------------------------------------------
> 
> As attribute usage is encouraged in cases where it make sense, I was
> reluctant to wrap usage of the affected attributes in a preprocessor
> condition checking if we are being compiled by clang, and what version
> of clang.
> 
> In my opinion that would lead to unnecessary complexity and harder
> maintainability, especially when things differ version to version.
> 
> Instead these changes include addition of support for checking
> supported attributes during configure, as well as a macro named
> `VLC_ATTR` that is used to apply the checked attributes (if they are
> supported by the compiler, otherwise it expands to an empty string).
> 
> Future
> ----------------------------------------------------------------------
> 
> I currently have patches that does what is listed below, but will
> submit them at a later time if the contents of this patch-batch is
> satisfactory:
> 
>  - replace usage of VLC_DEPRECATED with VLC_ATTR(deprecated)
> 
>  - add support for more attributes that can be used to further
>    strengthen the codebase (where applicable)
> 
>  - some further attribute related clean-up
> 
> Disclaimer
> ----------------------------------------------------------------------
> 
> I took about an hour to read up on `.m4` and `autoconf` prior to
> writing that part of the changes; besides reading patches to this
> codebase, that hour is my **only** experience with the toolchain.
> 
> Please go easy on me if there is something that should have been done
> differently.
> 
> Best Regards,\
> 
> Filip Roséen (6):
>   m4: add attributes.m4
>   configure: add check for attributes
>   configure: use attributes.m4 to check for attribute packed
>   configure: use attributes.m4 to check for attribute aligned
>   vlc_common: introduce macro VLC_ATTR
>   vlc_threads: use VLC_ATTR to apply attributes
> 
>  configure.ac          | 25 ++++++++++---------------
>  include/vlc_common.h  | 33 +++++++++++++++++++++++++++++++++
>  include/vlc_threads.h | 20 +++++++++++---------
>  m4/Makefile.am        |  1 +
>  m4/attributes.m4      | 41 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 96 insertions(+), 24 deletions(-)
>  create mode 100644 m4/attributes.m4
> 
> -- 
> 2.11.1
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170226/eedeb0c6/attachment.html>