Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -38,6 +38,7 @@ clangASTMatchers clangBasic clangLex + clangIndex clangSerialization clangTooling ) 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) { + Optional OV = loadYAML(FromFile); + if (!OV) + return false; + + for (const SerializedFunction &SF : *OV) { + DiscardedReturnValueCheck::Function &F = + ToMap + .try_emplace(SF.ID, + DiscardedReturnValueCheck::Function{0, 0, nullptr, {}}) + .first->second; + + F.ConsumedCalls += SF.ConsumedCalls; + F.TotalCalls += SF.TotalCalls; + } + + return true; +} + +static FunctionVec +yamlize(const DiscardedReturnValueCheck::FunctionMapTy &Map) { + FunctionVec SFs; + llvm::transform(Map, std::back_inserter(SFs), [](auto &&E) { + return SerializedFunction{E.first().str(), E.second.ConsumedCalls, + E.second.TotalCalls}; + }); + return SFs; +} + +static void writeYAML(StringRef Whence, FunctionVec Elements, + StringRef ToFile) { + std::error_code EC; + llvm::raw_fd_ostream FS(ToFile, EC, llvm::sys::fs::OF_Text); + if (EC) { + llvm::errs() << "DiscardedReturnValueCheck: Failed to write " << Whence + << " output file: " << EC.message(); + llvm::report_fatal_error("", false); + return; + } + + llvm::yaml::Output YAMLOut(FS); + YAMLOut << Elements; +} + static constexpr llvm::StringLiteral Call = "call"; static constexpr llvm::StringLiteral Consume = "consume"; @@ -32,10 +124,29 @@ void DiscardedReturnValueCheck::registerCall(const CallExpr *CE, const FunctionDecl *FD, + bool IncrementCounters, const void *ConsumingContext) { assert(CE && FD); FD = FD->getCanonicalDecl(); - Function &Data = CallMap.try_emplace(FD, Function{0, 0, {}}).first->second; + + 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"); + + IDIt = FunctionIDs.try_emplace(FD, ID.str().str()).first; + } + assert(IDIt != FunctionIDs.end()); + + Function &Data = + CallMap.try_emplace(IDIt->second, Function{0, 0, FD, {}}).first->second; + if (!Data.FD) + // If the map already contains data which was loaded from an earlier project + // pass, store FD (in the current TU) so diagose() can point to it properly. + Data.FD = FD; if (ConsumingContext) { using ConsumeVecTy = decltype(ConsumedCalls)::mapped_type; @@ -47,13 +158,14 @@ } else Data.DiscardedCEs.insert(CE); - if (ConsumingContext) - ++Data.ConsumedCalls; - ++Data.TotalCalls; + if (IncrementCounters) { + if (ConsumingContext) + ++Data.ConsumedCalls; + ++Data.TotalCalls; + } } -void DiscardedReturnValueCheck::diagnose(const FunctionDecl *FD, - const Function &F) { +void DiscardedReturnValueCheck::diagnose(const Function &F) { if (F.ratio() < ConsumeThreshold) return; @@ -61,7 +173,7 @@ diag(Call->getExprLoc(), "return value of %0 is used in most calls, but not in this one", DiagnosticIDs::Warning) - << *FD << F.ratio() << F.ConsumedCalls << F.TotalCalls; + << F.FD << F.ratio() << F.ConsumedCalls << F.TotalCalls; diag(Call->getExprLoc(), "value consumed or checked in %0%% (%1 out of %2) of cases", @@ -81,14 +193,17 @@ } void DiscardedReturnValueCheck::onStartOfTranslationUnit() { + CacheProjectDataLoadedSuccessfully.reset(); ConsumedCalls.clear(); CallMap.clear(); + FunctionIDs.clear(); } void DiscardedReturnValueCheck::onEndOfTranslationUnit() { // 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) + diagnose(Call.second); } namespace { @@ -204,7 +319,8 @@ Finder->addMatcher(traverse(TK_IgnoreUnlessSpelledInSource, Call), this); } -void DiscardedReturnValueCheck::check(const MatchFinder::MatchResult &Result) { +void DiscardedReturnValueCheck::matchResult( + const MatchFinder::MatchResult &Result, bool ShouldCount) { const auto *CE = Result.Nodes.getNodeAs(Call); assert(CE && "Bad matcher"); @@ -219,13 +335,51 @@ if (const auto *T = Result.Nodes.getNodeAs(Consume)) ConsumeNode = T; if (ConsumeNode) - return registerCall(CE, CE->getDirectCallee(), ConsumeNode); + return registerCall(CE, CE->getDirectCallee(), ShouldCount, ConsumeNode); // If ConsumeNode is left to be nullptr, the current match is not through the // "consuming" matcher. It might be the only match of this function (and then // it is discarded), or it might have been matched earlier and consumed. if (ConsumedCalls.find(CE) == ConsumedCalls.end()) - return registerCall(CE, CE->getDirectCallee(), nullptr); + return registerCall(CE, CE->getDirectCallee(), ShouldCount, nullptr); +} + +void DiscardedReturnValueCheck::collect( + const ast_matchers::MatchFinder::MatchResult &Result) { + matchResult(Result, true); +} + +void DiscardedReturnValueCheck::check(const MatchFinder::MatchResult &Result) { + // check() might be called without previous data existing if Tidy is executed + // in single-phase mode. + if (!CacheProjectDataLoadedSuccessfully.hasValue()) { + StringRef ProjectLevelData = getCompactedDataPath(); + bool Success = false; + if (!ProjectLevelData.empty() && CallMap.empty()) + Success = loadYAML(ProjectLevelData, CallMap); + + CacheProjectDataLoadedSuccessfully.emplace(Success); + } + // If there was previously loaded data, the current match callback should + // not invalidate the counters we just loaded. + bool ShouldCountMatches = !CacheProjectDataLoadedSuccessfully.getValue(); + + matchResult(Result, ShouldCountMatches); +} + +void DiscardedReturnValueCheck::postCollect(StringRef OutputFilename) { + writeYAML("postCollect()", yamlize(CallMap), OutputFilename); +} + +void DiscardedReturnValueCheck::compact( + const std::vector &PerTUCollectedData, StringRef OutputFile) { + for (const std::string &PerTUFilename : PerTUCollectedData) + if (!loadYAML(PerTUFilename, CallMap)) + llvm::errs() + << "DiscardedReturnValueCheck: Failed to load compact() input file " + << PerTUFilename << ".\n"; + + writeYAML("compact()", yamlize(CallMap), OutputFile); } } // namespace misc