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
