dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land.
Aah, MIG_NO_REPLY. LGTM. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:42 + std::vector<std::pair<CallDescription, unsigned>> Deallocators = { +#define CALL(required_args, deallocated_arg, ...) \ + {{{__VA_ARGS__}, required_args}, deallocated_arg} ---------------- Could you put a comment with an example indicating how CALL works for a particular example function? I think that will make is easier for folks to add support for new APIs in the future without getting it wrong. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:125 + // See if there's an annotated method in the superclass. + if (const auto *MD = dyn_cast<CXXMethodDecl>(D)) ---------------- Perhaps we could make it a Sema error if a method doesn't have a mig server routine annotation but it overrides a method that does. From a documentation/usability perspective it would be unfortunate if a human looking at a method to determine its convention would have to look at its super classes to see whether they have an annotation too. Although, thinking about it more that might make it a source-breaking change if an API author were to add annotation on a super class. Then API clients would have to add the annotations to get their projects to build, which would not be great.. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:162 +// Returns true if V can potentially represent a "successful" kern_return_t. +static bool isSuccess(SVal V, CheckerContext &C) { + ProgramStateRef State = C.getState(); ---------------- Could we rename this to "mayBeSuccess()" or something like that so that the name of function indicate the "potentially" part of the comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58397/new/ https://reviews.llvm.org/D58397 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits