llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Paul Kirth (ilovepi) <details> <summary>Changes</summary> Eventually, we want clang-doc to support arena allocation, but the widespread use of owning pointers in the data types prevents this. Rather than have wide scale refactoring, we can introduce a type alias that can be swapped out atomically to switch from smart pointers to raw pointers. This is the first of several refactorings that are intended to make the transition simpler. --- Patch is 30.82 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/184869.diff 16 Files Affected: - (modified) clang-tools-extra/clang-doc/BitcodeReader.cpp (+6-7) - (modified) clang-tools-extra/clang-doc/BitcodeReader.h (+3-4) - (modified) clang-tools-extra/clang-doc/Generators.cpp (+1-1) - (modified) clang-tools-extra/clang-doc/Generators.h (+2-2) - (modified) clang-tools-extra/clang-doc/HTMLGenerator.cpp (+2-2) - (modified) clang-tools-extra/clang-doc/JSONGenerator.cpp (+3-3) - (modified) clang-tools-extra/clang-doc/MDGenerator.cpp (+2-2) - (modified) clang-tools-extra/clang-doc/MDMustacheGenerator.cpp (+2-2) - (modified) clang-tools-extra/clang-doc/Mapper.cpp (+1-1) - (modified) clang-tools-extra/clang-doc/Representation.cpp (+4-5) - (modified) clang-tools-extra/clang-doc/Representation.h (+7-3) - (modified) clang-tools-extra/clang-doc/Serialize.cpp (+39-30) - (modified) clang-tools-extra/clang-doc/Serialize.h (+42-33) - (modified) clang-tools-extra/clang-doc/YAMLGenerator.cpp (+2-2) - (modified) clang-tools-extra/clang-doc/benchmarks/ClangDocBenchmark.cpp (+4-4) - (modified) clang-tools-extra/clang-doc/tool/ClangDocMain.cpp (+3-3) ``````````diff diff --git a/clang-tools-extra/clang-doc/BitcodeReader.cpp b/clang-tools-extra/clang-doc/BitcodeReader.cpp index 8c2f0041b340b..b9b23c48d42d4 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.cpp +++ b/clang-tools-extra/clang-doc/BitcodeReader.cpp @@ -1076,16 +1076,15 @@ llvm::Error ClangDocBitcodeReader::readBlockInfoBlock() { } template <typename T> -llvm::Expected<std::unique_ptr<Info>> -ClangDocBitcodeReader::createInfo(unsigned ID) { +llvm::Expected<OwnedPtr<Info>> ClangDocBitcodeReader::createInfo(unsigned ID) { llvm::TimeTraceScope("Reducing infos", "createInfo"); - std::unique_ptr<Info> I = std::make_unique<T>(); + OwnedPtr<Info> I = std::make_unique<T>(); if (auto Err = readBlock(ID, static_cast<T *>(I.get()))) return std::move(Err); - return std::unique_ptr<Info>{std::move(I)}; + return OwnedPtr<Info>{std::move(I)}; } -llvm::Expected<std::unique_ptr<Info>> +llvm::Expected<OwnedPtr<Info>> ClangDocBitcodeReader::readBlockToInfo(unsigned ID) { llvm::TimeTraceScope("Reducing infos", "readBlockToInfo"); switch (ID) { @@ -1112,9 +1111,9 @@ ClangDocBitcodeReader::readBlockToInfo(unsigned ID) { } // Entry point -llvm::Expected<std::vector<std::unique_ptr<Info>>> +llvm::Expected<std::vector<OwnedPtr<Info>>> ClangDocBitcodeReader::readBitcode() { - std::vector<std::unique_ptr<Info>> Infos; + std::vector<OwnedPtr<Info>> Infos; if (auto Err = validateStream()) return std::move(Err); diff --git a/clang-tools-extra/clang-doc/BitcodeReader.h b/clang-tools-extra/clang-doc/BitcodeReader.h index 8c1f9d67ac53e..025203acb543d 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.h +++ b/clang-tools-extra/clang-doc/BitcodeReader.h @@ -31,7 +31,7 @@ class ClangDocBitcodeReader { : Stream(Stream), Diags(Diags) {} // Main entry point, calls readBlock to read each block in the given stream. - llvm::Expected<std::vector<std::unique_ptr<Info>>> readBitcode(); + llvm::Expected<std::vector<OwnedPtr<Info>>> readBitcode(); private: enum class Cursor { BadBlock = 1, Record, BlockEnd, BlockBegin }; @@ -53,15 +53,14 @@ class ClangDocBitcodeReader { template <typename T> llvm::Error readRecord(unsigned ID, T I); // Allocate the relevant type of info and add read data to the object. - template <typename T> - llvm::Expected<std::unique_ptr<Info>> createInfo(unsigned ID); + template <typename T> llvm::Expected<OwnedPtr<Info>> createInfo(unsigned ID); // Helper function to step through blocks to find and dispatch the next record // or block to be read. llvm::Expected<Cursor> skipUntilRecordOrBlock(unsigned &BlockOrRecordID); // Helper function to set up the appropriate type of Info. - llvm::Expected<std::unique_ptr<Info>> readBlockToInfo(unsigned ID); + llvm::Expected<OwnedPtr<Info>> readBlockToInfo(unsigned ID); template <typename InfoType, typename T, typename CallbackFunction> llvm::Error handleSubBlock(unsigned ID, T Parent, CallbackFunction Function); diff --git a/clang-tools-extra/clang-doc/Generators.cpp b/clang-tools-extra/clang-doc/Generators.cpp index 7ad688d6d1b04..854f09b9b4a82 100644 --- a/clang-tools-extra/clang-doc/Generators.cpp +++ b/clang-tools-extra/clang-doc/Generators.cpp @@ -66,7 +66,7 @@ Error MustacheGenerator::setupTemplate( } Error MustacheGenerator::generateDocumentation( - StringRef RootDir, StringMap<std::unique_ptr<doc::Info>> Infos, + StringRef RootDir, StringMap<doc::OwnedPtr<doc::Info>> Infos, const clang::doc::ClangDocContext &CDCtx, std::string DirName) { { llvm::TimeTraceScope TS("Setup Templates"); diff --git a/clang-tools-extra/clang-doc/Generators.h b/clang-tools-extra/clang-doc/Generators.h index 7ba66947a830a..8562e3dfa99d8 100644 --- a/clang-tools-extra/clang-doc/Generators.h +++ b/clang-tools-extra/clang-doc/Generators.h @@ -30,7 +30,7 @@ class Generator { // Write out the decl info for the objects in the given map in the specified // format. virtual llvm::Error generateDocumentation( - StringRef RootDir, llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + StringRef RootDir, llvm::StringMap<doc::OwnedPtr<doc::Info>> Infos, const ClangDocContext &CDCtx, std::string DirName = "") = 0; // This function writes a file with the index previously constructed. @@ -132,7 +132,7 @@ struct MustacheGenerator : public Generator { /// JSON, and calls generateDocForJSON for each file. /// 4. A file of the desired format is created. llvm::Error generateDocumentation( - StringRef RootDir, llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + StringRef RootDir, llvm::StringMap<doc::OwnedPtr<doc::Info>> Infos, const clang::doc::ClangDocContext &CDCtx, std::string DirName) override; }; diff --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp b/clang-tools-extra/clang-doc/HTMLGenerator.cpp index 51b2d1f6a92f4..4cab14c2ce200 100644 --- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp +++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp @@ -45,7 +45,7 @@ class HTMLGenerator : public MustacheGenerator { Error setupTemplateResources(const ClangDocContext &CDCtx, json::Value &V, SmallString<128> RelativeRootPath); llvm::Error generateDocumentation( - StringRef RootDir, llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + StringRef RootDir, llvm::StringMap<doc::OwnedPtr<doc::Info>> Infos, const ClangDocContext &CDCtx, std::string DirName) override; }; @@ -183,7 +183,7 @@ Error HTMLGenerator::createResources(ClangDocContext &CDCtx) { } Error HTMLGenerator::generateDocumentation( - StringRef RootDir, llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + StringRef RootDir, llvm::StringMap<doc::OwnedPtr<doc::Info>> Infos, const ClangDocContext &CDCtx, std::string DirName) { return MustacheGenerator::generateDocumentation(RootDir, std::move(Infos), CDCtx, "html"); diff --git a/clang-tools-extra/clang-doc/JSONGenerator.cpp b/clang-tools-extra/clang-doc/JSONGenerator.cpp index 354bbf63d62a2..1b31cc065561e 100644 --- a/clang-tools-extra/clang-doc/JSONGenerator.cpp +++ b/clang-tools-extra/clang-doc/JSONGenerator.cpp @@ -17,7 +17,7 @@ class JSONGenerator : public Generator { bool Markdown = false; Error generateDocumentation(StringRef RootDir, - llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + llvm::StringMap<doc::OwnedPtr<doc::Info>> Infos, const ClangDocContext &CDCtx, std::string DirName) override; Error createResources(ClangDocContext &CDCtx) override; @@ -912,7 +912,7 @@ static Error serializeIndex(const ClangDocContext &CDCtx, StringRef RootDir, } static void serializeContexts(Info *I, - StringMap<std::unique_ptr<Info>> &Infos) { + StringMap<OwnedPtr<Info>> &Infos) { if (I->USR == GlobalNamespaceID) return; auto ParentUSR = I->ParentUSR; @@ -935,7 +935,7 @@ static void serializeContexts(Info *I, } Error JSONGenerator::generateDocumentation( - StringRef RootDir, llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + StringRef RootDir, llvm::StringMap<doc::OwnedPtr<doc::Info>> Infos, const ClangDocContext &CDCtx, std::string DirName) { StringSet<> CreatedDirs; StringMap<std::vector<doc::Info *>> FileToInfos; diff --git a/clang-tools-extra/clang-doc/MDGenerator.cpp b/clang-tools-extra/clang-doc/MDGenerator.cpp index 5487ca2794769..c72dd11bc9df8 100644 --- a/clang-tools-extra/clang-doc/MDGenerator.cpp +++ b/clang-tools-extra/clang-doc/MDGenerator.cpp @@ -405,7 +405,7 @@ class MDGenerator : public Generator { static const char *Format; llvm::Error generateDocumentation( - StringRef RootDir, llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + StringRef RootDir, llvm::StringMap<doc::OwnedPtr<doc::Info>> Infos, const ClangDocContext &CDCtx, std::string DirName) override; llvm::Error createResources(ClangDocContext &CDCtx) override; llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream &OS, @@ -415,7 +415,7 @@ class MDGenerator : public Generator { const char *MDGenerator::Format = "md"; llvm::Error MDGenerator::generateDocumentation( - StringRef RootDir, llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + StringRef RootDir, llvm::StringMap<doc::OwnedPtr<doc::Info>> Infos, const ClangDocContext &CDCtx, std::string DirName) { // Track which directories we already tried to create. llvm::StringSet<> CreatedDirs; diff --git a/clang-tools-extra/clang-doc/MDMustacheGenerator.cpp b/clang-tools-extra/clang-doc/MDMustacheGenerator.cpp index 2b0b9a2f89fe1..ef7c605d988c1 100644 --- a/clang-tools-extra/clang-doc/MDMustacheGenerator.cpp +++ b/clang-tools-extra/clang-doc/MDMustacheGenerator.cpp @@ -27,7 +27,7 @@ static std::unique_ptr<MustacheTemplateFile> IndexTemplate = nullptr; struct MDMustacheGenerator : public MustacheGenerator { static const char *Format; Error generateDocumentation(StringRef RootDir, - StringMap<std::unique_ptr<doc::Info>> Infos, + StringMap<doc::OwnedPtr<doc::Info>> Infos, const ClangDocContext &CDCtx, std::string DirName) override; Error setupTemplateFiles(const ClangDocContext &CDCtx) override; @@ -72,7 +72,7 @@ Error MDMustacheGenerator::setupTemplateFiles(const ClangDocContext &CDCtx) { } Error MDMustacheGenerator::generateDocumentation( - StringRef RootDir, StringMap<std::unique_ptr<doc::Info>> Infos, + StringRef RootDir, StringMap<doc::OwnedPtr<doc::Info>> Infos, const clang::doc::ClangDocContext &CDCtx, std::string Dirname) { return MustacheGenerator::generateDocumentation(RootDir, std::move(Infos), CDCtx, "md"); diff --git a/clang-tools-extra/clang-doc/Mapper.cpp b/clang-tools-extra/clang-doc/Mapper.cpp index 24e798af52e62..d567fd28acfc4 100644 --- a/clang-tools-extra/clang-doc/Mapper.cpp +++ b/clang-tools-extra/clang-doc/Mapper.cpp @@ -62,7 +62,7 @@ bool MapASTVisitor::mapDecl(const T *D, bool IsDefinition) { return true; } - std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>> CP; + std::pair<OwnedPtr<Info>, OwnedPtr<Info>> CP; { llvm::TimeTraceScope TS("emit info from astnode"); diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp index 7436b114eb9cb..066ba6552814d 100644 --- a/clang-tools-extra/clang-doc/Representation.cpp +++ b/clang-tools-extra/clang-doc/Representation.cpp @@ -85,12 +85,12 @@ llvm::StringRef commentKindToString(CommentKind Kind) { const SymbolID EmptySID = SymbolID(); template <typename T> -static llvm::Expected<std::unique_ptr<Info>> -reduce(std::vector<std::unique_ptr<Info>> &Values) { +static llvm::Expected<OwnedPtr<Info>> +reduce(std::vector<OwnedPtr<Info>> &Values) { if (Values.empty() || !Values[0]) return llvm::createStringError(llvm::inconvertibleErrorCode(), "no value to reduce"); - std::unique_ptr<Info> Merged = std::make_unique<T>(Values[0]->USR); + OwnedPtr<Info> Merged = std::make_unique<T>(Values[0]->USR); T *Tmp = static_cast<T *>(Merged.get()); for (auto &I : Values) Tmp->merge(std::move(*static_cast<T *>(I.get()))); @@ -122,8 +122,7 @@ static void reduceChildren(std::vector<T> &Children, } // Dispatch function. -llvm::Expected<std::unique_ptr<Info>> -mergeInfos(std::vector<std::unique_ptr<Info>> &Values) { +llvm::Expected<OwnedPtr<Info>> mergeInfos(std::vector<OwnedPtr<Info>> &Values) { if (Values.empty() || !Values[0]) return llvm::createStringError(llvm::inconvertibleErrorCode(), "no info values to merge"); diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h index fbcc2cfbecc4a..4773b89255202 100644 --- a/clang-tools-extra/clang-doc/Representation.h +++ b/clang-tools-extra/clang-doc/Representation.h @@ -21,12 +21,17 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include <array> +#include <memory> #include <optional> #include <string> namespace clang { namespace doc { +// An abstraction for owned pointers. Initially mapped to std::unique_ptr, +// to be eventually transitioned to bare pointers in an arena. +template <typename T> using OwnedPtr = std::unique_ptr<T>; + // SHA1'd hash of a USR. using SymbolID = std::array<uint8_t, 20>; @@ -89,7 +94,7 @@ struct CommentInfo { // the vector. bool operator<(const CommentInfo &Other) const; - std::vector<std::unique_ptr<CommentInfo>> + std::vector<OwnedPtr<CommentInfo>> Children; // List of child comments for this CommentInfo. SmallString<8> Direction; // Parameter direction (for (T)ParamCommand). SmallString<16> Name; // Name of the comment (for Verbatim and HTML). @@ -625,8 +630,7 @@ struct Index : public Reference { // A standalone function to call to merge a vector of infos into one. // This assumes that all infos in the vector are of the same type, and will fail // if they are different. -llvm::Expected<std::unique_ptr<Info>> -mergeInfos(std::vector<std::unique_ptr<Info>> &Values); +llvm::Expected<OwnedPtr<Info>> mergeInfos(std::vector<OwnedPtr<Info>> &Values); struct ClangDocContext { ClangDocContext(tooling::ExecutionContext *ECtx, StringRef ProjectName, diff --git a/clang-tools-extra/clang-doc/Serialize.cpp b/clang-tools-extra/clang-doc/Serialize.cpp index f07187132cc07..afd2ffd391ba9 100644 --- a/clang-tools-extra/clang-doc/Serialize.cpp +++ b/clang-tools-extra/clang-doc/Serialize.cpp @@ -346,7 +346,7 @@ static std::string serialize(T &I, DiagnosticsEngine &Diags) { return Buffer.str().str(); } -std::string serialize(std::unique_ptr<Info> &I, DiagnosticsEngine &Diags) { +std::string serialize(OwnedPtr<Info> &I, DiagnosticsEngine &Diags) { switch (I->IT) { case InfoType::IT_namespace: return serialize(*static_cast<NamespaceInfo *>(I.get()), Diags); @@ -488,7 +488,7 @@ static void InsertChild(ScopeChildren &Scope, VarInfo Info) { // parameter. Since each variant is used once, it's not worth having a more // elaborate system to automatically deduce this information. template <typename ChildType> -static std::unique_ptr<Info> makeAndInsertIntoParent(ChildType Child) { +static OwnedPtr<Info> makeAndInsertIntoParent(ChildType Child) { if (Child.Namespace.empty()) { // Insert into unnamed parent namespace. auto ParentNS = std::make_unique<NamespaceInfo>(); @@ -940,9 +940,10 @@ parseBases(RecordInfo &I, const CXXRecordDecl *D, bool IsFileInRootDir, } } -std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>> -emitInfo(const NamespaceDecl *D, const FullComment *FC, Location Loc, - bool PublicOnly) { +std::pair<OwnedPtr<Info>, OwnedPtr<Info>> emitInfo(const NamespaceDecl *D, + const FullComment *FC, + Location Loc, + bool PublicOnly) { auto NSI = std::make_unique<NamespaceInfo>(); bool IsInAnonymousNamespace = false; populateInfo(*NSI, D, FC, IsInAnonymousNamespace); @@ -954,7 +955,7 @@ emitInfo(const NamespaceDecl *D, const FullComment *FC, Location Loc, : NSI->Name; NSI->Path = getInfoRelativePath(NSI->Namespace); if (NSI->Namespace.empty() && NSI->USR == SymbolID()) - return {std::unique_ptr<Info>{std::move(NSI)}, nullptr}; + return {OwnedPtr<Info>{std::move(NSI)}, nullptr}; // Namespaces are inserted into the parent by reference, so we need to return // both the parent and the record itself. @@ -1011,9 +1012,10 @@ static void parseFriends(RecordInfo &RI, const CXXRecordDecl *D) { } } -std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>> -emitInfo(const RecordDecl *D, const FullComment *FC, Location Loc, - bool PublicOnly) { +std::pair<OwnedPtr<Info>, OwnedPtr<Info>> emitInfo(const RecordDecl *D, + const FullComment *FC, + Location Loc, + bool PublicOnly) { auto RI = std::make_unique<RecordInfo>(); bool IsInAnonymousNamespace = false; @@ -1083,9 +1085,10 @@ emitInfo(const RecordDecl *D, const FullComment *FC, Location Loc, return {std::move(RI), std::move(Parent)}; } -std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>> -emitInfo(const FunctionDecl *D, const FullComment *FC, Location Loc, - bool PublicOnly) { +std::pair<OwnedPtr<Info>, OwnedPtr<Info>> emitInfo(const FunctionDecl *D, + const FullComment *FC, + Location Loc, + bool PublicOnly) { FunctionInfo Func; bool IsInAnonymousNamespace = false; populateFunctionInfo(Func, D, FC, Loc, IsInAnonymousNamespace); @@ -1097,9 +1100,10 @@ emitInfo(const FunctionDecl *D, const FullComment *FC, Location Loc, return {nullptr, makeAndInsertIntoParent<FunctionInfo &&>(std::move(Func))}; } -std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>> -emitInfo(const CXXMethodDecl *D, const FullComment *FC, Location Loc, - bool PublicOnly) { +std::pair<OwnedPtr<Info>, OwnedPtr<Info>> emitInfo(const CXXMethodDecl *D, + const FullComment *FC, + Location Loc, + bool PublicOnly) { FunctionInfo Func; bool IsInAnonymousNamespace = false; populateFunctionInfo(Func, D, FC, Loc, IsInAnonymousNamespace); @@ -1140,9 +1144,10 @@ static void extractCommentFromDecl(const Decl *D, TypedefInfo &Info) { } } -std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>> -emitInfo(const TypedefDecl *D, const FullComment *FC, Location Loc, - bool PublicOnly) { +std::pair<OwnedPtr<Info>, OwnedPtr<Info>> emitInfo(const TypedefDecl *D, + const FullComment *FC, + Location Loc, + bool PublicOnly) { TypedefInfo Info; bool IsInAnonymousNamespace = false; populateInfo(Info, D, FC, IsInAnonymousNamespace); @@ -1172,9 +1177,10 @@ emitInfo(const TypedefDecl *D, const FullComment *FC, Location Loc, // A type alias is a C++ "using" declaration for a type. It gets mapped to a // TypedefInfo with the IsUsing flag set. -std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>> -emitInfo(const TypeAliasDecl *D, const FullComment *FC, Location Loc, - bool PublicOnly) { +std::pair<OwnedPtr<Info>, OwnedPtr<Info>> emitInfo(const TypeAliasDecl *D, + const FullComment *FC, + Location Loc, + bool PublicOnly) { TypedefInfo Info; bool IsInAnonymousNamespace = false; populateInfo(Info, D, FC, IsInAnonymousNamespace); @@ -1196,9 +1202,10 @@ emitInfo(const TypeAliasDecl *D, const FullComment *FC, Location Loc, return {nullptr, makeAndInsertIntoParent<TypedefInfo &&>(std::move(Info))}; } -std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>> -emitInfo(const EnumDecl *D, const FullComment *FC, Location Loc, - bool PublicOnly) { +std::pair<OwnedPtr<Info>, OwnedPtr<Info>> emitInfo(const EnumDecl *D, + const FullComment *FC, + Location Loc, + bool PublicOnly) { EnumInfo Enum; bool IsInAnonymousNamespace = false; populateSymbolInfo(Enum, D, FC, Loc, IsInAnonymousNamespace); @@ -1217,9 +1224,10 @@ emitInfo(const EnumDecl *D, const FullComment *FC, Location Loc, return {nullptr, makeAndInsertIntoParent<EnumInfo &&>(std::move(Enum))}; } -std::pair<std::unique_ptr<Info>, std::unique_ptr<Info>> -emitInfo(const ConceptDecl *D, const FullComment *FC, const L... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/184869 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
