Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)

[Martin Sebor <msebor@gmail.com>]

The bulk of this patch touches what I think is considered
the middle-end (attribs.c) so let me include its maintainers,
Ian, Jeff, and Richard:

   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01087.html

The high-level description and rationale for the change are
here:

   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00602.html

Thanks
Martin

On 08/17/2017 10:03 AM, Martin Sebor wrote:
> Attached is an updated patch with just the minor editorial
> tweaks for the issues pointed out by Marek (stray comments
> and spaces), and with the C++ and libstdc++ bits removed
> and posted separately.  I also added some text to the manual
> to clarify the const/pure effects.
>
> I thought quite a bit more about the const/pure attributes we
> discussed and tried the approach of warning only on a const
> declaration after one with attribute pure has been used, but
> otherwise allowing and silently ignoring pure after const.
> In the end I decided against it, for a few reasons (most of
> which I already mentioned but just to summarize).
>
> First, there is the risk that someone will write code based
> on the pure declaration even if there's no intervening call
> to the function between it and the const one.  Code tends to
> be sloppy, and it's also not uncommon to declare the same
> function more than once, for whatever reason.  (The ssa-ccp-2.c
> test is an example of the latter.)
>
> Second, there are cases of attribute conflicts that GCC already
> points out that are much more benign in their effects (the ones
> I know about are always_inline/noinline and cold/hot).  In light
> of the risk above it seems only helpful to include const/pure in
> the same set.
>
> Third, I couldn't find another pair of attributes that GCC would
> deliberately handle this way (silently accept both but prefer one
> over the other), and introducing a special case just for these
> two seemed like a wart.
>
> Finally, compiling Binutils, GDB, Glkibc, and the Linux kernel
> with the enhanced warning didn't turn up any code that does this
> sort of thing, either intentionally or otherwise.
>
> With that, I've left the handling unchanged.
>
> I do still have the question whether diagnosing attribute
> conflicts under -Wattributes is right.  The manual implies
> that -Wattributes is for attributes in the wrong places or
> on the wrong entities, and that -Wignored-attributes should
> be expected instead when GCC decides to drop one for some
> reason.
>
> It is a little unfortunate that many -Wattributes warnings
> print just "attribute ignored" (and have done so for years).
> I think they should all be enhanced to also print why the
> attribute is ignored (e.g., "'packed' attribute on function
> declarations ignored/not valid/not supported" or something
> like that).  Those that ignore attributes that would
> otherwise be valid e.g., because they conflict with other
> specifiers of the same attribute but with a different
> operand might then be candidate for changing to
> -Wignored-attributes.
>
> Thanks
> Martin
>