Unavailable method checker

[Jordan Rose]

On Oct 24, 2013, at 1:19 , Richard <tarka.t.otter at gmail.com> wrote:

> On 22 Oct 2013, at 18:43, Jordan Rose <jordan_rose at apple.com> wrote:
> 
>> It's also too bad that there's no highlight range here, but I guess the location is accurate enough.
> 
> Yes, I did originally have the highlight range, but I could not work out a way to get this from the ImplicitNullDerefEvent, so I removed it.

The ugly way to do it would be to crawl into the sink node's ProgramPoint, but I'm happy to add a Stmt* to the ImplicitNullDerefEvent that's allowed to be null. That can be in a later patch, though.


>> +      std::string SelStr = ME->getSelector().getAsString();
>> +      if (SelStr == "respondsToSelector:" ||
>> +          SelStr == "instancesRespondToSelector:") {
>> 
>> getAsString() is expensive because it constructs a new string on the heap. Better to check that it is a unary selector, then grab the first selector piece and check that.
>> 
> 
> Not sure I get you here, isn't only 'class' a unary selector? I changed this to breaking early if there is more than 1 arg, and using the selector name for the first slot only.

It looks like we're inconsistent about whether "class" is "unary" or "nullary"; the factory methods say "nullary", with "unary" being single-argument, but the predicate methods say "unary" means "non-keyword", i.e. "no arguments". I think the factory methods are right. Anyway, yes, your new code is what I meant.


>> 
>> +        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.


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131025/0899c76b/attachment.html>