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

[Steve Lhomme]

On Sun, Feb 26, 2017 at 11:32 PM, Filip Roséen <filip at atch.se> wrote:
> 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).

(for some reason gmail removed the numbers when quoting)
As I've been compiling VLC with MSVC (winrt ARM port) for a while I
prefer a clean define for each. Sometimes MSVC syntax is a bit
different or the order different from GCC so it's probably easier to
handle this way.  So I vote for #1

As a side note, we already have some compiler format detection in configure.ac:
AC_C_CONST
AC_C_INLINE
AC_C_RESTRICT

> 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).
>
> 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
>
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel