r195533 - Add back experimental attribute objc_suppress_protocol_methods (slightly renamed).

[Aaron Ballman]

On Wed, Nov 27, 2013 at 2:02 AM, Ted Kremenek <kremenek at apple.com> wrote:
>> On Nov 24, 2013, at 8:00 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> I thought id was something more special than just "any identifier?"  Eg)
>>
>> int i;
>> __attribute__((objc_suppress_protocol_methods(i)))
>> @interface foo
>> @end
>>
>> There would be no error fired here, but based on the error text, it
>> seems like there should be one.
>>
>
> This is valid.  Objective-C protocols are in their own namespace, and this
> attribute is meant to work regardless of whether or not the protocol is
> visible within the translation unit.  Thus this case is valid.  The ‘i’ here
> for the attribute parameter has nothing to do with ‘int i’.

I think we're talking past one another though. The diagnostic you have
will be issued when there is no identifier present (for instance,
you're given an expression as the sole argument). The diagnostic
issued says:

parameter of 'objc_suppress_protocol_methods' attribute must be a
single name of an Objective-C protocol

Except you're not checking whether what you receive is a single name
of an Objective-C protocol. Any identifier will do, such as passing
int i; That seems confusing to me -- I would assume that if you
perform the lookup and *know* what you have it not a protocol, then
you would issue the error. If you perform the lookup and don't find
anything, I can understand suppressing the diagnostic in that case.
Does that make more sense?

> This is what I have in Attr.td:
>
> def ObjCSuppressProtocol : InheritableAttr {
>   let Spellings = [GNU<"objc_suppress_protocol_methods">];
>   let Subjects = [ObjCInterface];
>   let Args = [IdentifierArgument<"Protocol">];
> }
>
> and this is what we have in the test case:
>
>   __attribute__((objc_suppress_protocol_methods(ProtoA, ProtoB))) //
> expected-error {{use of undeclared identifier 'ProtoB'}}
>
> Am I missing something?  From what I can tell the optional bit has been
> removed, and yet the error remains as it is.  For this test case, it is
> intentional that ProtoB is not declared; that’s perfectly fine.  The problem
> is that the attribute takes one argument.

Ah, this is an error generated from parsing, not sema. As we're
parsing the args in ParseGNUAttributeArgs, we're calling
ParseAssignmentExpression on ProtoB, and it's not being found as a
primary expression, so it generates the diagnostic. A better test in
this case would be to supply an identifier that can be successfully
looked up. Sorry for the confusion!

~Aaron