Two patches attached again: part1 no behavior change, part2 as before.

> Actually, this whole function looks like it's just checking for a
> recursive path.  You could change it to return `bool`, and rename it
> something like `hasRecursiveCall()`, and leave the state-related logic
> out of the helper.  Maybe better in a follow-up patch?

I went ahead and took care of this in the first patch, since it cleans things 
up nicely.

> I think this would be clearer like this:
> 
>    const CXXMemberCallExpr *MCE = dyn_cast<CXXMemberCallExpr>(CE);
>    if (!MCE || isa<CXXThisExpr>(MCE->getImplicitObjectArgument()) ||
>        !MCE->getMethodDecl()->isVirtual())
>      return FoundPath;

Yup, done.

> In either case, this could be merged with the early exit:
> 
>    if (State == FoundPathWithNoRecursiveCall)
>      if (ExitID == ID)
>        return;
>      if (hasRecusiveCallInPath(FD, Block)
>        State = FoundPath;
>    }

Done.

Attachment: checkForFunctionCall-part1.patch
Description: Binary data

Attachment: checkForFunctionCall-part2.patch
Description: Binary data

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

Reply via email to