Separate DIERef vs. user_id_t: GetForwardDeclClangTypeToDie()

[jankratochvil]

Summary

Reasons are the same as: D74637

[labath]

The thing I said on the other patch about DIERef vs user_id_t still applies. Let's continue the discussion there...

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

37 ↗

(On Diff #244873)

I'd very much like to avoid a default-constructed DIERef -- in the previous refactoring, I've deleted that concept and tried to use Optional<DIERef> in contexts, where the object can be empty.

[labath]

Per discussion on the other patch, I think this is fine too, just can we get rid of the DIERef default constructor? I think all you'd need is to use the insert DenseMap function instead of operator[].

[jankratochvil]

Sorry for that DenseMap::insert, I see it is the same as std::map, I should study DenseMap more.

[labath]

In D74690#1879121, @jankratochvil wrote:

Sorry for that DenseMap::insert, I see it is the same as std::map, I should study DenseMap more.

Yep, it tried to be similar to std::map, but there are some (sometimes subtle) differences.

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

1723–1725

Actually, try_emplace would be slightly better here. (Sorry for the back-and-forth :/).

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

if (!die.HasChildren()) {
                  ClangUtil::RemoveFastQualifiers(clang_type)
                      .GetOpaqueQualType()) &&
              "Type already in the forward declaration map!");
       // Can't assume m_ast.GetSymbolFile() is actually a
       // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple
       // binaries.
       dwarf->GetForwardDeclDieToClangType()[die.GetDIE()] =
           clang_type.GetOpaqueQualType();
-      dwarf->GetForwardDeclClangTypeToDie()
-          [ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType()] =
-          die.GetID();
+      dwarf->GetForwardDeclClangTypeToDie().try_emplace(
+          ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(),
+          *die.GetDIERef());
       m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
     }
   }
 
   // If we made a clang type, set the trivial abi if applicable: We only
   // do this for pass by value - which implies the Trivial ABI. There
   // isn't a way to assert that something that would normally be pass by
   // value is pass by reference, so we ignore that attribute if set.

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

 protected:
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb_private::Type *>
       DIEToTypePtr;
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP>
       DIEToVariableSP;
   typedef llvm::DenseMap<const DWARFDebugInfoEntry *,
                          lldb::opaque_compiler_type_t>
       DIEToClangType;
-  typedef llvm::DenseMap<lldb::opaque_compiler_type_t, lldb::user_id_t>
-      ClangTypeToDIE;
+  typedef llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> ClangTypeToDIE;
 
   DISALLOW_COPY_AND_ASSIGN(SymbolFileDWARF);
 
   virtual void LoadSectionData(lldb::SectionType sect_type,
                                lldb_private::DWARFDataExtractor &data);
 
   bool DeclContextMatchesThisSymbolFile(
       const lldb_private::CompilerDeclContext &decl_ctx);