[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

LGTM.
Any ideas for improvement, @JonasToth?
I'd rather have it merged and improve on it later if there are ideas on how to do it better.



================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:32
+  // Try to catch both std::function and boost::function
+  return (ParamType.getAsString().find("::function") != std::string::npos);
+}
----------------
I see that you check for any `::function` but you only test `std::function`. Could you add a test case for `boost::function` or any other `xyz::function` please?


================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:127
+  auto &Context = *Result.Context;
+  // check for the existance of the keyword being used as the ``[[nodiscard]]``
+  if (!doesNoDiscardMacroExist(Context, NoDiscardMacro)) {
----------------
existance -> existence


================
Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:231
+
+// don't mark typical ``[[nodisscard]]`` candidates if the class
+// has mutable member variables
----------------
Single 's' in `[[nodiscard]]`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55433/new/

https://reviews.llvm.org/D55433