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.",
----------------
dblaikie wrote:
> 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?
It's a similar error message but it's in a different place (because of where
the bases vs members are constructed. I don't think it's worth moving the error
message until we have a way to actually search the rest of the images).
================
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)
----------------
dblaikie wrote:
> loladiro wrote:
> > clayborg wrote:
> > > This patch is not needed. CXXRecordDecl is a TagType and it will be
> > > handled by the code below.
> > I thought so too, but it crashed without it. I now, realize that all that's
> > required is to use getAsTagDecl below rather than casting through the
> > TagType. That should handle this case as well. Will update.
> 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?)?
This is required for both cases. The code was there, but it doesn't seem like
it ever actually worked because of this.
================
Comment at: source/Symbol/ClangASTType.cpp:5851-5857
@@ -5850,2 +5850,9 @@
clang::QualType qual_type (GetQualType());
+ clang::CXXRecordDecl *cxx_record_decl =
qual_type->getAsCXXRecordDecl();
+ if (cxx_record_decl)
+ {
+ cxx_record_decl->startDefinition();
+ return true;
+ }
+
const clang::Type *t = qual_type.getTypePtr();
----------------
clayborg wrote:
> This patch is not needed. CXXRecordDecl is a TagType and it will be handled
> by the code below.
I thought so too, but it crashed without it. I now, realize that all that's
required is to use getAsTagDecl below rather than casting through the TagType.
That should handle this case as well. Will update.
================
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'
----------------
dblaikie wrote:
> Should these tests test for the specific error message/behavior rather than
> "do anything so long as it's not crashing"?
Yes, but I didn't know how to do that, so I figured I'd put it up and somebody
who know lldb will help out.
================
Comment at: test/types/forward_member.cpp:7
@@ +6,3 @@
+extern template struct foo<int>;
+typedef foo<int> fooint;
+
----------------
dblaikie wrote:
> are the typedefs (of foo<int> and of bar) required here? I imagine not
> (similarly for the forward_inheritance example)
For some reason I had trouble getting it to crash without the typedefs, but
that was before I understood the issue fully, so I'll have another go at it.
http://reviews.llvm.org/D10509
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
