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

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:45
+  //
+  for (const auto *Par : Node.parameters()) {
+    const Type *ParType = Par->getType().getTypePtr();
----------------
curdeius wrote:
> Wouldn't something along the lines:
> ```
> const auto &parameters = 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.
I've just seen that you can use `.param_begin()` and `.param_end()` instead of `parameters.begin()/end()`.
I'd also reduce the use of `auto` here, because it's pretty hard to read it IMO.
So, I'd suggest

```
return std::any_of(Node.param_begin(), Node.param_end(), [](const ParmVarDecl *Par) {
  const QualType &ParType = Par->getType();

  if (isNonConstReferenceType(ParType)) {
    return true;
  }
  if (ParType.getTypePtr()->isPointerType()) {
    return true;
  }
  return false;
});
```


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

https://reviews.llvm.org/D55433