curdeius added a comment.
Some more minor remarks. I'll give this check a try to see how it behaves on
some of my projects.
I agree that a high rate of false positives is possible (and is a bit of
spoiler) but I wouldn't reject this IMO useful check because of that.
Anyway, everything looks pretty clean for me.
================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:45
+ //
+ for (const auto *Par : Node.parameters()) {
+ const Type *ParType = Par->getType().getTypePtr();
----------------
Wouldn't something along the lines:
```
const auto ¶meters = Node.parameters();
return std::any_of(parameters.cbegin(), parameters.cend(), [](const auto *Par) {
const Type *ParType = Par->getType().getTypePtr();
if (isNonConstReferenceType(Par->getType())) {
return true;
}
if (ParType->isPointerType()) {
return true;
}
return false;
});
```
be a little bit more expressive?
I would also refactor it differently:
```
const Type &ParType = Par->getType(); // not sure about Type
if (isNonConstReferenceType(ParType)) {
// ...
if (ParType.getTypePtr()->isPointerType()) { // is ParType.isPointerType()
possible?
// ...
```
but that's a matter of taste.
================
Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:12
+or non constant references, and non pointer arguments, and of which the
+member functions are themselve const, have not means of altering any state
+or passing values other than the return type, unless they are using mutable
----------------
themselve -> themselves, have not means -> have no means
================
Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:3
+// RUN: -config="{CheckOptions: [{key:
modernize-use-nodiscard.ReplacementString, value: 'NO_DISCARD'}]}" \
+// RUN: -- -std=c++17 \
+
----------------
I'm not sure what guidelines dictate, but I'd love to see another test file
with `-std=c++14` for instance (or whatever minimal version to necessary to use
`clang::WarnUnusedResult`, if any).
A very small test would be ok, you can also move some parts of this file into a
new one.
================
Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:63
+
+ // TODO fix CHECK-FIXES to handle "[[" "]]"
+ [[nodiscard]] bool f11() const;
----------------
You might want to get rid of this `TODO` note now, same for the one on the top
of the file.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55433/new/
https://reviews.llvm.org/D55433
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
