Author: Paul Kirth Date: 2026-05-14T12:47:45-07:00 New Revision: 9870819761b07a99c44ded0f1750fca7ec226223
URL: https://github.com/llvm/llvm-project/commit/9870819761b07a99c44ded0f1750fca7ec226223 DIFF: https://github.com/llvm/llvm-project/commit/9870819761b07a99c44ded0f1750fca7ec226223.diff LOG: Restore clang-doc arena allocation (#197595) This patch restores the commits reverted in 49f8ad172446dd54dd14c2333a7a0f638e37d05a. It effectively reapplies: Reapply "[clang-doc][nfc] Default initialize all StringRef members (#191641)" Reapply "[clang-doc] Initialize StringRef members in Info types (#191637)" Reapply "[clang-doc] Initialize member variable (#191570)" Reapply "[clang-doc] Merge data into persistent memory (#190056)" Reapply "[clang-doc] Support deep copy between arenas for merging (#190055)" This version has been updated to account for the new InfoNode<T> paradigm and APIs introduced earlier. Its logic is largely unchanged: it still performs deep copies between the transient and persistent arenas, merges the Info types incrementally, and clears the transient data between merge operations. It also incorporates a few of the cleanups we applied when trying to fix the Darwin x86 bots prior to reverting, and a new test case for issues brought up at that time. Added: Modified: clang-tools-extra/clang-doc/Representation.cpp clang-tools-extra/clang-doc/Representation.h clang-tools-extra/clang-doc/Serialize.cpp clang-tools-extra/clang-doc/tool/ClangDocMain.cpp clang-tools-extra/unittests/clang-doc/MergeTest.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp index a0b334e0be598..738c8b6559eb6 100644 --- a/clang-tools-extra/clang-doc/Representation.cpp +++ b/clang-tools-extra/clang-doc/Representation.cpp @@ -116,7 +116,30 @@ static void reduceChildren(OwningVec<T> &Children, Children, [Ptr](const auto &C) { return C.Ptr->USR == Ptr->USR; }); if (It == Children.end()) { - Children.push_back(*allocateListNodeTransient<T>(Ptr)); + InfoNode<T> *NewNode = allocateListNodePersistent<T>(Ptr->USR); + NewNode->Ptr->merge(std::move(*Ptr)); + Children.push_back(*NewNode); + } else { + It->Ptr->merge(std::move(*Ptr)); + } + } +} + +template <> +void reduceChildren<Reference>(OwningVec<Reference> &Children, + OwningVec<Reference> &&ChildrenToMerge) { + while (!ChildrenToMerge.empty()) { + Reference *Ptr = ChildrenToMerge.front().Ptr; + ChildrenToMerge.pop_front(); + + auto It = llvm::find_if( + Children, [Ptr](const auto &C) { return C.Ptr->USR == Ptr->USR; }); + if (It == Children.end()) { + InfoNode<Reference> *NewNode = allocateListNodePersistent<Reference>(); + NewNode->Ptr->USR = Ptr->USR; + NewNode->Ptr->RefType = Ptr->RefType; + NewNode->Ptr->merge(std::move(*Ptr)); + Children.push_back(*NewNode); } else { It->Ptr->merge(std::move(*Ptr)); } @@ -129,11 +152,108 @@ static void mergeUnkeyed(OwningVec<T> &Target, OwningVec<T> &&Source) { T *Ptr = Source.front().Ptr; Source.pop_front(); - if (!llvm::any_of(Target, [Ptr](const auto &E) { return *E.Ptr == *Ptr; })) - Target.push_back(*allocateListNodeTransient<T>(Ptr)); + if (!llvm::any_of(Target, + [Ptr](const auto &E) { return *E.Ptr == *Ptr; })) { + Target.push_back(*allocateListNodePersistent<T>(*Ptr)); + } } } +template <> +void mergeUnkeyed<CommentInfo>(OwningVec<CommentInfo> &Target, + OwningVec<CommentInfo> &&Source) { + while (!Source.empty()) { + CommentInfo *Ptr = Source.front().Ptr; + Source.pop_front(); + + if (!llvm::any_of(Target, + [Ptr](const auto &E) { return *E.Ptr == *Ptr; })) { + Target.push_back( + *allocateListNodePersistent<CommentInfo>(*Ptr, PersistentArena)); + } + } +} + +llvm::Error mergeSingleInfo(doc::OwnedPtr<doc::Info> &Reduced, + doc::OwnedPtr<doc::Info> &&NewInfo, + llvm::BumpPtrAllocator &Arena) { + if (!Reduced) { + switch (NewInfo->IT) { + case InfoType::IT_namespace: + Reduced = allocatePtr<NamespaceInfo>(Arena, NewInfo->USR); + break; + case InfoType::IT_record: + Reduced = allocatePtr<RecordInfo>(Arena, NewInfo->USR); + break; + case InfoType::IT_enum: + Reduced = allocatePtr<EnumInfo>(Arena, NewInfo->USR); + break; + case InfoType::IT_function: + Reduced = allocatePtr<FunctionInfo>(Arena, NewInfo->USR); + break; + case InfoType::IT_typedef: + Reduced = allocatePtr<TypedefInfo>(Arena, NewInfo->USR); + break; + case InfoType::IT_concept: + Reduced = allocatePtr<ConceptInfo>(Arena, NewInfo->USR); + break; + case InfoType::IT_variable: + Reduced = allocatePtr<VarInfo>(Arena, NewInfo->USR); + break; + case InfoType::IT_friend: + Reduced = allocatePtr<FriendInfo>(Arena, NewInfo->USR); + break; + default: + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "unknown info type"); + } + } + + if (Reduced->IT != NewInfo->IT) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "info types mismatch"); + + switch (Reduced->IT) { + case InfoType::IT_namespace: + static_cast<NamespaceInfo *>(getPtr(Reduced)) + ->merge(std::move(*static_cast<NamespaceInfo *>(getPtr(NewInfo)))); + break; + case InfoType::IT_record: + static_cast<RecordInfo *>(getPtr(Reduced)) + ->merge(std::move(*static_cast<RecordInfo *>(getPtr(NewInfo)))); + break; + case InfoType::IT_enum: + static_cast<EnumInfo *>(getPtr(Reduced)) + ->merge(std::move(*static_cast<EnumInfo *>(getPtr(NewInfo)))); + break; + case InfoType::IT_function: + static_cast<FunctionInfo *>(getPtr(Reduced)) + ->merge(std::move(*static_cast<FunctionInfo *>(getPtr(NewInfo)))); + break; + case InfoType::IT_typedef: + static_cast<TypedefInfo *>(getPtr(Reduced)) + ->merge(std::move(*static_cast<TypedefInfo *>(getPtr(NewInfo)))); + break; + case InfoType::IT_concept: + static_cast<ConceptInfo *>(getPtr(Reduced)) + ->merge(std::move(*static_cast<ConceptInfo *>(getPtr(NewInfo)))); + break; + case InfoType::IT_variable: + static_cast<VarInfo *>(getPtr(Reduced)) + ->merge(std::move(*static_cast<VarInfo *>(getPtr(NewInfo)))); + break; + case InfoType::IT_friend: + static_cast<FriendInfo *>(getPtr(Reduced)) + ->merge(std::move(*static_cast<FriendInfo *>(getPtr(NewInfo)))); + break; + default: + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "unknown info type"); + } + + return llvm::Error::success(); +} + // Dispatch function. llvm::Expected<OwnedPtr<Info>> mergeInfos(OwningPtrArray<Info> &Values) { if (Values.empty() || !Values[0]) @@ -164,6 +284,20 @@ llvm::Expected<OwnedPtr<Info>> mergeInfos(OwningPtrArray<Info> &Values) { llvm_unreachable("unhandled enumerator"); } +TemplateSpecializationInfo::TemplateSpecializationInfo( + const TemplateSpecializationInfo &Other, llvm::BumpPtrAllocator &Arena) + : SpecializationOf(Other.SpecializationOf) { + Params = allocateArray(Other.Params, Arena); +} + +TemplateInfo::TemplateInfo(const TemplateInfo &Other, + llvm::BumpPtrAllocator &Arena) { + Params = allocateArray(Other.Params, Arena); + if (Other.Specialization) + Specialization = TemplateSpecializationInfo(*Other.Specialization, Arena); + Constraints = allocateArray(Other.Constraints, Arena); +} + bool CommentInfo::operator==(const CommentInfo &Other) const { auto FirstCI = std::tie(Kind, Text, Name, Direction, ParamName, CloseName, SelfClosing, Explicit, AttrKeys, AttrValues, Args); @@ -273,10 +407,14 @@ bool Reference::mergeable(const Reference &Other) { void Reference::merge(Reference &&Other) { assert(mergeable(Other)); + assert(RefType != InfoType::IT_default && + "Merging reference with default InfoType"); if (Name.empty()) Name = Other.Name; if (Path.empty()) Path = Other.Path; + if (QualName.empty()) + QualName = Other.QualName; if (DocumentationFileName.empty()) DocumentationFileName = Other.DocumentationFileName; } @@ -291,16 +429,42 @@ void FriendInfo::merge(FriendInfo &&Other) { SymbolInfo::merge(std::move(Other)); } +FriendInfo::FriendInfo(const FriendInfo &Other, llvm::BumpPtrAllocator &Arena) + : SymbolInfo(Other, Arena) { + Ref = Other.Ref; + if (Other.Template) + Template.emplace(*Other.Template, Arena); + if (Other.ReturnType) + ReturnType = Other.ReturnType; + if (!Other.Params.empty()) + Params = allocateArray(Other.Params, Arena); + IsClass = Other.IsClass; +} + +Info::Info(const Info &Other, llvm::BumpPtrAllocator &Arena) + : Path(Other.Path), Name(Other.Name), + DocumentationFileName(Other.DocumentationFileName), USR(Other.USR), + ParentUSR(Other.ParentUSR), IT(Other.IT) { + Namespace = allocateArray(Other.Namespace, Arena); + if (!Other.Description.empty()) { + for (const auto &Desc : Other.Description) { + CommentInfo *NewDesc = allocatePtr<CommentInfo>(Arena, Desc, Arena); + Description.push_back(*allocateListNode<CommentInfo>(Arena, NewDesc)); + } + } +} + void Info::mergeBase(Info &&Other) { assert(mergeable(Other)); + assert(IT != InfoType::IT_default && "Merging info with default InfoType"); if (USR == EmptySID) USR = Other.USR; if (Name == "") Name = Other.Name; if (Path == "") Path = Other.Path; - if (Namespace.empty()) - Namespace = std::move(Other.Namespace); + if (Namespace.empty() && !Other.Namespace.empty()) + Namespace = allocateArray(Other.Namespace, PersistentArena); // Unconditionally extend the description, since each decl may have a comment. mergeUnkeyed(Description, std::move(Other.Description)); if (ParentUSR == EmptySID) @@ -313,6 +477,17 @@ bool Info::mergeable(const Info &Other) { return IT == Other.IT && USR == Other.USR; } +SymbolInfo::SymbolInfo(const SymbolInfo &Other, llvm::BumpPtrAllocator &Arena) + : Info(Other, Arena), DefLoc(Other.DefLoc), MangledName(Other.MangledName), + IsStatic(Other.IsStatic) { + if (!Other.Loc.empty()) { + for (const auto &L : Other.Loc) { + Location *NewL = allocatePtr<Location>(Arena, L); + Loc.push_back(*allocateListNode<Location>(Arena, NewL)); + } + } +} + void SymbolInfo::merge(SymbolInfo &&Other) { assert(mergeable(Other)); if (!DefLoc) @@ -322,6 +497,8 @@ void SymbolInfo::merge(SymbolInfo &&Other) { // Unconditionally extend the list of locations, since we want all of them. mergeUnkeyed(Loc, std::move(Other.Loc)); mergeBase(std::move(Other)); + if (!IsStatic) + IsStatic = Other.IsStatic; } NamespaceInfo::NamespaceInfo(SymbolID USR, StringRef Name, StringRef Path) @@ -343,37 +520,76 @@ void NamespaceInfo::merge(NamespaceInfo &&Other) { RecordInfo::RecordInfo(SymbolID USR, StringRef Name, StringRef Path) : SymbolInfo(InfoType::IT_record, USR, Name, Path) {} +// FIXME: This constructor is currently unsafe for cross-arena copies of +// populated records. Because a default copy of ScopeChildren will shallow-copy +// the intrusive pointers, leading to a use-after-free when the TransientArena +// is reset. Subsequent patches will address this by deep-copying children +// individually via reduceChildren. +RecordInfo::RecordInfo(const RecordInfo &Other, llvm::BumpPtrAllocator &Arena) + : SymbolInfo(Other, Arena), TagType(Other.TagType), + IsTypeDef(Other.IsTypeDef) { + Members = deepCopyArray(Other.Members, Arena); + Parents = allocateArray(Other.Parents, Arena); + VirtualParents = allocateArray(Other.VirtualParents, Arena); + Bases = deepCopyArray(Other.Bases, Arena); + Friends = deepCopyArray(Other.Friends, Arena); +} + +MemberTypeInfo::MemberTypeInfo(const MemberTypeInfo &Other, + llvm::BumpPtrAllocator &Arena) + : FieldTypeInfo(Other), Access(Other.Access), IsStatic(Other.IsStatic) { + if (!Other.Description.empty()) { + for (const auto &Desc : Other.Description) { + CommentInfo *NewDesc = allocatePtr<CommentInfo>(Arena, Desc, Arena); + Description.push_back(*allocateListNode<CommentInfo>(Arena, NewDesc)); + } + } +} + void RecordInfo::merge(RecordInfo &&Other) { assert(mergeable(Other)); if (!llvm::to_underlying(TagType)) TagType = Other.TagType; IsTypeDef = IsTypeDef || Other.IsTypeDef; - if (Members.empty()) - Members = std::move(Other.Members); - if (Bases.empty()) - Bases = std::move(Other.Bases); - if (Parents.empty()) - Parents = std::move(Other.Parents); - if (VirtualParents.empty()) - VirtualParents = std::move(Other.VirtualParents); - if (Friends.empty()) - Friends = std::move(Other.Friends); + if (Members.empty() && !Other.Members.empty()) + Members = deepCopyArray(Other.Members, PersistentArena); + if (Bases.empty() && !Other.Bases.empty()) + Bases = deepCopyArray(Other.Bases, PersistentArena); + if (Parents.empty() && !Other.Parents.empty()) + Parents = allocateArray(Other.Parents, PersistentArena); + if (VirtualParents.empty() && !Other.VirtualParents.empty()) + VirtualParents = allocateArray(Other.VirtualParents, PersistentArena); + if (Friends.empty() && !Other.Friends.empty()) + Friends = deepCopyArray(Other.Friends, PersistentArena); // Reduce children if necessary. reduceChildren(Children.Records, std::move(Other.Children.Records)); reduceChildren(Children.Functions, std::move(Other.Children.Functions)); reduceChildren(Children.Enums, std::move(Other.Children.Enums)); reduceChildren(Children.Typedefs, std::move(Other.Children.Typedefs)); - if (!Template) - Template = Other.Template; + if (!Template && Other.Template) + Template = TemplateInfo(*Other.Template, PersistentArena); SymbolInfo::merge(std::move(Other)); } +EnumValueInfo::EnumValueInfo(const EnumValueInfo &Other, + llvm::BumpPtrAllocator &Arena) + : Name(Other.Name), Value(Other.Value), ValueExpr(Other.ValueExpr) { + if (!Other.Description.empty()) { + for (const auto &Desc : Other.Description) { + CommentInfo *NewDesc = allocatePtr<CommentInfo>(Arena, Desc, Arena); + Description.push_back(*allocateListNode<CommentInfo>(Arena, NewDesc)); + } + } +} + void EnumInfo::merge(EnumInfo &&Other) { assert(mergeable(Other)); if (!Scoped) Scoped = Other.Scoped; - if (Members.empty()) - Members = std::move(Other.Members); + if (!BaseType && Other.BaseType) + BaseType = std::move(Other.BaseType); + if (Members.empty() && !Other.Members.empty()) + Members = deepCopyArray(Other.Members, PersistentArena); SymbolInfo::merge(std::move(Other)); } @@ -387,10 +603,10 @@ void FunctionInfo::merge(FunctionInfo &&Other) { ReturnType = std::move(Other.ReturnType); if (Parent.USR == EmptySID && Parent.Name == "") Parent = std::move(Other.Parent); - if (Params.empty()) - Params = std::move(Other.Params); - if (!Template) - Template = Other.Template; + if (Params.empty() && !Other.Params.empty()) + Params = allocateArray(Other.Params, PersistentArena); + if (!Template && Other.Template) + Template = TemplateInfo(*Other.Template, PersistentArena); SymbolInfo::merge(std::move(Other)); } @@ -400,8 +616,8 @@ void TypedefInfo::merge(TypedefInfo &&Other) { IsUsing = Other.IsUsing; if (Underlying.Type.Name == "") Underlying = Other.Underlying; - if (!Template) - Template = Other.Template; + if (!Template && Other.Template) + Template = TemplateInfo(*Other.Template, PersistentArena); SymbolInfo::merge(std::move(Other)); } @@ -411,10 +627,11 @@ void ConceptInfo::merge(ConceptInfo &&Other) { IsType = Other.IsType; if (ConstraintExpression.empty()) ConstraintExpression = std::move(Other.ConstraintExpression); - if (Template.Constraints.empty()) - Template.Constraints = std::move(Other.Template.Constraints); - if (Template.Params.empty()) - Template.Params = std::move(Other.Template.Params); + if (Template.Constraints.empty() && !Other.Template.Constraints.empty()) + Template.Constraints = + allocateArray(Other.Template.Constraints, PersistentArena); + if (Template.Params.empty() && !Other.Template.Params.empty()) + Template.Params = allocateArray(Other.Template.Params, PersistentArena); SymbolInfo::merge(std::move(Other)); } @@ -429,6 +646,11 @@ void VarInfo::merge(VarInfo &&Other) { BaseRecordInfo::BaseRecordInfo() : RecordInfo() {} +BaseRecordInfo::BaseRecordInfo(const BaseRecordInfo &Other, + llvm::BumpPtrAllocator &Arena) + : RecordInfo(Other, Arena), Access(Other.Access), + IsVirtual(Other.IsVirtual), IsParent(Other.IsParent) {} + BaseRecordInfo::BaseRecordInfo(SymbolID USR, StringRef Name, StringRef Path, bool IsVirtual, AccessSpecifier Access, bool IsParent) diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h index 638110acd479a..7d84998afd77b 100644 --- a/clang-tools-extra/clang-doc/Representation.h +++ b/clang-tools-extra/clang-doc/Representation.h @@ -52,6 +52,7 @@ class ConcurrentStringPool { ConcurrentStringPool &getGlobalStringPool(); extern thread_local llvm::BumpPtrAllocator TransientArena; +extern thread_local llvm::BumpPtrAllocator PersistentArena; inline StringRef internString(const Twine &T) { if (T.isTriviallyEmpty()) @@ -175,6 +176,15 @@ template <typename T> InfoNode<T> *allocateListNodeTransient(T *Item) { return allocateListNode<T>(TransientArena, Item); } +template <typename T, typename... Args> +InfoNode<T> *allocateListNodePersistent(Args &&...args) { + return allocateListNode<T>(PersistentArena, std::forward<Args>(args)...); +} + +template <typename T> InfoNode<T> *allocateListNodePersistent(T *Item) { + return allocateListNode<T>(PersistentArena, Item); +} + // An abstraction for lists that are dynamically managed (inserted/removed). // To be eventually transitioned to llvm::simple_ilist. template <typename T> using OwningVec = llvm::simple_ilist<InfoNode<T>>; @@ -411,6 +421,10 @@ struct TemplateParamInfo { }; struct TemplateSpecializationInfo { + TemplateSpecializationInfo() = default; + TemplateSpecializationInfo(const TemplateSpecializationInfo &Other, + llvm::BumpPtrAllocator &Arena); + // Indicates the declaration that this specializes. SymbolID SpecializationOf; @@ -430,6 +444,9 @@ struct ConstraintInfo { // Records the template information for a struct or function that is a template // or an explicit template specialization. struct TemplateInfo { + TemplateInfo() = default; + TemplateInfo(const TemplateInfo &Other, llvm::BumpPtrAllocator &Arena); + // May be empty for non-partial specializations. llvm::ArrayRef<TemplateParamInfo> Params = {}; @@ -461,6 +478,7 @@ struct FieldTypeInfo : public TypeInfo { // Info for member types. struct MemberTypeInfo : public FieldTypeInfo { MemberTypeInfo() = default; + MemberTypeInfo(const MemberTypeInfo &Other, llvm::BumpPtrAllocator &Arena); MemberTypeInfo(const TypeInfo &TI, StringRef Name, AccessSpecifier Access, bool IsStatic = false) : FieldTypeInfo(TI, Name), Access(Access), IsStatic(IsStatic) {} @@ -517,6 +535,7 @@ struct Info { StringRef Name = StringRef(), StringRef Path = StringRef()) : Path(internString(Path)), Name(internString(Name)), USR(USR), IT(IT) {} + Info(const Info &Other, llvm::BumpPtrAllocator &Arena); Info(const Info &Other) = delete; Info(Info &&Other) = default; @@ -579,6 +598,8 @@ struct SymbolInfo : public Info { StringRef Name = StringRef(), StringRef Path = StringRef()) : Info(IT, USR, Name, Path) {} + SymbolInfo(const SymbolInfo &Other, llvm::BumpPtrAllocator &Arena); + void merge(SymbolInfo &&I); bool operator<(const SymbolInfo &Other) const { @@ -607,6 +628,7 @@ struct FriendInfo : public SymbolInfo { FriendInfo(const InfoType IT, const SymbolID &USR, const StringRef Name = StringRef()) : SymbolInfo(IT, USR, Name) {} + FriendInfo(const FriendInfo &Other, llvm::BumpPtrAllocator &Arena); bool mergeable(const FriendInfo &Other); void merge(FriendInfo &&Other); @@ -658,6 +680,8 @@ struct RecordInfo : public SymbolInfo { RecordInfo(SymbolID USR = SymbolID(), StringRef Name = StringRef(), StringRef Path = StringRef()); + RecordInfo(const RecordInfo &Other, llvm::BumpPtrAllocator &Arena); + void merge(RecordInfo &&I); // Type of this record (struct, class, union, interface). @@ -712,6 +736,7 @@ struct TypedefInfo : public SymbolInfo { struct BaseRecordInfo : public RecordInfo { BaseRecordInfo(); + BaseRecordInfo(const BaseRecordInfo &Other, llvm::BumpPtrAllocator &Arena); BaseRecordInfo(SymbolID USR, StringRef Name, StringRef Path, bool IsVirtual, AccessSpecifier Access, bool IsParent); @@ -731,6 +756,8 @@ struct EnumValueInfo { : Name(internString(Name)), Value(internString(Value)), ValueExpr(internString(ValueExpr)) {} + EnumValueInfo(const EnumValueInfo &Other, llvm::BumpPtrAllocator &Arena); + bool operator==(const EnumValueInfo &Other) const { return std::tie(Name, Value, ValueExpr) == std::tie(Other.Name, Other.Value, Other.ValueExpr); @@ -806,6 +833,12 @@ struct Index : public Reference { // if they are diff erent. llvm::Expected<OwnedPtr<Info>> mergeInfos(OwningPtrArray<Info> &Values); +// Merges a single new Info into an existing Reduced Info (allocating it if +// needed). +llvm::Error mergeSingleInfo(doc::OwnedPtr<doc::Info> &Reduced, + doc::OwnedPtr<doc::Info> &&NewInfo, + llvm::BumpPtrAllocator &Arena); + struct ClangDocContext { ClangDocContext(tooling::ExecutionContext *ECtx, StringRef ProjectName, bool PublicOnly, StringRef OutDirectory, StringRef SourceRoot, diff --git a/clang-tools-extra/clang-doc/Serialize.cpp b/clang-tools-extra/clang-doc/Serialize.cpp index 17180368550b6..1c8969b56441a 100644 --- a/clang-tools-extra/clang-doc/Serialize.cpp +++ b/clang-tools-extra/clang-doc/Serialize.cpp @@ -233,14 +233,16 @@ void ClangDocCommentVisitor::parseComment(const comments::Comment *C) { if (NumChildren > 0) { CommentInfo *ChildrenArray = TransientArena.Allocate<CommentInfo>(NumChildren); - unsigned I = 0; + unsigned Idx = 0; for (comments::Comment *Child : llvm::make_range(C->child_begin(), C->child_end())) { - new (&ChildrenArray[I]) CommentInfo(); - ClangDocCommentVisitor Visitor(ChildrenArray[I]); + new (&ChildrenArray[Idx]) CommentInfo(); + ClangDocCommentVisitor Visitor(ChildrenArray[Idx]); Visitor.parseComment(Child); - I++; + Idx++; } + assert(Idx == NumChildren && + "Mismatch between child_count and actual children"); CurrentCI.Children = llvm::ArrayRef<CommentInfo>(ChildrenArray, NumChildren); } diff --git a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp index f627ee5887528..3c38901f4a0f9 100644 --- a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp +++ b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp @@ -29,6 +29,7 @@ #include "clang/Tooling/CommonOptionsParser.h" #include "clang/Tooling/Execution.h" #include "llvm/ADT/APFloat.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" @@ -358,15 +359,22 @@ Example usage for a project using a compile commands database: llvm::hardware_concurrency(ExecutorConcurrency)); { llvm::TimeTraceScope TS("Reduce"); - for (auto &Group : USRToBitcode) { - Pool.async([&, &Diags = Diags]() { // time trace decoding bitcode - if (FTimeTrace) + for (const auto &Group : USRToBitcode) { + StringRef Key = Group.getKey(); + std::vector<StringRef> Bitcodes = Group.getValue(); + Pool.async([Key, Bitcodes, &CDCtx, &Diags, &USRToInfo, &USRToInfoMutex, + &IndexMutex, &DiagMutex, &Error, DiagIDBitcodeReading, + DiagIDBitcodeMerging]() { + if (CDCtx.FTimeTrace) llvm::timeTraceProfilerInitialize(200, "clang-doc"); - doc::OwningPtrVec<doc::Info> Infos; + doc::OwnedPtr<doc::Info> Reduced = nullptr; { - llvm::TimeTraceScope Red("decoding bitcode"); - for (auto &Bitcode : Group.getValue()) { + llvm::TimeTraceScope Red("decoding and merging bitcode"); + for (const auto &Bitcode : Bitcodes) { + + llvm::scope_exit ArenaGuard( + [] { clang::doc::TransientArena.Reset(); }); llvm::BitstreamCursor Stream(Bitcode); doc::ClangDocBitcodeReader Reader(Stream, Diags); auto ReadInfos = Reader.readBitcode(); @@ -378,25 +386,17 @@ Example usage for a project using a compile commands database: Error = true; return; } - std::move(ReadInfos->begin(), ReadInfos->end(), - std::back_inserter(Infos)); - } - } // time trace decoding bitcode - - doc::OwnedPtr<doc::Info> Reduced; - - { - llvm::TimeTraceScope Merge("merging bitcode"); - auto ExpReduced = doc::mergeInfos(Infos); - - if (!ExpReduced) { - std::lock_guard<llvm::sys::Mutex> Guard(DiagMutex); - Diags.Report(DiagIDBitcodeMerging) - << toString(ExpReduced.takeError()); - return; + for (auto &I : *ReadInfos) { + if (auto Err = doc::mergeSingleInfo( + Reduced, std::move(I), clang::doc::PersistentArena)) { + std::lock_guard<llvm::sys::Mutex> Guard(DiagMutex); + Diags.Report(DiagIDBitcodeMerging) + << toString(std::move(Err)); + return; + } + } } - Reduced = std::move(*ExpReduced); - } // time trace merging bitcode + } // time trace decoding and merging bitcode // Add a reference to this Info in the Index { @@ -408,7 +408,7 @@ Example usage for a project using a compile commands database: { llvm::TimeTraceScope Merge("USRToInfo"); std::lock_guard<llvm::sys::Mutex> Guard(USRToInfoMutex); - USRToInfo[Group.getKey()] = std::move(Reduced); + USRToInfo[Key] = std::move(Reduced); } if (CDCtx.FTimeTrace) diff --git a/clang-tools-extra/unittests/clang-doc/MergeTest.cpp b/clang-tools-extra/unittests/clang-doc/MergeTest.cpp index 8b7c20bc0adfb..a8e7b9be1c39a 100644 --- a/clang-tools-extra/unittests/clang-doc/MergeTest.cpp +++ b/clang-tools-extra/unittests/clang-doc/MergeTest.cpp @@ -111,6 +111,113 @@ TEST_F(MergeTest, mergeNamespaceInfos) { CheckNamespaceInfo(InfoAsNamespace(&Expected), InfoAsNamespace(Actual.get())); } +TEST_F(MergeTest, mergeSingleNamespaceInfo) { + NamespaceInfo One; + One.Name = "Namespace"; + Reference Ns1[] = {Reference(EmptySID, "A", InfoType::IT_namespace)}; + One.Namespace = llvm::ArrayRef(Ns1); + + Reference RA(NonEmptySID, "ChildNamespace", InfoType::IT_namespace); + InfoNode<Reference> RANode(&RA); + One.Children.Namespaces.push_back(RANode); + Reference RC1(NonEmptySID, "ChildStruct", InfoType::IT_record); + InfoNode<Reference> RC1Node(&RC1); + One.Children.Records.push_back(RC1Node); + + FunctionInfo F1; + F1.Name = "OneFunction"; + F1.USR = NonEmptySID; + InfoNode<FunctionInfo> F1Node(&F1); + One.Children.Functions.push_back(F1Node); + + EnumInfo E1; + E1.Name = "OneEnum"; + E1.USR = NonEmptySID; + InfoNode<EnumInfo> E1Node(&E1); + One.Children.Enums.push_back(E1Node); + + NamespaceInfo Two; + Two.Name = "Namespace"; + Reference Ns2[] = {Reference(EmptySID, "A", InfoType::IT_namespace)}; + Two.Namespace = llvm::ArrayRef(Ns2); + + Reference RB(EmptySID, "OtherChildNamespace", InfoType::IT_namespace); + InfoNode<Reference> RBNode(&RB); + Two.Children.Namespaces.push_back(RBNode); + Reference RC2(EmptySID, "OtherChildStruct", InfoType::IT_record); + InfoNode<Reference> RC2Node(&RC2); + Two.Children.Records.push_back(RC2Node); + + FunctionInfo F2; + F2.Name = "TwoFunction"; + InfoNode<FunctionInfo> F2Node(&F2); + Two.Children.Functions.push_back(F2Node); + + EnumInfo E2; + E2.Name = "TwoEnum"; + InfoNode<EnumInfo> E2Node(&E2); + Two.Children.Enums.push_back(E2Node); + + NamespaceInfo Expected; + Expected.Name = "Namespace"; + Reference NsExpected[] = {Reference(EmptySID, "A", InfoType::IT_namespace)}; + Expected.Namespace = llvm::ArrayRef(NsExpected); + + Reference RC(NonEmptySID, "ChildNamespace", InfoType::IT_namespace); + InfoNode<Reference> RCNode(&RC); + Expected.Children.Namespaces.push_back(RCNode); + Reference RCE1(NonEmptySID, "ChildStruct", InfoType::IT_record); + InfoNode<Reference> RCE1Node(&RCE1); + Expected.Children.Records.push_back(RCE1Node); + Reference RD(EmptySID, "OtherChildNamespace", InfoType::IT_namespace); + InfoNode<Reference> RDNode(&RD); + Expected.Children.Namespaces.push_back(RDNode); + Reference RCE2(EmptySID, "OtherChildStruct", InfoType::IT_record); + InfoNode<Reference> RCE2Node(&RCE2); + Expected.Children.Records.push_back(RCE2Node); + + FunctionInfo FE1; + FE1.Name = "OneFunction"; + FE1.USR = NonEmptySID; + InfoNode<FunctionInfo> FE1Node(&FE1); + Expected.Children.Functions.push_back(FE1Node); + + FunctionInfo FE2; + FE2.Name = "TwoFunction"; + InfoNode<FunctionInfo> FE2Node(&FE2); + Expected.Children.Functions.push_back(FE2Node); + + EnumInfo EE1; + EE1.Name = "OneEnum"; + EE1.USR = NonEmptySID; + InfoNode<EnumInfo> EE1Node(&EE1); + Expected.Children.Enums.push_back(EE1Node); + + EnumInfo EE2; + EE2.Name = "TwoEnum"; + InfoNode<EnumInfo> EE2Node(&EE2); + Expected.Children.Enums.push_back(EE2Node); + NamespaceInfo ReducedObj; + ReducedObj.IT = InfoType::IT_namespace; + doc::OwnedPtr<doc::Info> Reduced = &ReducedObj; + + Info *PtrOne = &One; + auto Err1 = mergeSingleInfo(Reduced, std::move(PtrOne), doc::PersistentArena); + assert(!Err1); + + Info *PtrTwo = &Two; + auto Err2 = mergeSingleInfo(Reduced, std::move(PtrTwo), doc::PersistentArena); + assert(!Err2); + + CheckNamespaceInfo(InfoAsNamespace(&Expected), + static_cast<NamespaceInfo *>(getPtr(Reduced))); + + auto *RedNS = static_cast<NamespaceInfo *>(getPtr(Reduced)); + // Check that children functions are NOT the same instances as in One or Two + ASSERT_NE(RedNS->Children.Functions.front().Ptr, &F1); + ASSERT_NE(RedNS->Children.Functions.back().Ptr, &F2); +} + TEST_F(MergeTest, mergeRecordInfos) { RecordInfo One; One.Name = "r"; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
