[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Aaron Ballman via Phabricator via cfe-commits]

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