[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check
bahramib created this revision.
bahramib added reviewers: aaron.ballman, njames93, whisperity.
bahramib added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, rnkovacs, xazax.hun, mgorny.
Herald added a project: All.
bahramib requested review of this revision.
Flags function calls which return value is discarded if most of the other calls
of the function consume the return value. This check takes the threshold
for "most" as a config option and works on statistics gathered from the
call sites encountered in the analyzed file.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124446
Files:
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp
clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.h
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc-discarded-return-value.rst
clang-tools-extra/test/clang-tidy/checkers/misc-discarded-return-value-50p.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc-discarded-return-value-50p.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-discarded-return-value-50p.cpp
@@ -0,0 +1,856 @@
+// RUN: %check_clang_tidy %s misc-discarded-return-value -std=c++17 %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: misc-discarded-return-value.ConsumeThreshold, value: 50} \
+// RUN: ]}'
+
+extern bool Coin;
+extern int sink(int);
+extern int sink2(int, int);
+
+namespace std {
+
+using size_t = decltype(sizeof(void *));
+
+int printf(const char *Format, ...);
+
+template
+T &&declval() noexcept;
+
+template
+struct default_delete {};
+
+template
+struct initializer_list {
+ initializer_list(const T *, std::size_t) {}
+};
+
+template
+struct numeric_limits {
+ static constexpr std::size_t min() noexcept { return 0; }
+ static constexpr std::size_t max() noexcept { return 4; }
+};
+
+template
+struct remove_reference { typedef T type; };
+template
+struct remove_reference { typedef T type; };
+template
+struct remove_reference { typedef T type; };
+
+template
+typename remove_reference::type &&move(T &&V) noexcept {
+ return static_cast::type &&>(V);
+}
+
+template >
+class unique_ptr {
+public:
+ unique_ptr();
+ explicit unique_ptr(T *);
+ template
+ unique_ptr(unique_ptr &&);
+};
+
+} // namespace std
+
+void voidFn();
+void voidTest() {
+ for (voidFn();; voidFn())
+;
+ voidFn(); // NO-WARN: void functions do not count for usages.
+}
+
+[[nodiscard]] int nodiscard();
+void nodiscardTest() {
+ int Consume = nodiscard();
+ nodiscard(); // NO-WARN from the check - [[nodiscard]] handled by Sema.
+}
+
+int silence();
+void silenceTest() {
+ (void)silence();
+ static_cast(silence());
+ reinterpret_cast(silence());
+ silence();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: return value of 'silence' is used in most calls, but not in this one [misc-discarded-return-value]
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: note: value consumed or checked in 75% (3 out of 4) of cases
+}
+
+int varInit();
+int varInit2();
+void varinitTest() {
+ int X = varInit();
+ varInit();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: return value of 'varInit' is used in most calls, but not in this one [misc-discarded-return-value]
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: note: value consumed or checked in 50% (1 out of 2) of cases
+
+ int Y = varInit2(), Z = varInit2();
+ varInit2();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: return value of 'varInit2' is used in most calls, but not in this one [misc-discarded-return-value]
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: note: value consumed or checked in 66% (2 out of 3) of cases
+}
+
+int passToFn();
+void passToFnTest() {
+ sink(passToFn());
+ passToFn();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: return value of 'passToFn'
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: note: value consumed or checked in 50% (1 out of 2)
+}
+
+int *array();
+int index();
+void indexTest() {
+ int T[4];
+ array()[index()];
+ T[index()];
+ array()[0];
+
+ index();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: return value of 'index'
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: note: value consumed or checked in 66% (2 out of 3)
+
+ array();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: return value of 'array'
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: note: value consumed or checked in 66% (2 out of 3)
+}
+
+int varargVal();
+void varargTest() {
+ std::printf("%d %d", varargVal(), varargVal());
+ varargVal();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: return value of 'varargVal'
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: note: value consumed or checked in 66% (2 out of 3)
+}
+
+int unary();
+void unaryTest() {
+ if (!unary())
+unary();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return value of '
[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information
bahramib created this revision. bahramib added reviewers: aaron.ballman, njames93, whisperity, martong. bahramib added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, arphaman, rnkovacs, xazax.hun. Herald added a project: All. bahramib requested review of this revision. This patch adds the necessary infrastructure bindings and overloads that allow checks to implement collecting and emitting per-TU data to a directory from which the diagnosis mode can read it and decide whether to diagnose. What checks emit and how they transform the data is a per-check implementation detail. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D124447 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang-tools-extra/clang-tidy/ClangTidy.h clang-tools-extra/clang-tidy/ClangTidyCheck.cpp clang-tools-extra/clang-tidy/ClangTidyCheck.h clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h clang-tools-extra/clang-tidy/ClangTidyOptions.h clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/index.rst Index: clang-tools-extra/docs/clang-tidy/index.rst === --- clang-tools-extra/docs/clang-tidy/index.rst +++ clang-tools-extra/docs/clang-tidy/index.rst @@ -227,6 +227,32 @@ complete list of passes, use the :option:`--list-checks` and :option:`-load` options together. + --multipass-dir= - + When executing project-level analysis, specify + a directory where data can be stored inbetween + phases. + --multipass-phase= - + When executing project-level analysis, specify + which phase of the analysis to run. Multi-pass + project-level analysis requires the execution + of 3 passes in sequence. Not all checks support + this feature. +=collect - + Collect per-TU analysis data from checks that are + capable of multi-pass analysis. + + This pass can be executed in parallel. +=compact - + Transform the per-TU data into a single project-level + data to be consumed for diagnostics. + + This pass CAN NOT be executed in parallel! +=diagnose- + Emit diagnostics of the code, using the previously + collected and compacted data, or with per-TU data + only for single-pass analysis analysis. + + This pass can be executed in parallel. -p=- Build path --quiet- Run clang-tidy in quiet mode. This suppresses @@ -418,5 +444,44 @@ :program:`clang-tidy` will generate a ``clang-tidy-nolint`` error diagnostic if any ``NOLINTBEGIN``/``NOLINTEND`` comment violates these requirements. +Project-level analysis +== + +By default, Clang-Tidy runs checks on every translation unit of the project +separately. +Some checks, however, might benefit from and give better or more meaningful +results, or only work, when executed not for a single file, but for the entire +project. +The **multi-pass** analysis can be used in this case, with which checks can first +`collect` information into a temporary data location (``--multipass-dir``) on the +disk, `compact` per-TU data into project-level data, and `diagnose` with the +project-level data in mind. +The phase is selected by passing the appropriate value to the +``--multipass-phase`` command-line parameter. + +Whether a check supports project-level analysis, and how project-level data is +stored and transformed from per-TU to "global" values is specific to each +individual check. + +.. code-block:: bash +$ clang-tidy --checks=... file1.cpp file2.cpp + +$ clang-tidy --checks=... --multipass-phase=collect --multipass-dir="TidyData" file1.cpp file2.cpp +# No output to the terminal. +$ ls ./TidyData +blah.file1.cpp.01234567.yaml +blah.file2.cpp.89abcdef.yaml + +$ clang-tidy --checks=... --multipass-phase=compact --multipass-dir="TidyData" +# No file list required to be specified. +# No output to the terminal. +$ ls ./TidyData +blah.file1.cpp.01234567.yaml +blah.file2.cpp.89abcdef.yaml +blah.yaml + +
[PATCH] D124448: [clang-tidy] Add project-level analysis support to misc-discarded-return-value
bahramib created this revision.
bahramib added reviewers: aaron.ballman, njames93, whisperity, martong.
bahramib added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, rnkovacs, xazax.hun, mgorny.
Herald added a project: All.
bahramib requested review of this revision.
This enables calculating the consumed/discarded distribution of function
calls for the entire project, resulting in more meaningful diagnostics.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124448
Files:
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp
clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.h
Index: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.h
===
--- clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.h
+++ clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.h
@@ -12,6 +12,7 @@
#include "../ClangTidyCheck.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/StringMap.h"
#include
namespace clang {
@@ -28,18 +29,23 @@
struct Function {
std::size_t ConsumedCalls;
std::size_t TotalCalls;
+const FunctionDecl *FD;
llvm::SmallPtrSet DiscardedCEs;
/// Returns ConsumedCalls / TotalCalls expressed as a whole percentage.
std::uint8_t ratio() const;
};
- using FunctionMapTy = llvm::DenseMap;
+ using FunctionMapTy = llvm::StringMap;
DiscardedReturnValueCheck(StringRef Name, ClangTidyContext *Context);
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void onStartOfTranslationUnit() override;
void onEndOfTranslationUnit() override;
+ void collect(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void postCollect(StringRef OutputFile) override;
+ void compact(const std::vector &PerTuCollectedData,
+ StringRef OutputFile) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
@@ -49,16 +55,25 @@
/// the remaining 2 will be warned.
const std::uint8_t ConsumeThreshold;
+ /// Contains whether during diagnostics the data was loaded from a serialized
+ /// project-level file created by compact().
+ /// (This is only used as a result cache so no several rounds of lookup is
+ /// made.)
+ Optional CacheProjectDataLoadedSuccessfully;
+
/// Stores AST nodes which we have observed to be consuming calls.
/// (This is a helper data structure to prevent matchers matching consuming
/// contexts firing multiple times and messing up the statistics created.)
llvm::DenseMap> ConsumedCalls;
FunctionMapTy CallMap;
+ llvm::DenseMap FunctionIDs;
+ void matchResult(const ast_matchers::MatchFinder::MatchResult &Result,
+ bool ShouldCount);
void registerCall(const CallExpr *CE, const FunctionDecl *FD,
-const void *ConsumingContext);
- void diagnose(const FunctionDecl *FD, const Function &F);
+bool IncrementCounters, const void *ConsumingContext);
+ void diagnose(const Function &F);
};
} // namespace misc
Index: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp
@@ -9,11 +9,103 @@
#include "DiscardedReturnValueCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Index/USRGeneration.h"
+#include "llvm/Support/YAMLParser.h"
+#include "llvm/Support/YAMLTraits.h"
+#include "llvm/Support/raw_ostream.h"
using namespace clang;
using namespace clang::ast_matchers;
using namespace clang::tidy::misc;
+namespace {
+
+/// The format of the YAML document the checker uses in multi-pass project-level
+/// mode. This is the same for the per-TU and the per-project data.
+struct SerializedFunction {
+ std::string ID;
+ std::size_t ConsumedCalls, TotalCalls;
+};
+using FunctionVec = std::vector;
+
+} // namespace
+
+namespace llvm {
+namespace yaml {
+
+template <> struct MappingTraits {
+ static void mapping(IO &IO, SerializedFunction &F) {
+IO.mapRequired("ID", F.ID);
+IO.mapRequired("Consumed", F.ConsumedCalls);
+IO.mapRequired("Total", F.TotalCalls);
+ }
+};
+
+} // namespace yaml
+} // namespace llvm
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(SerializedFunction)
+
+static Optional loadYAML(StringRef File) {
+ using namespace llvm;
+
+ ErrorOr> IStream =
+ MemoryBuffer::getFileAsStream(File);
+ if (!IStream)
+return None;
+
+ FunctionVec R;
+ yaml::Input YIn{**IStream};
+ YIn >> R;
+
+ return R;
+}
+
+static bool loadYAML(StringRef FromFile,
+ DiscardedReturnValueCheck::FunctionMapTy &ToMap) {
+ Opt
