Thanks Jordan!
On Jul 19, 2012, at 5:21 PM, Jordan Rose wrote:
> It came out very nicely!
>
> A few small comments...
>
> On Jul 19, 2012, at 4:38 PM, Anna Zaks wrote:
>
>> +
>> + /// \bried Default implementation of call evaluation.
>> + void defaultEvalCall(NodeBuilder &B, ExplodedNode *Pred,
>> const CallEvent &Call);
>
> Typo: "bried".
>
thanks
>
>> +ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
>> + const LocationContext *LCtx,
>> + ProgramStateRef State) {
>> + const Expr *E = Call.getOriginExpr();
>> + if (!E)
>> + return State;
>> +
>> + // Some method families have known return values.
>> + if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(&Call)) {
>> + switch (Msg->getMethodFamily()) {
>> + default:
>> + break;
>> + case OMF_autorelease:
>> + case OMF_retain:
>> + case OMF_self: {
>> + // These methods return their receivers.
>> + return State->BindExpr(E, LCtx, Msg->getReceiverSVal());
>> + break;
>> + }
>> + }
>> + }
>
> The inner braces are not necessary anymore here. Also, you have a break after
> a return.
I'd like to keep the braces of the if since the switch is visually long (do we
have a coding standard for unnecessary braces?).
>
>
>> +void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
>> const CallEvent &Call) {
>> + ProgramStateRef State = 0;
>> + const Expr *E = Call.getOriginExpr();
>> +
>> // Try to inline the call.
>> // The origin expression here is just used as a kind of checksum;
>> // for CallEvents that do not have origin expressions, this should still be
>> // safe.
>> - const Expr *E = Call.getOriginExpr();
>> - ProgramStateRef state = getInlineFailedState(Pred, E);
>> - if (state == 0 && inlineCall(Dst, Call, Pred))
>> - return;
>> + if (!isa<ObjCMethodCall>(Call)) {
>> + State = getInlineFailedState(Pred->getState(), E);
>> + if (State == 0 && inlineCall(Call, Pred)) {
>> + // If we inlined the call, the successor has been manually added onto
>> + // the work list and we should not consider it for subsequent call
>> + // handling steps.
>> + Bldr.takeNodes(Pred);
>> + return;
>> + }
>> + }
>>
>
> I would just take out the check for isa<ObjCMethodCall>. The check for
> "dynamic dispatch" inside inlineCall should catch this, and
> getInlineFailedState isn't too expensive.
>
Yes, this is WIP. Plan is to remove it after I figure out if inlineCall can
handle this (which is probably does according to your comment).
>
>
>> @@ -182,13 +180,19 @@
>>
>> // Check if the "raise" message was sent.
>> assert(notNilState);
>> - if (msg.getSelector() == RaiseSel)
>> - RaisesException = true;
>> + if (msg.getSelector() == RaiseSel) {
>> + // If we raise an exception, for now treat it as a sink.
>> + // Eventually we will want to handle exceptions properly.
>> + Bldr.generateNode(currentStmt, Pred, Pred->getState(), true);
>> + continue;
>> + }
>>
>> - // If we raise an exception, for now treat it as a sink.
>> - // Eventually we will want to handle exceptions properly.
>> - // Dispatch to plug-in transfer function.
>> - evalObjCMessage(Bldr, msg, Pred, notNilState, RaisesException);
>> + // Generate a transition to non-Nil state.
>> + if (notNilState != state)
>> + Pred = Bldr.generateNode(currentStmt, Pred, notNilState);
>> +
>> + // Evaluate the call.
>> + defaultEvalCall(Bldr, Pred, msg);
>> }
>> } else {
>> // Check for special class methods.
>> @@ -222,19 +226,25 @@
>> }
>>
>> Selector S = msg.getSelector();
>> + bool RaisesException = false;
>> for (unsigned i = 0; i < NUM_RAISE_SELECTORS; ++i) {
>> if (S == NSExceptionInstanceRaiseSelectors[i]) {
>> RaisesException = true;
>> break;
>> }
>> }
>> + if (RaisesException) {
>> + // If we raise an exception, for now treat it as a sink.
>> + // Eventually we will want to handle exceptions properly.
>> + Bldr.generateNode(currentStmt, Pred, Pred->getState(), true);
>> + continue;
>> + }
>> +
>> }
>> }
>>
>> - // If we raise an exception, for now treat it as a sink.
>> - // Eventually we will want to handle exceptions properly.
>> - // Dispatch to plug-in transfer function.
>> - evalObjCMessage(Bldr, msg, Pred, Pred->getState(), RaisesException);
>> + // Evaluate the call.
>> + defaultEvalCall(Bldr, Pred, msg);
>> }
>> }
>
> You can probably merge some pieces of the two branches now (the class and
> instance method branches).
I'll move out the call to defaultEvalCall, bit not much else is easily movable.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
