> On 2015-Jul-23, at 11:54, Vedant Kumar <v...@apple.com> wrote: > > 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. > > <checkForFunctionCall-part1.patch><checkForFunctionCall-part2.patch>
LGTM. I committed for you in r243038 and r243039. _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
