[cfe-commits] r56843 - in /cfe/trunk: include/clang/Basic/DiagnosticKinds.def lib/Sema/Sema.h lib/Sema/SemaDeclObjC.cpp lib/Sema/SemaExprObjC.cpp test/SemaObjC/method-lookup-3.m

[steve naroff]

On Sep 30, 2008, at 4:33 PM, Daniel Dunbar wrote:

> Hi Steve,
>
> Thanks for fixing this. Now that I have been using it for a little  
> while I
> have noticed a few inconsistencies that raise some questions.
>
> On Tue, Sep 30, 2008 at 7:38 AM, Steve Naroff <snaroff at apple.com>  
> wrote:
>> +  if (MethList.Method && MethList.Next) {
>> +    Diag(R.getBegin(), diag::warn_multiple_method_decl,  
>> Sel.getName(), R);
>> +    Diag(MethList.Method->getLocStart(), diag::warn_using_decl,
>> +         MethList.Method->getSourceRange());
>> +    for (ObjCMethodList *Next = MethList.Next; Next; Next = Next- 
>> >Next)
>> +      Diag(Next->Method->getLocStart(), diag::warn_also_found_decl,
>> +           Next->Method->getSourceRange());
>> +  }
>
> One problem here is that the 2nd and subsequent locations may be  
> inside of
> system headers. This leads to confusing error messages since they  
> end up
> getting suppressed and it isn't possible to tell the source of the  
> conflict. I'm
> wondering if there is an easy fix for this, or if we should try to
> solve the "multipart"
> error message problem more generally.
>

 From my perspective, it would be useful to override the suppressed  
warning for a multipart diagnostic that originates from the user's  
source code. As an alternative, we could "pretty print" the method  
prototype (like GCC), however this wouldn't be consistent with other  
clang diags. This type of verbose warning is best handled in an IDE.  
The good news is most modern ObjC code avoids liberal use of  
"id" (since method resolution for dynamically typed objects is a bit  
adhoc).

This should be easy to support (though I haven't done any  
investigation).

> This also depends on whether we should suppress this warning for
> conflict arising
> from system headers. The right behavior may depend on whether the  
> signature
> we end up using comes from a system header or not.
>
>
>> +    // FIXME: consider using LookupInstanceMethodInGlobalPool,  
>> since it will be
>> +    // faster than the following method (which can do *many*  
>> linear searches).
>> +    // The idea is to add class info to InstanceMethodPool.
>
> I think we need to do this. Consider the following code
> --
> @interface A
> +(void) foo;
> @end
> @interface B
> +(int) foo;
> @end
>
> void f0(id x) {
>  int i = [x foo];
> }
> --
> which clang currently rejects (gcc emits the multiple methods  
> warnings).
>

Please file a separate radar for this.

> Finally, it appears the gcc is promiscuously accepting method  
> conflicts for
> certain types. For example:
> --
> typedef struct s0 T0;
> typedef struct s1 T1;
>
> @interface A
> +(T0*) foo;
> @end
> @interface B
> +(T1*) foo;
> @end
>
> void f0(id x) {
>  void *bar = [x foo];
> }
> --
>

Please file a separate radar for this as well.

I knew fixing this bug would raise some issues...thanks for the follow- 
through!

snaroff

> Let me know if you would prefer me to file bugs for any of these.
>
> - Daniel