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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm trying to generate an example of the modernize-use-nodiscard checker on something open source...as I developed this checker on Windows I struggled a little getting cmake to build me the json file to run clang-tidy over everything and a lot of projects aren't setup of c++17 yet which is needed for [[nodiscard]] to be allowed,  but using ClangPowerTools inside Visual studio I decided to run it on **protobuf**

I ran "Tidy Fix" with just the following custom checks "-*,modernize-use-nodiscard" this will fix some of the headers (alot of the headers actually)

recompiling the project, I see this one, which made me laugh...

  protobuf\protobuf\src\google\protobuf\descriptor.cc(4238): warning C4834: discarding return value of function with 'nodiscard' attribute

from descriptor.h I can see its fixed the header adding [[nodiscard]]'s

  // Tries to find something in the fallback database and link in the
  // corresponding proto file.  Returns true if successful, in which case
  // the caller should search for the thing again.  These are declared
  // const because they are called by (semantically) const methods.
  [[nodiscard]] bool TryFindFileInFallbackDatabase(const std::string& name) const;
  [[nodiscard]] bool TryFindSymbolInFallbackDatabase(const std::string& name) const;

ironically there is a comment in the code...

    if (tables_->FindFile(proto.dependency(i)) == NULL &&
        (pool_->underlay_ == NULL ||
         pool_->underlay_->FindFileByName(proto.dependency(i)) == NULL)) {
      // We don't care what this returns since we'll find out below anyway.
      pool_->TryFindFileInFallbackDatabase(proto.dependency(i));
      ^^^^^^
    }
  }
  tables_->pending_files_.pop_back();

For me its as much about why the comment was put there, someone was having to explain why they didn't care about the return, but this could easily have been a bug

Bug the clang-tidy -fix doesn't break the build, the patch is large but nothing broke.

F7661182: protobuf.patch <https://reviews.llvm.org/F7661182>


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55433