[lldb] Make ClangASTImporter::LayoutRecordType always return the same layout

[teemperor]

Summary

ClangASTImporter::LayoutRecordType provides record layouts for Clang to use during CodeGen.

Currently this method only provides a layout once and then deletes the layout from the internal
storage. The next time we get a request for the same record layout we pretend that we don't know
the record and leave CodeGen on its own to produce the layout. This system currently seems to work as we
usually request a layout over an ASTContext which caches the result, so the ClangASTImporter
will most likely never be asked twice to layout a record unless the ASTContext changes at some
point.

I think it makes sense that we do not rely on the caching mechanism in the ASTContext to save
us from doing the wrong thing and instead always return the same layout from ClangASTContext.
Also marks the method as const because it is essentially just a getter now.

lldb/include/lldb/Symbol/ClangASTImporter.h

public:
 
   bool LayoutRecordType(
       const clang::RecordDecl *record_decl, uint64_t &bit_size,
       uint64_t &alignment,
       llvm::DenseMap<const clang::FieldDecl *, uint64_t> &field_offsets,
       llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits>
           &base_offsets,
       llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits>
-          &vbase_offsets);
+          &vbase_offsets) const;
 
   bool CanImport(const CompilerType &type);
 
   bool Import(const CompilerType &type);
 
   bool CompleteType(const CompilerType &compiler_type);
 
   void CompleteDecl(clang::Decl *decl);

lldb/source/Symbol/ClangASTImporter.cpp

 
 bool ClangASTImporter::LayoutRecordType(
     const clang::RecordDecl *record_decl, uint64_t &bit_size,
     uint64_t &alignment,
     llvm::DenseMap<const clang::FieldDecl *, uint64_t> &field_offsets,
     llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits>
         &base_offsets,
     llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits>
-        &vbase_offsets) {
-  RecordDeclToLayoutMap::iterator pos =
+        &vbase_offsets) const {
+  RecordDeclToLayoutMap::const_iterator pos =
       m_record_decl_to_layout_map.find(record_decl);
-  bool success = false;
   base_offsets.clear();
   vbase_offsets.clear();
   if (pos != m_record_decl_to_layout_map.end()) {
     bit_size = pos->second.bit_size;
     alignment = pos->second.alignment;
-    field_offsets.swap(pos->second.field_offsets);
-    base_offsets.swap(pos->second.base_offsets);
-    vbase_offsets.swap(pos->second.vbase_offsets);
-    m_record_decl_to_layout_map.erase(pos);
-    success = true;
-  } else {
-    bit_size = 0;
-    alignment = 0;
-    field_offsets.clear();
-  }
-  return success;
+    field_offsets = pos->second.field_offsets;
+    base_offsets = pos->second.base_offsets;
+    vbase_offsets = pos->second.vbase_offsets;
+    return true;
+  }
+  return false;
 }
 
 void ClangASTImporter::InsertRecordDecl(clang::RecordDecl *decl,
                                         const LayoutInfo &layout) {
   m_record_decl_to_layout_map.insert(std::make_pair(decl, layout));
 }
 
 void ClangASTImporter::CompleteDecl(clang::Decl *decl) {

lldb/unittests/Symbol/TestClangASTImporter.cpp

TEST_F(TestClangASTImporter, RecordLayout) {
 
   ClangASTImporter importer;
   ClangASTImporter::LayoutInfo layout_info;
   layout_info.bit_size = 15;
   layout_info.alignment = 2;
   layout_info.field_offsets[field] = 1;
   importer.InsertRecordDecl(source_record, layout_info);
 
+  // Check that asking for the layout multiple times will always return the
+  // same result. Three times should be enough to find any changing
+  for (unsigned i = 0; i < 3U; ++i) {
+    SCOPED_TRACE("Iteration: " + std::to_string(i));
+
     uint64_t bit_size;
     uint64_t alignment;
     llvm::DenseMap<const clang::FieldDecl *, uint64_t> field_offsets;
     llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits> base_offsets;
-  llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits> vbase_offsets;
+    llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits>
+        vbase_offsets;
     importer.LayoutRecordType(source_record, bit_size, alignment, field_offsets,
                               base_offsets, vbase_offsets);
 
     EXPECT_EQ(15U, bit_size);
     EXPECT_EQ(2U, alignment);
     EXPECT_EQ(1U, field_offsets.size());
     EXPECT_EQ(1U, field_offsets[field]);
     EXPECT_EQ(0U, base_offsets.size());
     EXPECT_EQ(0U, vbase_offsets.size());
   }
+}