xazax.hun added inline comments.
================ Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:72 +REGISTER_MAP_WITH_PROGRAMSTATE(CtorMap, const MemRegion *, bool) +REGISTER_MAP_WITH_PROGRAMSTATE(DtorMap, const MemRegion *, bool) + ---------------- I was wondering if there is another way to represent the state. We could have a two element (bool based) enum class like: ``` enum class ObjectState : bool { CtorCalled, DtorCalled }; ``` Se we could have only one map in the GDM. Either the destructor is called for an object or the constructor. Or in case none of them is called on a path, the state is empty. What do you think? ================ Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:86 + const LocationContext *LCtx = N->getLocationContext(); + ProgramStateManager &PSM = State->getStateManager(); + auto &SVB = PSM.getSValBuilder(); ---------------- You could move the declarations of PSM and SVB after the next early return. ================ Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:107 + + std::string DeclName; + std::string InfoText; ---------------- You could eliminate this local variable. ================ Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:169 - // FIXME: The interprocedural diagnostic experience here is not good. - // Ultimately this checker should be re-written to be path sensitive. - // For now, only diagnose intraprocedurally, by default. - if (IsInterprocedural) { - os << "Call Path : "; - // Name of current visiting CallExpr. - os << *CE->getDirectCallee(); - - // Name of the CallExpr whose body is current being walked. - if (visitingCallExpr) - os << " <-- " << *visitingCallExpr->getDirectCallee(); - // Names of FunctionDecls in worklist with state PostVisited. - for (SmallVectorImpl<const CallExpr *>::iterator I = WList.end(), - E = WList.begin(); I != E; --I) { - const FunctionDecl *FD = (*(I-1))->getDirectCallee(); - assert(FD); - if (VisitedFunctions[FD] == PostVisited) - os << " <-- " << *FD; - } + if (IsPureOnly && !MD->isPure()) + return; ---------------- These two checks could be moved before you query the CXXThisVal. ================ Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:247 + C.emitReport(std::move(Reporter)); + return; } ---------------- This return is redundant. https://reviews.llvm.org/D34275 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits