On Oct 27, 2013, at 6:55 , Richard <[email protected]> wrote:

> 
> On 25 Oct 2013, at 18:51, Jordan Rose <[email protected]> wrote:
> 
>>>> 
>>>> +        const ObjCSelectorExpr *SelExpr = 
>>>> cast<ObjCSelectorExpr>(ME->getArg(0));
>>>> 
>>>> This could fail if the user passes a SEL variable.
>>>> 
>>> 
>>> Well spotted. I have added a helper method to extract the ObjCSelectorExpr 
>>> in the case of a SEL variable, it is very ugly code though, is there an 
>>> easier way to do this? see selectorForArgument().
>> 
>> The way you've done it will break for re-assignments, so that won't work. I 
>> think the right thing to do is just ignore that case for now; in the long 
>> term, I think the correct solution is to model SEL regions in the same sort 
>> of way as we do string literals.
>> 
> 
> OK, I removed the code and replaced it with a TODO for now.
> 
>> 
>> I'm still concerned about the diagnostic text:
>> 
>> +  os << "Calling method introduced in ";
>> +  os << Introduced;
>> +  os << " (deployment target is ";
>> +  os << deploymentTarget(State);
>> +  os << ")";
>> 
>> At the very least we need to distinguish "method" and "function"; for bonus 
>> points, saying that the entire class or protocol was introduced in version X 
>> would be nice polish.
>> 
>> Jordan
> 
> How about the attached patch? It prints the class or protocol version when 
> the method has no specific introduced version.

The logic all seems correct here, but I think we can still do better on the 
message. The class and protocol cases seem a little underinformative to me, 
especially since the analyzer doesn't have a note where the availability shows 
up. So instead of a version kind, how about just passing the decl that has the 
availability attribute? That way we can say "method", "function", "class 
'Foo'", or "protocol 'Foo'".

Also, it would be nice to include the platform name with the version. This code 
is stolen from DeclBase.cpp, but I don't have a better place for it right now.

  StringRef TargetPlatform = Context.getTargetInfo().getPlatformName();
  StringRef PrettyPlatformName
    = AvailabilityAttr::getPrettyPlatformName(TargetPlatform);
  if (PrettyPlatformName.empty())
    PrettyPlatformName = TargetPlatform;

"Calling method introduced in OS X 10.9 (deployment target is 10.8)"

But other than this polish this is looking very good. Have you tried running it 
on existing projects?
Jordan

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to