[llvm-debuginfo-analyzer] Remove LVScope::Children container (llvm#144750)

Remove the `LVScope::Children` container and use `llvm::concat()`
instead to return a view over the types, symbols, and sub-scopes
contained in a given `LVScope`.

Fixes llvm#69160.

Author: jalopezg-git

llvm/include/llvm/DebugInfo/LogicalView/Core/LVObject.h

@@ -82,6 +82,8 @@ using LVScopes = SmallVector<LVScope *, 8>;
 using LVSymbols = SmallVector<LVSymbol *, 8>;
 using LVTypes = SmallVector<LVType *, 8>;
 
+using LVElementsView = detail::concat_range<LVElement *const, const LVScopes &,
+                                            const LVTypes &, const LVSymbols &>;
 using LVOffsets = SmallVector<LVOffset, 8>;
 
 // The following DWARF documents detail the 'tombstone' concept:

llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h

@@ -14,6 +14,7 @@
 #ifndef LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVSCOPE_H
 #define LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVSCOPE_H
 
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/DebugInfo/LogicalView/Core/LVElement.h"
 #include "llvm/DebugInfo/LogicalView/Core/LVLocation.h"
 #include "llvm/DebugInfo/LogicalView/Core/LVSort.h"
@@ -94,6 +95,11 @@ class LLVM_ABI LVScope : public LVElement {
   LVProperties<LVScopeKind> Kinds;
   LVProperties<Property> Properties;
   static LVScopeDispatch Dispatch;
+  // Empty containers used in `getChildren()` in case there is no Types,
+  // Symbols, or Scopes.
+  static const LVTypes EmptyTypes;
+  static const LVSymbols EmptySymbols;
+  static const LVScopes EmptyScopes;
 
   // Size in bits if this scope represents also a compound type.
   uint32_t BitSize = 0;
@@ -128,14 +134,6 @@ class LLVM_ABI LVScope : public LVElement {
   std::unique_ptr<LVLines> Lines;
   std::unique_ptr<LVLocations> Ranges;
 
-  // Vector of elements (types, scopes and symbols).
-  // It is the union of (*Types, *Symbols and *Scopes) to be used for
-  // the following reasons:
-  // - Preserve the order the logical elements are read in.
-  // - To have a single container with all the logical elements, when
-  //   the traversal does not require any specific element kind.
-  std::unique_ptr<LVElements> Children;
-
   // Resolve the template parameters/arguments relationship.
   void resolveTemplate();
   void printEncodedArgs(raw_ostream &OS, bool Full) const;
@@ -213,7 +211,23 @@ class LLVM_ABI LVScope : public LVElement {
   const LVScopes *getScopes() const { return Scopes.get(); }
   const LVSymbols *getSymbols() const { return Symbols.get(); }
   const LVTypes *getTypes() const { return Types.get(); }
-  const LVElements *getChildren() const { return Children.get(); }
+  // Return view over union of child Scopes, Types, and Symbols, in that order.
+  //
+  // Calling `LVScope::sort()` ensures that each of groups is sorted according
+  // to the given criteria (see also `LVOptions::setSortMode()`). Because
+  // `getChildren()` iterates over the concatenation, the result returned by
+  // this function is not necessarily sorted. If order is important, use
+  // `getSortedChildren()`.
+  LVElementsView getChildren() const {
+    return llvm::concat<LVElement *const>(Scopes ? *Scopes : EmptyScopes,
+                                          Types ? *Types : EmptyTypes,
+                                          Symbols ? *Symbols : EmptySymbols);
+  }
+  // Return vector of child Scopes, Types, and Symbols that is sorted using
+  // `SortFunction`. This requires copy + sort; if order is not important,
+  // use `getChildren()` instead.
+  LVElements getSortedChildren(
+      LVSortFunction SortFunction = llvm::logicalview::getSortFunction()) const;
 
   void addElement(LVElement *Element);
   void addElement(LVLine *Line);
@@ -222,7 +236,6 @@ class LLVM_ABI LVScope : public LVElement {
   void addElement(LVType *Type);
   void addObject(LVLocation *Location);
   void addObject(LVAddress LowerAddress, LVAddress UpperAddress);
-  void addToChildren(LVElement *Element);
 
   // Add the missing elements from the given 'Reference', which is the
   // scope associated with any DW_AT_specification, DW_AT_abstract_origin.

llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp

@@ -107,10 +107,16 @@ LVScopeDispatch LVScope::Dispatch = {
     {LVScopeKind::IsTryBlock, &LVScope::getIsTryBlock},
     {LVScopeKind::IsUnion, &LVScope::getIsUnion}};
 
-void LVScope::addToChildren(LVElement *Element) {
-  if (!Children)
-    Children = std::make_unique<LVElements>();
-  Children->push_back(Element);
+const LVTypes LVScope::EmptyTypes{};
+const LVSymbols LVScope::EmptySymbols{};
+const LVScopes LVScope::EmptyScopes{};
+
+LVElements LVScope::getSortedChildren(LVSortFunction SortFunction) const {
+  const auto UnsortedChildren = getChildren();
+  LVElements Elements{UnsortedChildren.begin(), UnsortedChildren.end()};
+  if (SortFunction)
+    llvm::stable_sort(Elements, SortFunction);
+  return Elements;
 }
 
 void LVScope::addElement(LVElement *Element) {
@@ -175,7 +181,6 @@ void LVScope::addElement(LVScope *Scope) {
 
   // Add it to parent.
   Scopes->push_back(Scope);
-  addToChildren(Scope);
   Scope->setParent(this);
 
   // Notify the reader about the new element being added.
@@ -202,7 +207,6 @@ void LVScope::addElement(LVSymbol *Symbol) {
 
   // Add it to parent.
   Symbols->push_back(Symbol);
-  addToChildren(Symbol);
   Symbol->setParent(this);
 
   // Notify the reader about the new element being added.
@@ -229,7 +233,6 @@ void LVScope::addElement(LVType *Type) {
 
   // Add it to parent.
   Types->push_back(Type);
-  addToChildren(Type);
   Type->setParent(this);
 
   // Notify the reader about the new element being added.
@@ -277,15 +280,12 @@ bool LVScope::removeElement(LVElement *Element) {
   if (Element->getIsLine())
     return RemoveElement(Lines);
 
-  if (RemoveElement(Children)) {
-    if (Element->getIsSymbol())
-      return RemoveElement(Symbols);
-    if (Element->getIsType())
-      return RemoveElement(Types);
-    if (Element->getIsScope())
-      return RemoveElement(Scopes);
-    llvm_unreachable("Invalid element.");
-  }
+  if (Element->getIsSymbol())
+    return RemoveElement(Symbols);
+  if (Element->getIsType())
+    return RemoveElement(Types);
+  if (Element->getIsScope())
+    return RemoveElement(Scopes);
 
   return false;
 }
@@ -356,9 +356,8 @@ void LVScope::updateLevel(LVScope *Parent, bool Moved) {
   setLevel(Parent->getLevel() + 1);
 
   // Update the children.
-  if (Children)
-    for (LVElement *Element : *Children)
-      Element->updateLevel(this, Moved);
+  for (LVElement *Element : getChildren())
+    Element->updateLevel(this, Moved);
 
   // Update any lines.
   if (Lines)
@@ -374,13 +373,12 @@ void LVScope::resolve() {
   LVElement::resolve();
 
   // Resolve the children.
-  if (Children)
-    for (LVElement *Element : *Children) {
-      if (getIsGlobalReference())
-        // If the scope is a global reference, mark all its children as well.
-        Element->setIsGlobalReference();
-      Element->resolve();
-    }
+  for (LVElement *Element : getChildren()) {
+    if (getIsGlobalReference())
+      // If the scope is a global reference, mark all its children as well.
+      Element->setIsGlobalReference();
+    Element->resolve();
+  }
 }
 
 void LVScope::resolveName() {
@@ -633,14 +631,13 @@ Error LVScope::doPrint(bool Split, bool Match, bool Print, raw_ostream &OS,
         options().getPrintFormatting() &&
         getLevel() < options().getOutputLevel()) {
       // Print the children.
-      if (Children)
-        for (const LVElement *Element : *Children) {
-          if (Match && !Element->getHasPattern())
-            continue;
-          if (Error Err =
-                  Element->doPrint(Split, Match, Print, *StreamSplit, Full))
-            return Err;
-        }
+      for (const LVElement *Element : getSortedChildren()) {
+        if (Match && !Element->getHasPattern())
+          continue;
+        if (Error Err =
+                Element->doPrint(Split, Match, Print, *StreamSplit, Full))
+          return Err;
+      }
 
       // Print the line records.
       if (Lines)
@@ -692,7 +689,6 @@ void LVScope::sort() {
           Traverse(Parent->Symbols, SortFunction);
           Traverse(Parent->Scopes, SortFunction);
           Traverse(Parent->Ranges, compareRange);
-          Traverse(Parent->Children, SortFunction);
 
           if (Parent->Scopes)
             for (LVScope *Scope : *Parent->Scopes)
@@ -978,9 +974,8 @@ bool LVScope::equals(const LVScopes *References, const LVScopes *Targets) {
 void LVScope::report(LVComparePass Pass) {
   getComparator().printItem(this, Pass);
   getComparator().push(this);
-  if (Children)
-    for (LVElement *Element : *Children)
-      Element->report(Pass);
+  for (LVElement *Element : getSortedChildren())
+    Element->report(Pass);
 
   if (Lines)
     for (LVLine *Line : *Lines)
@@ -1656,9 +1651,8 @@ void LVScopeCompileUnit::printMatchedElements(raw_ostream &OS,
       // Print the view for the matched scopes.
       for (const LVScope *Scope : MatchedScopes) {
         Scope->print(OS);
-        if (const LVElements *Elements = Scope->getChildren())
-          for (LVElement *Element : *Elements)
-            Element->print(OS);
+        for (LVElement *Element : Scope->getSortedChildren())
+          Element->print(OS);
       }
     }
 

llvm/test/tools/llvm-debuginfo-analyzer/COFF/01-coff-print-basic-details.test

@@ -18,31 +18,14 @@
 ; sorted by the debug information internal offset; it includes its lexical
 ; level and debug info format.
 
-; RUN: llvm-debuginfo-analyzer --attribute=level,format \
-; RUN:                         --output-sort=offset \
-; RUN:                         --print=scopes,symbols,types,lines,instructions \
-; RUN:                         %p/Inputs/test-codeview-clang.o 2>&1 | \
-; RUN: FileCheck --strict-whitespace -check-prefix=ONE %s
-
-; If `--output-sort=id`, elements are iterated in the order in which they were
-; added (which matches the increasing offset of the reference output).
 ; RUN: llvm-debuginfo-analyzer --attribute=level,format \
 ; RUN:                         --output-sort=id \
 ; RUN:                         --print=scopes,symbols,types,lines,instructions \
 ; RUN:                         %p/Inputs/test-codeview-clang.o 2>&1 | \
 ; RUN: FileCheck --strict-whitespace -check-prefix=ONE %s
 
-; If `--output-sort=none`, `LVScope::Children` is not sorted; it, however,
-; reflects the order in which elements were added (same as `--output-sort=id`).
-; This is expected to change once #69160 is resolved though.
-; RUN: llvm-debuginfo-analyzer --attribute=level,format \
-; RUN:                         --output-sort=none \
-; RUN:                         --print=scopes,symbols,types,lines,instructions \
-; RUN:                         %p/Inputs/test-codeview-clang.o 2>&1 | \
-; RUN: FileCheck --strict-whitespace -check-prefix=ONE %s
-
 ; RUN: llvm-debuginfo-analyzer --attribute=level,format \
-; RUN:                         --output-sort=offset \
+; RUN:                         --output-sort=id \
 ; RUN:                         --print=elements \
 ; RUN:                         %p/Inputs/test-codeview-clang.o 2>&1 | \
 ; RUN: FileCheck --strict-whitespace -check-prefix=ONE %s
@@ -80,3 +63,43 @@
 ; ONE-NEXT: [003]                 {Code} 'addq	$0x20, %rsp'
 ; ONE-NEXT: [003]                 {Code} 'retq'
 ; ONE-NEXT: [002]               {TypeAlias} 'INTPTR' -> '* const int'
+
+; RUN: llvm-debuginfo-analyzer --attribute=level,format \
+; RUN:                         --output-sort=none \
+; RUN:                         --print=scopes,symbols,types,lines,instructions \
+; RUN:                         %p/Inputs/test-codeview-clang.o 2>&1 | \
+; RUN: FileCheck --strict-whitespace -check-prefix=ONE-NOSORT %s
+
+; ONE-NOSORT:      Logical View:
+; ONE-NOSORT-NEXT: [000]           {File} 'test-codeview-clang.o' -> COFF-x86-64
+; ONE-NOSORT-EMPTY:
+; ONE-NOSORT-NEXT: [001]             {CompileUnit} 'test.cpp'
+; ONE-NOSORT-NEXT: [002]               {Function} extern not_inlined 'foo' -> 'int'
+; ONE-NOSORT-NEXT: [003]                 {Block}
+; ONE-NOSORT-NEXT: [004]                   {Variable} 'CONSTANT' -> 'const int'
+; ONE-NOSORT-NEXT: [004]     5             {Line}
+; ONE-NOSORT-NEXT: [004]                   {Code} 'movl	$0x7, 0x4(%rsp)'
+; ONE-NOSORT-NEXT: [004]     6             {Line}
+; ONE-NOSORT-NEXT: [004]                   {Code} 'movl	$0x7, 0x1c(%rsp)'
+; ONE-NOSORT-NEXT: [004]                   {Code} 'jmp	0x8'
+; ONE-NOSORT-NEXT: [003]                 {TypeAlias} 'INTEGER' -> 'int'
+; ONE-NOSORT-NEXT: [003]                 {Parameter} 'ParamPtr' -> '* const int'
+; ONE-NOSORT-NEXT: [003]                 {Parameter} 'ParamUnsigned' -> 'unsigned'
+; ONE-NOSORT-NEXT: [003]                 {Parameter} 'ParamBool' -> 'bool'
+; ONE-NOSORT-NEXT: [003]     2           {Line}
+; ONE-NOSORT-NEXT: [003]                 {Code} 'subq	$0x20, %rsp'
+; ONE-NOSORT-NEXT: [003]                 {Code} 'andb	$0x1, %r8b'
+; ONE-NOSORT-NEXT: [003]                 {Code} 'movb	%r8b, 0x1b(%rsp)'
+; ONE-NOSORT-NEXT: [003]                 {Code} 'movl	%edx, 0x14(%rsp)'
+; ONE-NOSORT-NEXT: [003]                 {Code} 'movq	%rcx, 0x8(%rsp)'
+; ONE-NOSORT-NEXT: [003]     3           {Line}
+; ONE-NOSORT-NEXT: [003]                 {Code} 'testb	$0x1, 0x1b(%rsp)'
+; ONE-NOSORT-NEXT: [003]                 {Code} 'je	0x15'
+; ONE-NOSORT-NEXT: [003]     8           {Line}
+; ONE-NOSORT-NEXT: [003]                 {Code} 'movl	0x14(%rsp), %eax'
+; ONE-NOSORT-NEXT: [003]                 {Code} 'movl	%eax, 0x1c(%rsp)'
+; ONE-NOSORT-NEXT: [003]     9           {Line}
+; ONE-NOSORT-NEXT: [003]                 {Code} 'movl	0x1c(%rsp), %eax'
+; ONE-NOSORT-NEXT: [003]                 {Code} 'addq	$0x20, %rsp'
+; ONE-NOSORT-NEXT: [003]                 {Code} 'retq'
+; ONE-NOSORT-NEXT: [002]               {TypeAlias} 'INTPTR' -> '* const int'

llvm/test/tools/llvm-debuginfo-analyzer/DWARF/01-dwarf-compare-logical-elements.test

@@ -35,8 +35,8 @@
 ; ONE-NEXT:  [002]     1         {TypeAlias} 'INTPTR' -> '* const int'
 ; ONE-NEXT:  [002]     2         {Function} extern not_inlined 'foo' -> 'int'
 ; ONE-NEXT:  [003]                 {Block}
-; ONE-NEXT:  [004]     5             {Variable} 'CONSTANT' -> 'const INTEGER'
 ; ONE-NEXT: +[004]     4             {TypeAlias} 'INTEGER' -> 'int'
+; ONE-NEXT:  [004]     5             {Variable} 'CONSTANT' -> 'const INTEGER'
 ; ONE-NEXT:  [003]     2           {Parameter} 'ParamBool' -> 'bool'
 ; ONE-NEXT:  [003]     2           {Parameter} 'ParamPtr' -> 'INTPTR'
 ; ONE-NEXT:  [003]     2           {Parameter} 'ParamUnsigned' -> 'unsigned int'