aaron.ballman added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:39
+ if (MatchedCallExpr &&
+ MatchedCallExpr->hasUnusedResultAttr(*Result.Context)) {
+ diag(MatchedCallExpr->getExprLoc(), "Unused result from error function %0")
----------------
Is the presence of the attribute actually important, or is it simply expected
that the declarations will have the attribute and so this check is benign?
================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:40
+ MatchedCallExpr->hasUnusedResultAttr(*Result.Context)) {
+ diag(MatchedCallExpr->getExprLoc(), "Unused result from error function %0")
+ << MatchedCallExpr->getDirectCallee()->getNameAsString();
----------------
clang-tidy diagnostics are nongrammatical in the same way the compiler error
messages are: don't capitalize the start of the sentence, no terminating
punctuation, etc. I'd probably reword to `result from error function %0 is
unused`
================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:41
+ diag(MatchedCallExpr->getExprLoc(), "Unused result from error function %0")
+ << MatchedCallExpr->getDirectCallee()->getNameAsString();
+ }
----------------
You should pass the result from `getDirectCallee()` rather than converting to a
string. The diagnostics engine can handle any `NamedDecl *` automatically,
including proper quoting, etc.
================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:48
+ diag(MatchedTransitiveCallExpr->getExprLoc(),
+ "Unused result from function %0, which returns an error value")
+ << MatchedTransitiveCallExpr->getDirectCallee()->getNameAsString();
----------------
Similar comments here. How about `result from function %0 is unused but
represents an error value`?
================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:49
+ "Unused result from function %0, which returns an error value")
+ << MatchedTransitiveCallExpr->getDirectCallee()->getNameAsString();
+ }
----------------
Similar issue here as above.
================
Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:51
+ }
+}
+
----------------
Is there a way for the user to explicitly disable this warning, or is there no
such thing as a false positive as far as the kernel is concerned?
Should the diagnostic also produce a fix-it, or is there not a sufficiently
common replacement pattern to be used?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59963/new/
https://reviews.llvm.org/D59963
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
