[Lldb-commits] [PATCH] Don't crash if a member declaration is incomplete

[David Blaikie]

REPOSITORY
  rL LLVM

================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2182
@@ +2181,3 @@
+                                {
+                                    GetObjectFile()->GetModule()->ReportError ("0x%8.8" PRIx64 "  DW_TAG_member '%s' refers to type '%s' that is a forward declaration, not a complete definition."
+                                                                               "\nThis can happen due to missing images or compiler bugs.",
----------------
Is this the same error text (modulo "member" rather than "inheritance") as the inheritance case that was already handled? Should the message go in one place rather than being duplicated?

================
Comment at: source/Symbol/ClangASTType.cpp:5851
@@ -5850,1 +5850,3 @@
         clang::QualType qual_type (GetQualType());
+        clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
+        if (cxx_record_decl)
----------------
clayborg wrote:
> This patch is not needed. CXXRecordDecl is a TagType and it will be handled by the code below. 
I'm guessing this (without context (both in the patch, and as I'm not an LLDB developer) I'm stabbing in the dark) is to address the forward decl as a base class produced by templates case? That wasn't handled, judging by the test case you added (or was that test case just for coverage, even though it was already covered?)?

================
Comment at: test/types/TestForwardTypes.py:33
@@ +32,3 @@
+    def test_forward_inheritance(self):
+        """Test that bases with forwarded debug info don't crash the compiler"""
+        self.source = 'forward_inheritance.cpp'
----------------
Should these tests test for the specific error message/behavior rather than "do anything so long as it's not crashing"?

================
Comment at: test/types/forward_inheritance.cpp:2
@@ +1,3 @@
+template<typename T>
+class foo {
+  void func() { }
----------------
(I'd usually use struct rather than class when access doesn't matter - but... it doesn't matter, so whichever suits)

================
Comment at: test/types/forward_inheritance.cpp:9
@@ +8,3 @@
+
+class bar : public fooint {
+};
----------------
you could drop the 'public' here, it doesn't matter (though, again, would probably use struct rather than class, as well)

================
Comment at: test/types/forward_member.cpp:7
@@ +6,3 @@
+extern template struct foo<int>;
+typedef foo<int> fooint; 
+
----------------
are the typedefs (of foo<int> and of bar) required here? I imagine not (similarly for the forward_inheritance example)

http://reviews.llvm.org/D10509

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/