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

[David Blaikie via lldb-commits]

dblaikie added a subscriber: dblaikie.

================
Comment at: test/lang/cpp/incomplete-types/Makefile:8
@@ +7,3 @@
+
+ifneq (,$(findstring clang,$(CC)))
+  CFLAGS_LIMIT += -flimit-debug-info
----------------
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)

================
Comment at: test/lang/cpp/incomplete-types/TestCppIncompleteTypes.py:5
@@ +4,3 @@
+
+class TestCppIncompleteTypes(TestBase):
+
----------------
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)

================
Comment at: test/lang/cpp/incomplete-types/length.h:4
@@ +3,3 @@
+
+#include <string>
+
----------------
Having a test case depend on the entire header of <string> seems a bit brittle - and means this test may or may not test the behavior you're interested in, depending on how the standard library is defined (depending on whether it actually has an explicit instantiation decl/def for basic_string<char> or not)


http://reviews.llvm.org/D13066