[Lldb-commits] [PATCH] D13066: DWARFASTParserClang::CompleteTypeFromDWARF: Handle incomplete baseclass or child

[Siva Chandra via lldb-commits]

sivachandra added inline comments.

================
Comment at: test/lang/cpp/incomplete-types/Makefile:8
@@ +7,3 @@
+
+ifneq (,$(findstring clang,$(CC)))
+  CFLAGS_LIMIT += -flimit-debug-info
----------------
dblaikie wrote:
> In case it's interesting, you can get similarly problematic DWARF by using a dynamic class (one with virtual functions) with a key function that is not defined in the current translation unit. 
> 
> GCC implements this behavior as well (whereas GCC doesn't implement the behavior that is triggering on basic_string involving explicit instantiation declarations/definitions) and also has a flag for it: -femit-class-debug-always (I think last I recall, Eric Christopher mentioned he'd looked at the GCC implementation of this flag and it differed in some ways from Clang's, so he was reticent to add a compatibility flag for this in Clang... but we could discuss/revisit that, perhaps (though I suppose it wouldn't allow you to /enable/ this optimization, only disable it - not sure if there's a way to opt-in in GCC))
> 
> Though it's easy to "disable" the feature by simply not triggering it, rather than using a flag to turn it off - eg: providing no key function for a type (if you're triggering the dynamic class case, not the template case), or removing an explicit instantiation declaration/definition (if you're triggering the template case)
Thanks for this info. I can remove the use of std::string and make use of a dynamic class to test this behavior. And, if it can be tested with GCC as well, then @skipIfGcc can be removed.

================
Comment at: test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py:5
@@ +4,3 @@
+
+class TestCppIncompleteTypes(TestBase):
+
----------------
dblaikie wrote:
> You don't seem to have a test case for the shared library case in the bug report? (decl in one obj, def in the other, shared library/object/whatever boundary between the two)
> 
> (& not sure what the behavior is in the case where the def is provided in one obj and they are statically linked together - bug report mentions the failure doesn't occur, but I don't know if it does the right thing (actually finds the definition) - I know lldb used to not do the right thing there, and had some error message about the compiler being wrong)
Per my understanding, LLDB does not search the world to complete or lookup a type. So, shared library case will not still work. If the objects are statically linked, it will however.

Making it work in the shared library case is the next step. I treated pr24812 as unrelated to this. I can open it back if you feel it should be kept open until the shared library case is fixed.


http://reviews.llvm.org/D13066