<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">code{white-space: pre;}</style>
</head>
<body>
<p>Hi <code>vlc-devel</code>,</p>
<p>As per the information posted by <em>Rémi Denis-Courmount</em> in the below linked message, relying solely on information available during <em>configure</em> is not sufficient as <em>VLC</em>-headers might be used by entities differently than that used during that stage.</p>
<ul>
<li>https://mailman.videolan.org/pipermail/vlc-devel/2017-February/111752.html</li>
</ul>
<p>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).</p>
<ol style="list-style-type: decimal">
<li><p>Maintain a <code>#if</code>-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).</p></li>
<li><p>Use the information available from <em>configure</em> <strong>if</strong> 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. <code>#ifdef __GNUC__</code> would unconditionally define the macros to what the <em>“compiler of our dream”</em> would implement.</p></li>
</ol>
<p>As I do not believe alternative <code>#1</code> will be easy to maintain, I would go with alternative <code>#2</code>. Below is a link to a branch where such is implemented (based on the patch-batch in which this email is a reply).</p>
<ul>
<li>https://github.com/FilipRoseen-refp/vlc/commits/0000_attributes</li>
</ul>
<p>Where the relevant commit with changes compared to the proposed implementation is available in the following commit:</p>
<ul>
<li>https://github.com/FilipRoseen-refp/vlc/commit/68ad551238b0b70de1c48bd977ac37dfe5de5f28</li>
</ul>
<p>Best Regards,<br />
Filip</p>
<p>On 2017-02-26 20:13, Filip Roséen wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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
</code></pre>
</blockquote>
</body>
</html>