[llvm-debuginfo-analyzer] Remove LVScope::Children container

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`.

Author: jalopezg-git

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,14 @@ 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 Types, Symbols, and Scopes.
+  auto getUnsortedChildren() const {
+    return llvm::concat<LVElement *const>(Types ? *Types : EmptyTypes,
+                                          Symbols ? *Symbols : EmptySymbols,
+                                          Scopes ? *Scopes : EmptyScopes);
+  }
+  // Return sorted children (see `LVOptions::setSortMode()`).
+  LVElements getChildren() const;
 
   void addElement(LVElement *Element);
   void addElement(LVLine *Line);
@@ -222,7 +227,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,22 @@ 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::getChildren() const {
+  LVElements Elements(getUnsortedChildren());
+  if (LVSortFunction SortFunction = getSortFunction()) {
+    llvm::stable_sort(Elements, SortFunction);
+  } else {
+    // No specific sort function, sort by `LVObject::ID` which replicates the
+    // order in which the elements were created.
+    llvm::stable_sort(Elements, [](const LVObject *LHS, const LVObject *RHS) {
+      return LHS.ID < RHS.ID;
+    });
+  }
+  return Elements;
 }
 
 void LVScope::addElement(LVElement *Element) {
@@ -175,7 +187,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 +213,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 +239,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 +286,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 +362,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 +379,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 +637,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 : getChildren()) {
+        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 +695,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 +980,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 : getChildren())
+    Element->report(Pass);
 
   if (Lines)
     for (LVLine *Line : *Lines)
@@ -1656,9 +1657,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->getChildren())
+          Element->print(OS);
       }
     }
 

llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp

@@ -163,13 +163,12 @@ void checkUnspecifiedParameters(LVReader *Reader) {
   LVPublicNames::const_iterator IterNames = PublicNames.cbegin();
   LVScope *Function = (*IterNames).first;
   EXPECT_EQ(Function->getName(), "foo_printf");
-  const LVElements *Elements = Function->getChildren();
-  ASSERT_NE(Elements, nullptr);
+  const auto Elements = Function->getChildren();
   // foo_printf is a variadic function whose prototype is
   // `int foo_printf(const char *, ...)`, where the '...' is represented by a
   // DW_TAG_unspecified_parameters, i.e. we expect to find at least one child
   // for which getIsUnspecified() returns true.
-  EXPECT_TRUE(llvm::any_of(*Elements, [](const LVElement *elt) {
+  EXPECT_TRUE(llvm::any_of(Elements, [](const LVElement *elt) {
     return elt->getIsSymbol() &&
            static_cast<const LVSymbol *>(elt)->getIsUnspecified();
   }));
@@ -183,10 +182,9 @@ void checkScopeModule(LVReader *Reader) {
   EXPECT_EQ(Root->getFileFormatName(), "Mach-O 64-bit x86-64");
   EXPECT_EQ(Root->getName(), DwarfClangModule);
 
-  ASSERT_NE(CompileUnit->getChildren(), nullptr);
-  LVElement *FirstChild = *(CompileUnit->getChildren()->begin());
-  EXPECT_EQ(FirstChild->getIsScope(), 1);
-  LVScopeModule *Module = static_cast<LVScopeModule *>(FirstChild);
+  ASSERT_NE(CompileUnit->getScopes(), nullptr);
+  LVElement *FirstScope = *(CompileUnit->getScopes()->begin());
+  LVScopeModule *Module = static_cast<LVScopeModule *>(FirstScope);
   EXPECT_EQ(Module->getIsModule(), 1);
   EXPECT_EQ(Module->getName(), "DebugModule");
 }