On Oct 24, 2013, at 1:19 , Richard <[email protected]> wrote:
> On 22 Oct 2013, at 18:43, Jordan Rose <[email protected]> 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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
