On 2/11/19 1:23 PM, Jakub Jelinek wrote:
On Mon, Feb 11, 2019 at 12:20:42PM -0700, Martin Sebor wrote:
--- gcc/c-family/c-attribs.c (revision 268774)
+++ gcc/c-family/c-attribs.c (working copy)
@@ -4032,8 +4032,12 @@ validate_attribute (location_t atloc, tree oper, t
if (TYPE_P (oper))
tmpdecl = build_decl (atloc, TYPE_DECL, tmpid, oper);
+ else if (DECL_P (oper))
+ tmpdecl = build_decl (atloc, TREE_CODE (oper), tmpid, TREE_TYPE (oper));
+ else if (EXPR_P (oper))
+ tmpdecl = build_decl (atloc, TYPE_DECL, tmpid, TREE_TYPE (oper));
else
- tmpdecl = build_decl (atloc, TREE_CODE (oper), tmpid, TREE_TYPE (oper));
+ return false;
/* Temporarily clear CURRENT_FUNCTION_DECL to make decl_attributes
believe the DECL declared above is at file scope. (See bug 87526.) */
Why do you this kind of validation at all? You do compare the arguments
later on in the caller, why isn't that sufficient? Creating some decl (and
ignoring for that whether the attribute is decl_required, type_required and/or
function_type_required) is a sure way to get many warnings, even if the
arguments are reasonable.
The snippet above deals with validating an attribute with one or
more arguments in the context where it's being queried, to make
sure the whole attribute specification makes any sense at all.
It's meant to catch gross mistakes like
__builtin_has_attribute (func, alloc_size ("1"));
but it's far from perfect.
The difference in the example you asked about in your other mail
can be seen here:
const int i = 1;
enum {
e = __builtin_has_attribute (1 + 0, alloc_size ("foo")),
f = __builtin_has_attribute (i + 1, alloc_size ("bar"))
};
Because the EPPR_P(oper) test fails for the first enumerator
the first invalid attribute specification is not diagnosed. But
because it succeeds for (i + 1), the second specification triggers
a warning about alloc_size being only applicable to function types.
I suppose this could improved by handling constants the same as
expressions.
But this is far from the only limitation. Attributes with no
arguments don't even make it this far. Because the validation
depends on decl_attributes to detect these mistakes and that
function doesn't distinguish success from failure, the validation
succeeds even for non-sensical attributes. But it should never
cause the built-in to give a false positive. There are at least
a couple of FIXMEs in the code where I would like to improve
the validation in GCC 10. But none of them is inherent in
the design of the built-in or serious enough to seriously
compromise its usefulness.
Martin
