On Thu, 17 Aug 2017, Martin Sebor wrote:
> +/* Check LAST_DECL and NODE of the same symbol for attributes that are
> + recorded in EXCL to be mutually exclusive with ATTRNAME, diagnose
> + them, and return true if any have been found. NODE can be a DECL
> + or a TYPE. */
> +
> +static bool
> +diag_attr_exclusions (tree last_decl, tree node, tree attrname,
> + const attribute_spec *spec)
EXCL is not an argument to this function, so the comment above it should
not refer to EXCL (presumably it should refer to SPEC instead).
> + note &= warning (OPT_Wattributes,
> + "ignoring attribute %qE in declaration of "
> + "a built-in function qD because it conflicts "
> + "with attribute %qs",
> + attrname, node, excl->name);
%qD not qD, presumably.
(Generically, warning_at would be preferred to warning, but that may best
be kept separate if you don't already have a location available here.)
> +static const struct attribute_spec::exclusions attr_gnu_inline_exclusions[] =
> +{
> + ATTR_EXCL ("gnu_inline", true, true, true),
> + ATTR_EXCL ("noinline", true, true, true),
> + ATTR_EXCL (NULL, false, false, false),
> +};
This says gnu_inline is incompatible with noinline, and is listed as the
EXCL field for the gnu_inline attribute.
> +static const struct attribute_spec::exclusions attr_inline_exclusions[] =
> +{
> + ATTR_EXCL ("always_inline", true, true, true),
> + ATTR_EXCL ("noinline", true, true, true),
> + ATTR_EXCL (NULL, false, false, false),
> +};
This is listed as the EXCL field for the noinline attribute, but does not
mention gnu_inline. Does this mean some asymmetry in when that pair is
diagnosed? I don't see tests for that pair added by the patch.
(Of course, gnu_inline + always_inline is OK, and attr_inline_exclusions
is also used for the always_inline attribute in this patch.)
In general, the data structures where you need to ensure manually that if
attribute A is listed in EXCL for B, then attribute B is also listed in
EXCL for A, seem concerning. I'd expect either data structures that make
such asymmetry impossible, or a self-test that verifies that the tables in
use are in fact symmetric (unless there is some reason the symmetry is not
in fact required and symmetric diagnostics still result from asymmetric
tables - in which case the various combinations and orderings of
gnu_inline and noinline definitely need tests to show that the diagnostics
work).
> +both the @code{const} and the @code{pure} attribute is diagnnosed.
s/diagnnosed/diagnosed/
--
Joseph S. Myers
[email protected]
