[lldb/DWARF] Add a utility function for (forceful) completion of types

[labath]

Summary

Unify the code for requiring a complete type and move it into a single
place. The only functional change is that the "cannot start a definition
of an incomplete type" is upgrated from a runtime error/warning to an
lldbassert. An plain assert might also be fine, since (AFAICT) this can
only happen in case of a programmer error.

[shafik]

LGTM

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1339

This has to be a TagDecl so why use auto?

[teemperor]

LGTM too, thanks!

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1339

const TagDecl *td = ClangUtil::GetAsTagDecl(type);

[labath]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1339

I think this is a borderline case of "use auto when the type is obvious" policy -- the type name is embedded in the function name, but there's technically no guarantee that it really returns that (it could return an Expected<TagDecl> among other things). OTOH, the name is short enough to just spell it out.
The ClangUtil thingy is cool though.

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

lldb::TypeSP ParseEnum(const lldb_private::SymbolContext &sc,
                          const DWARFDIE &die, ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP ParseSubroutine(const DWARFDIE &die,
                                ParsedDWARFTypeAttributes &attrs);
   // FIXME: attrs should be passed as a const reference.
   lldb::TypeSP ParseArrayType(const DWARFDIE &die,
                               ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP ParsePointerToMemberType(const DWARFDIE &die,
                                         const ParsedDWARFTypeAttributes &attrs);
+
+  /// Complete a type from debug info, or mark it as forcefully completed if
+  /// there is no of the type in the current Module. Call this function in
+  /// contexts where the usual C++ rules require a type to be complete (base
+  /// class, member, etc.).
+  void CompleteType(lldb_private::CompilerType type);
 };
 
 /// Parsed form of all attributes that are relevant for type reconstruction.
 /// Some attributes are relevant for all kinds of types (declaration), while
 /// others are only meaningful to a specific type (is_virtual)
 struct ParsedDWARFTypeAttributes {
   explicit ParsedDWARFTypeAttributes(const DWARFDIE &die);
 

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

TypeSP DWARFASTParserClang::ParseArrayType(const DWARFDIE &die,
   llvm::Optional<SymbolFile::ArrayInfo> array_info = ParseChildArrayInfo(die);
   if (array_info) {
     attrs.byte_stride = array_info->byte_stride;
     attrs.bit_stride = array_info->bit_stride;
   }
   if (attrs.byte_stride == 0 && attrs.bit_stride == 0)
     attrs.byte_stride = element_type->GetByteSize().getValueOr(0);
   CompilerType array_element_type = element_type->GetForwardCompilerType();
-
-  if (TypeSystemClang::IsCXXClassType(array_element_type) &&
-      !array_element_type.GetCompleteType()) {
-    ModuleSP module_sp = die.GetModule();
-
-    // Mark the class as complete, but we make a note of the fact that
-    // this class is not _really_ complete so we can later search for a
-    // definition in a different module.
-    // Since we provide layout assistance, all ivars in this class and other
-    // classes will be fine even if we are not able to find the definition
-    // elsewhere.
-    if (TypeSystemClang::StartTagDeclarationDefinition(array_element_type)) {
-      TypeSystemClang::CompleteTagDeclarationDefinition(array_element_type);
-      const auto *td =
-          TypeSystemClang::GetQualType(array_element_type.GetOpaqueQualType())
-              .getTypePtr()
-              ->getAsTagDecl();
-      m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
-    } else {
-      module_sp->ReportError("DWARF DIE at 0x%8.8x was not able to "
-                             "start its definition.\nPlease file a "
-                             "bug and attach the file at the start "
-                             "of this error message",
-                             type_die.GetOffset());
-    }
-  }
+  CompleteType(array_element_type);
 
   uint64_t array_element_bit_stride =
       attrs.byte_stride * 8 + attrs.bit_stride;
   CompilerType clang_type;
   if (array_info && array_info->element_orders.size() > 0) {
     uint64_t num_elements = 0;
     auto end = array_info->element_orders.rend();
     for (auto pos = array_info->element_orders.rbegin(); pos != end; ++pos) {
if (llvm::Optional<uint64_t> clang_type_size =
     return std::make_shared<Type>(die.GetID(), dwarf, attrs.name,
                                   *clang_type_size, nullptr, LLDB_INVALID_UID,
                                   Type::eEncodingIsUID, nullptr, clang_type,
                                   Type::ResolveState::Forward);
   }
   return nullptr;
 }
 
+void DWARFASTParserClang::CompleteType(CompilerType type) {
+  // Technically, enums can be incomplete too, but we don't handle those as they
+  // are emitted even under -flimit-debug-info.
+  if (!TypeSystemClang::IsCXXClassType(type))
+    return;
+
+  if (type.GetCompleteType())
+    return;
+
+  // No complete definition in this module.  Mark the class as complete to
+  // satisfy local ast invariants, but make a note of the fact that
+  // it is not _really_ complete so we can later search for a definition in a
+  // different module.
+  // Since we provide layout assistance, layouts of types containing this class
+  // will be correct even if we  are not able to find the definition elsewhere.
+  bool started = TypeSystemClang::StartTagDeclarationDefinition(type);
+  lldbassert(started && "Unable to start a class type definition.");
+  TypeSystemClang::CompleteTagDeclarationDefinition(type);
+  const clang::TagDecl *td = ClangUtil::GetAsTagDecl(type);
+  m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
+}
+
 TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType(
     const SymbolContext &sc, const DWARFDIE &die, TypeSP type_sp) {
   if (!type_sp)
     return type_sp;
 
   SymbolFileDWARF *dwarf = die.GetDWARF();
   TypeList &type_list = dwarf->GetTypeList();
   DWARFDIE sc_parent_die = SymbolFileDWARF::GetParentSymbolContextDIE(die);
if (die.HasChildren()) {
 
     if (!bases.empty()) {
       // Make sure all base classes refer to complete types and not forward
       // declarations. If we don't do this, clang will crash with an
       // assertion in the call to clang_type.TransferBaseClasses()
       for (const auto &base_class : bases) {
         clang::TypeSourceInfo *type_source_info =
             base_class->getTypeSourceInfo();
-        if (type_source_info) {
-          CompilerType base_class_type =
-              m_ast.GetType(type_source_info->getType());
-          if (!base_class_type.GetCompleteType()) {
-            // We mark the class as complete to allow the TransferBaseClasses
-            // call to succeed. But we make a note of the fact that this class
-            // is not _really_ complete so we can later search for a definition
-            // in a different module.
-            if (TypeSystemClang::StartTagDeclarationDefinition(
-                    base_class_type)) {
-              TypeSystemClang::CompleteTagDeclarationDefinition(
-                  base_class_type);
-              const auto *td = TypeSystemClang::GetQualType(
-                                   base_class_type.GetOpaqueQualType())
-                                   .getTypePtr()
-                                   ->getAsTagDecl();
-              m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
-            }
-          }
-        }
+        if (type_source_info)
+          CompleteType(m_ast.GetType(type_source_info->getType()));
       }
 
       m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(),
                                 std::move(bases));
     }
   }
 
   m_ast.AddMethodOverridesForCXXRecordType(clang_type.GetOpaqueQualType());
if (!is_artificial) {
                 }
 
                 member_clang_type =
                     m_ast.CreateArrayType(member_array_element_type, 0, false);
               }
             }
           }
 
-          if (TypeSystemClang::IsCXXClassType(member_clang_type) &&
-              !member_clang_type.GetCompleteType()) {
-            // Mark the class as complete, but we make a note of the fact that
-            // this class is not _really_ complete so we can later search for a
-            // definition in a different module.
-            // Since we provide layout assistance, all ivars in this class and
-            // other classes will be fine even if we are not able to find the
-            // definition elsewhere.
-            if (TypeSystemClang::StartTagDeclarationDefinition(
-                    member_clang_type)) {
-              TypeSystemClang::CompleteTagDeclarationDefinition(
-                  member_clang_type);
-              const auto *td = TypeSystemClang::GetQualType(
-                                   member_clang_type.GetOpaqueQualType())
-                                   .getTypePtr()
-                                   ->getAsTagDecl();
-              m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
-            } else {
-              module_sp->ReportError(
-                  "DWARF DIE at 0x%8.8x (class %s) has a member variable "
-                  "0x%8.8x (%s) whose type claims to be a C++ class but we "
-                  "were not able to start its definition.\nPlease file a "
-                  "bug and attach the file at the start of this error "
-                  "message",
-                  parent_die.GetOffset(), parent_die.GetName(), die.GetOffset(),
-                  name);
-            }
-          }
+          CompleteType(member_clang_type);
 
           field_decl = TypeSystemClang::AddFieldToRecordType(
               class_clang_type, name, member_clang_type, accessibility,
               bit_size);
 
           m_ast.SetMetadataAsUserID(field_decl, die.GetID());
 
           layout_info.field_offsets.insert(