attribute handler oddness in MEP and STORMY16 ports

While going through the attribute tables to sort out separation or trees 
and types, I'm seeing a couple of ports with similar handlers that look 
a little odd.

the code sequence in the handler looks like:

if (TREE_CODE (*node) != VAR_DECL
       && TREE_CODE (*node) != POINTER_TYPE
       && TREE_CODE (*node) != TYPE_DECL)
     {
       warning (OPT_Wattributes,
                "%<__BELOW100__%> attribute only applies to variables");
       *no_add_attrs = true;
     }

My question is
a) what is POINTER_TYPE doing in there.. what has that got to do with 
applying to variables, and
b) should TYPE_DECL also be there?

I can almost see the argument for b).... MAYBE, but then the message 
doesn't really make sense.   but POINTER_TYPE seems really odd.

so ports that use that message:

avr, bfin, x86: handlers all make sure its *ONLY* a VAR_DECL handled
mep, stormy16 : uses the suspect sequence.  My guess is one copied the 
other.

In fact, the mep port does have a couple of other handlers that check 
for only VAR_DECL associated with that warning.

mep also has another handler for "variable and functions" that looks like:
if (TREE_CODE (*node) != VAR_DECL
       && TREE_CODE (*node) != FUNCTION_DECL
       && TREE_CODE (*node) != METHOD_TYPE
       && TREE_CODE (*node) != POINTER_TYPE
       && TREE_CODE (*node) != TYPE_DECL)
{
       warning (0, "%qE attribute only applies to variables and functions",
                name);
       *no_add = true;
     }

Most ports use VAR_DECL and FUNCTION_DECL for this warning.  Its odd 
that METHOD_TYPE is there, but not FUNCTION_TYPE if types were really 
handled...
And finally, also in mep :

   if (TREE_CODE (*node) != FUNCTION_DECL
       && TREE_CODE (*node) != METHOD_TYPE)
     {
       warning (0, "%qE attribute only applies to functions", name);
       *no_add = true;
     }

It doesn't seem that METHOD_TYPE should be there, unless FUNCTION_TYPE 
was also important.  It does have other handlers where the check is only 
for FUNCTION_DECL *OR*  FUNCTION_TYPE,  not this odd hybrid.   It's the 
only port that has this.

If the handlers are ONLY going to deal with decls, the attribute table 
ought to have the decl_required flag set as well, which the other ports 
do, but these 2 do not.. perhaps because of the existing _TYPE handling...?

Assuming I understand this right, I've attached a patch which covers mep 
and stormy16.  It corrects those tables and handlers to work the way I 
think they are suppose to.   They build, but I cant test them... can 
some with that ability run them and see if they are correct?

Assuming no one has a counter argument to what I'm saying... Maybe there 
is some need for this that I'm just missing :-)

Andrew

btw, a bit of followup...

There were a few other ports that had checks similar to that in their 
handlers.. ie in rs6000.c:rs6000_handle_longcall_attribute() and 
i386:ix86_handle_cconv_attribute()
if (TREE_CODE (*node) != FUNCTION_TYPE
        && TREE_CODE (*node) != FIELD_DECL
        && TREE_CODE (*node) != TYPE_DECL)
       {
         warning (OPT_Wattributes, "%qE attribute only applies to 
functions",
<....>

so perhaps it originated here or in one of the other ports that show 
this... The difference is that this attribute has the "type-required" 
bit set (unlike the mep and stormy16 ports). If you trace through 
decl_attributes() one finds that those 2 DECL cases are actually 
impossible to ever see.   if a decl is processed and the type_required 
bit is set, then the handler is actually called with  &TREE_TYPE (node), 
so if TYPE_DECL or FIELD_DECL were being used, the handler would only 
ever see whatever their actual type is...    I've removed those checks 
on my x86 box and have no failures.

Andrew


On 11/28/2014 10:46 AM, Andrew MacLeod wrote:
> While going through the attribute tables to sort out separation or 
> trees and types, I'm seeing a couple of ports with similar handlers 
> that look a little odd.
>
> the code sequence in the handler looks like:
>
> if (TREE_CODE (*node) != VAR_DECL
>       && TREE_CODE (*node) != POINTER_TYPE
>       && TREE_CODE (*node) != TYPE_DECL)
>     {
>       warning (OPT_Wattributes,
>                "%<__BELOW100__%> attribute only applies to variables");
>       *no_add_attrs = true;
>     }
>
> My question is
> a) what is POINTER_TYPE doing in there.. what has that got to do with 
> applying to variables, and
> b) should TYPE_DECL also be there?
>
> I can almost see the argument for b).... MAYBE, but then the message 
> doesn't really make sense.   but POINTER_TYPE seems really odd.
>
> so ports that use that message:
>
> avr, bfin, x86: handlers all make sure its *ONLY* a VAR_DECL handled
> mep, stormy16 : uses the suspect sequence.  My guess is one copied the 
> other.
>
> In fact, the mep port does have a couple of other handlers that check 
> for only VAR_DECL associated with that warning.
>
> mep also has another handler for "variable and functions" that looks 
> like:
> if (TREE_CODE (*node) != VAR_DECL
>       && TREE_CODE (*node) != FUNCTION_DECL
>       && TREE_CODE (*node) != METHOD_TYPE
>       && TREE_CODE (*node) != POINTER_TYPE
>       && TREE_CODE (*node) != TYPE_DECL)
> {
>       warning (0, "%qE attribute only applies to variables and 
> functions",
>                name);
>       *no_add = true;
>     }
>
> Most ports use VAR_DECL and FUNCTION_DECL for this warning.  Its odd 
> that METHOD_TYPE is there, but not FUNCTION_TYPE if types were really 
> handled...
> And finally, also in mep :
>
>   if (TREE_CODE (*node) != FUNCTION_DECL
>       && TREE_CODE (*node) != METHOD_TYPE)
>     {
>       warning (0, "%qE attribute only applies to functions", name);
>       *no_add = true;
>     }
>
> It doesn't seem that METHOD_TYPE should be there, unless FUNCTION_TYPE 
> was also important.  It does have other handlers where the check is 
> only for FUNCTION_DECL *OR*  FUNCTION_TYPE, not this odd hybrid.   
> It's the only port that has this.
>
> If the handlers are ONLY going to deal with decls, the attribute table 
> ought to have the decl_required flag set as well, which the other 
> ports do, but these 2 do not.. perhaps because of the existing _TYPE 
> handling...?
>
> Assuming I understand this right, I've attached a patch which covers 
> mep and stormy16.  It corrects those tables and handlers to work the 
> way I think they are suppose to.   They build, but I cant test them... 
> can some with that ability run them and see if they are correct?
>
> Assuming no one has a counter argument to what I'm saying... Maybe 
> there is some need for this that I'm just missing :-)
>
> Andrew

My memories of why I did MeP the way I did are... vague.  I recall it
had to do with getting the attributes to apply to C++ objects
correctly, since C++ objects tend to be "complicated" and gcc didn't
always pass me what I expected.

> think they are suppose to.   They build, but I cant test them... can 
> some with that ability run them and see if they are correct?

MeP and stormy16 are both cgen/sid simulators.