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

Rémi Denis-Courmont remi at remlab.net
Mon Feb 27 18:00:15 CET 2017


Le sunnuntaina 26. helmikuuta 2017, 23.32.06 EET Filip Roséen a écrit :
> 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,

Believe what you will. But on the one hand, we have been doing that for years 
with few problems. The only difficulty is finding the correct compiler 
versions, and even that is mostly a matter of time reading documentation.

We have automated tests catching most errors, and the policy is simple and 
clear-cut so it has proven easy to enforce in review (post- or pre-commit).

On the other hand, we used to have configure-dependent headers, and that did 
cause problems, and turned out a massive pain to fix:

> 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).

First, that is missing.c all over again. People keep forgetting to update it, 
or don´t sufficiently validate their updates. And missing.c at least gets some 
coverage from the mobile ports; that wouldn´t.

And then we penalize out-of-tree plugins. They would lose compile-time 
diagnostics, such that the ever so useful format string warnings. To avoid, 
this, you would have to do both #1 and #2, resulting in the all disadvantages 
of both options, and none of the advantages.

But the worst part is that we could very easily end up with silently 
mismatched binary interfaces for the out-of-tree plugins, if we used separate 
definitions.

>  - 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:

Sorry but this looks to me like a solution looking for a problem to solve.

-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/



More information about the vlc-devel mailing list