[PATCH] D124448: [clang-tidy] Add project-level analysis support to misc-discarded-return-value

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:132
+
+  auto IDIt = FunctionIDs.find(FD);
+  if (IDIt == FunctionIDs.end()) {
----------------
Please spell out the type instead of `auto`.


================
Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:132-142
+  auto IDIt = FunctionIDs.find(FD);
+  if (IDIt == FunctionIDs.end()) {
+    SmallString<128> ID;
+    bool USRFailure = clang::index::generateUSRForDecl(FD, ID);
+    if (USRFailure)
+      ID = FD->getDeclName().getAsString();
+    assert(!ID.empty() && "Generating name for function failed");
----------------
martong wrote:
> Please spell out the type instead of `auto`.
This could be split out nicely in a member function, e.g. `findFunctionID`.


================
Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:204
   // Once everything had been counted, emit the results.
-  for (const auto &Call : CallMap)
-    diagnose(Call.first, Call.second);
+  if (getPhase() == MultipassProjectPhase::Diagnose)
+    for (const auto &Call : CallMap)
----------------
How is it emitting dianoses then in the singleTU mode?


================
Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:365
+  // not invalidate the counters we just loaded.
+  bool ShouldCountMatches = !CacheProjectDataLoadedSuccessfully.getValue();
+
----------------
?


================
Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.h:70
   FunctionMapTy CallMap;
+  llvm::DenseMap<const FunctionDecl *, std::string> FunctionIDs;
 
----------------
Some docs could be useful here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124448