teemperor created this revision.
teemperor added a reviewer: shafik.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.
teemperor removed reviewers: martong, a.sidorin.
Herald added a reviewer: a.sidorin.
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.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D71603
Files:
lldb/include/lldb/Symbol/ClangASTImporter.h
lldb/source/Symbol/ClangASTImporter.cpp
lldb/unittests/Symbol/TestClangASTImporter.cpp
Index: lldb/unittests/Symbol/TestClangASTImporter.cpp
===================================================================
--- lldb/unittests/Symbol/TestClangASTImporter.cpp
+++ lldb/unittests/Symbol/TestClangASTImporter.cpp
@@ -203,18 +203,25 @@
layout_info.field_offsets[field] = 1;
importer.InsertRecordDecl(source_record, layout_info);
- 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;
- 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());
+ // 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;
+ 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());
+ }
}
Index: lldb/source/Symbol/ClangASTImporter.cpp
===================================================================
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -525,26 +525,23 @@
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();
+ field_offsets = pos->second.field_offsets;
+ base_offsets = pos->second.base_offsets;
+ vbase_offsets = pos->second.vbase_offsets;
+ return true;
}
- return success;
+ bit_size = 0;
+ alignment = 0;
+ field_offsets.clear();
+ return false;
}
void ClangASTImporter::InsertRecordDecl(clang::RecordDecl *decl,
Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===================================================================
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -67,7 +67,7 @@
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);
Index: lldb/unittests/Symbol/TestClangASTImporter.cpp
===================================================================
--- lldb/unittests/Symbol/TestClangASTImporter.cpp
+++ lldb/unittests/Symbol/TestClangASTImporter.cpp
@@ -203,18 +203,25 @@
layout_info.field_offsets[field] = 1;
importer.InsertRecordDecl(source_record, layout_info);
- 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;
- 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());
+ // 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;
+ 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());
+ }
}
Index: lldb/source/Symbol/ClangASTImporter.cpp
===================================================================
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -525,26 +525,23 @@
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();
+ field_offsets = pos->second.field_offsets;
+ base_offsets = pos->second.base_offsets;
+ vbase_offsets = pos->second.vbase_offsets;
+ return true;
}
- return success;
+ bit_size = 0;
+ alignment = 0;
+ field_offsets.clear();
+ return false;
}
void ClangASTImporter::InsertRecordDecl(clang::RecordDecl *decl,
Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===================================================================
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -67,7 +67,7 @@
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);
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
