[Lldb-commits] [lldb] 85bcc1e - [lldb] Make SBType::IsTypeComplete more consistent by forcing the loading of definitions

Raphael Isemann via lldb-commits lldb-commits at lists.llvm.org
Sat Oct 30 04:28:55 PDT 2021


Author: Raphael Isemann
Date: 2021-10-30T13:28:27+02:00
New Revision: 85bcc1eb2f56af00123a8d6b99e9ad677613767c

URL: https://github.com/llvm/llvm-project/commit/85bcc1eb2f56af00123a8d6b99e9ad677613767c
DIFF: https://github.com/llvm/llvm-project/commit/85bcc1eb2f56af00123a8d6b99e9ad677613767c.diff

LOG: [lldb] Make SBType::IsTypeComplete more consistent by forcing the loading of definitions

Currently calling SBType::IsTypeComplete returns true for record types if and
only if the underlying record in our internal Clang AST has a definition.

The function however doesn't actually force the loading of any external
definition from debug info, so it currently can return false even if the type is
actually defined in a program's debug info but LLDB hasn't lazily created the
definition yet.

This patch changes the behaviour to always load the definition first so that
IsTypeComplete now consistently returns true if there is a definition in the
module/target.

The motivation for this patch is twofold:

* The API is now arguably more useful for the user which don't know or care
about the internal lazy loading mechanism of LLDB.

* With D101950 there is no longer a good way to ask a Decl for a definition
without automatically pulling in a definition from the ExternalASTSource. The
current behaviour doesn't seem useful enough to justify the necessary
workarounds to preserve it for a time after D101950.

Note that there was a test that used this API to test lazy loading of debug info
but that has been replaced with TestLazyLoading by now (which just dumps the
internal Clang AST state instead).

Reviewed By: aprantl

Differential Revision: https://reviews.llvm.org/D112615

Added: 
    lldb/test/API/lang/cpp/complete-type-check/Makefile
    lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
    lldb/test/API/lang/cpp/complete-type-check/main.cpp
    lldb/test/API/lang/objc/complete-type-check/Makefile
    lldb/test/API/lang/objc/complete-type-check/TestObjCIsTypeComplete.py
    lldb/test/API/lang/objc/complete-type-check/main.m

Modified: 
    lldb/bindings/interface/SBType.i
    lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Removed: 
    lldb/test/API/functionalities/type_completion/Makefile
    lldb/test/API/functionalities/type_completion/TestTypeCompletion.py
    lldb/test/API/functionalities/type_completion/main.cpp


################################################################################
diff  --git a/lldb/bindings/interface/SBType.i b/lldb/bindings/interface/SBType.i
index 500bc99ca8cd4..d6e8db3ab4287 100644
--- a/lldb/bindings/interface/SBType.i
+++ b/lldb/bindings/interface/SBType.i
@@ -837,6 +837,21 @@ public:
     lldb::SBTypeMemberFunction
     GetMemberFunctionAtIndex (uint32_t idx);
 
+    %feature("docstring",
+    "Returns true if the type is completely defined.
+
+    Language-specific behaviour:
+
+    * C: Returns false for struct types that were only forward declared in the
+      type's `SBTarget`/`SBModule`. Otherwise returns true.
+    * C++: Returns false for template/non-template struct/class types and
+      scoped enums that were only forward declared inside the type's
+      `SBTarget`/`SBModule`. Otherwise returns true.
+    * Objective-C: Follows the same behavior as C for struct types. Objective-C
+      classes are considered complete unless they were only forward declared via
+      ``@class ClassName`` in the type's `SBTarget`/`SBModule`. Otherwise
+      returns true.
+    ") IsTypeComplete;
     bool
     IsTypeComplete ();
 

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index aa0a8086cb28c..9beccf30f94be 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2940,7 +2940,12 @@ bool TypeSystemClang::IsCharType(lldb::opaque_compiler_type_t type) {
 }
 
 bool TypeSystemClang::IsCompleteType(lldb::opaque_compiler_type_t type) {
-  const bool allow_completion = false;
+  // If the type hasn't been lazily completed yet, complete it now so that we
+  // can give the caller an accurate answer whether the type actually has a
+  // definition. Without completing the type now we would just tell the user
+  // the current (internal) completeness state of the type and most users don't
+  // care (or even know) about this behavior.
+  const bool allow_completion = true;
   return GetCompleteQualType(&getASTContext(), GetQualType(type),
                              allow_completion);
 }

diff  --git a/lldb/test/API/functionalities/type_completion/TestTypeCompletion.py b/lldb/test/API/functionalities/type_completion/TestTypeCompletion.py
deleted file mode 100644
index fb0b1096f4d4b..0000000000000
--- a/lldb/test/API/functionalities/type_completion/TestTypeCompletion.py
+++ /dev/null
@@ -1,155 +0,0 @@
-"""
-Check that types only get completed when necessary.
-"""
-
-
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class TypeCompletionTestCase(TestBase):
-
-    mydir = TestBase.compute_mydir(__file__)
-
-    @expectedFailureAll(
-        compiler="icc",
-        bugnumber="often fails with 'NameAndAddress should be valid.")
-    # Fails with gcc 4.8.1 with llvm.org/pr15301 LLDB prints incorrect sizes
-    # of STL containers
-    def test_with_run_command(self):
-        """Check that types only get completed when necessary."""
-        self.build()
-        self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
-
-        lldbutil.run_break_set_by_source_regexp(
-            self, "// Set break point at this line.")
-
-        self.runCmd("run", RUN_SUCCEEDED)
-
-        # The stop reason of the thread should be breakpoint.
-        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-                    substrs=['stopped',
-                             'stop reason = breakpoint'])
-
-        # This is the function to remove the custom formats in order to have a
-        # clean slate for the next test case.
-        def cleanup():
-            self.runCmd('type category enable -l c++', check=False)
-
-        self.runCmd('type category disable -l c++', check=False)
-
-        # Execute the cleanup function during test case tear down.
-        self.addTearDownHook(cleanup)
-
-        p_vector = self.dbg.GetSelectedTarget().GetProcess(
-        ).GetSelectedThread().GetSelectedFrame().FindVariable('p')
-        p_type = p_vector.GetType()
-        self.assertFalse(
-            p_type.IsTypeComplete(),
-            'vector<T> complete but it should not be')
-
-        self.runCmd("continue")
-
-        p_vector = self.dbg.GetSelectedTarget().GetProcess(
-        ).GetSelectedThread().GetSelectedFrame().FindVariable('p')
-        p_type = p_vector.GetType()
-        self.assertFalse(
-            p_type.IsTypeComplete(),
-            'vector<T> complete but it should not be')
-
-        self.runCmd("continue")
-
-        self.runCmd("frame variable p --show-types")
-
-        p_vector = self.dbg.GetSelectedTarget().GetProcess(
-        ).GetSelectedThread().GetSelectedFrame().FindVariable('p')
-        p_type = p_vector.GetType()
-        self.assertTrue(
-            p_type.IsTypeComplete(),
-            'vector<T> should now be complete')
-        name_address_type = p_type.GetTemplateArgumentType(0)
-        self.assertTrue(
-            name_address_type.IsValid(),
-            'NameAndAddress should be valid')
-        self.assertFalse(
-            name_address_type.IsTypeComplete(),
-            'NameAndAddress complete but it should not be')
-
-        self.runCmd("continue")
-
-        self.runCmd("frame variable guy --show-types")
-
-        p_vector = self.dbg.GetSelectedTarget().GetProcess(
-        ).GetSelectedThread().GetSelectedFrame().FindVariable('p')
-        p_type = p_vector.GetType()
-        self.assertTrue(
-            p_type.IsTypeComplete(),
-            'vector<T> should now be complete')
-        name_address_type = p_type.GetTemplateArgumentType(0)
-        self.assertTrue(
-            name_address_type.IsValid(),
-            'NameAndAddress should be valid')
-        self.assertTrue(
-            name_address_type.IsTypeComplete(),
-            'NameAndAddress should now be complete')
-        field0 = name_address_type.GetFieldAtIndex(0)
-        self.assertTrue(
-            field0.IsValid(),
-            'NameAndAddress::m_name should be valid')
-        string = field0.GetType().GetPointeeType()
-        self.assertTrue(string.IsValid(), 'CustomString should be valid')
-        self.assertFalse(string.IsTypeComplete(),
-                         'CustomString complete but it should not be')
-
-        self.runCmd("continue")
-
-        p_vector = self.dbg.GetSelectedTarget().GetProcess(
-        ).GetSelectedThread().GetSelectedFrame().FindVariable('p')
-        p_type = p_vector.GetType()
-        self.assertTrue(
-            p_type.IsTypeComplete(),
-            'vector<T> should now be complete')
-        name_address_type = p_type.GetTemplateArgumentType(0)
-        self.assertTrue(
-            name_address_type.IsValid(),
-            'NameAndAddress should be valid')
-        self.assertTrue(
-            name_address_type.IsTypeComplete(),
-            'NameAndAddress should now be complete')
-        field0 = name_address_type.GetFieldAtIndex(0)
-        self.assertTrue(
-            field0.IsValid(),
-            'NameAndAddress::m_name should be valid')
-        string = field0.GetType().GetPointeeType()
-        self.assertTrue(string.IsValid(), 'CustomString should be valid')
-        self.assertFalse(string.IsTypeComplete(),
-                         'CustomString complete but it should not be')
-
-        self.runCmd('type category enable -l c++', check=False)
-        self.runCmd('frame variable guy --show-types --ptr-depth=1')
-
-        p_vector = self.dbg.GetSelectedTarget().GetProcess(
-        ).GetSelectedThread().GetSelectedFrame().FindVariable('p')
-        p_type = p_vector.GetType()
-        self.assertTrue(
-            p_type.IsTypeComplete(),
-            'vector<T> should now be complete')
-        name_address_type = p_type.GetTemplateArgumentType(0)
-        self.assertTrue(
-            name_address_type.IsValid(),
-            'NameAndAddress should be valid')
-        self.assertTrue(
-            name_address_type.IsTypeComplete(),
-            'NameAndAddress should now be complete')
-        field0 = name_address_type.GetFieldAtIndex(0)
-        self.assertTrue(
-            field0.IsValid(),
-            'NameAndAddress::m_name should be valid')
-        string = field0.GetType().GetPointeeType()
-        self.assertTrue(string.IsValid(), 'CustomString should be valid')
-        self.assertTrue(
-            string.IsTypeComplete(),
-            'CustomString should now be complete')

diff  --git a/lldb/test/API/functionalities/type_completion/main.cpp b/lldb/test/API/functionalities/type_completion/main.cpp
deleted file mode 100644
index 2154d5805c97f..0000000000000
--- a/lldb/test/API/functionalities/type_completion/main.cpp
+++ /dev/null
@@ -1,72 +0,0 @@
-#include <string.h>
-#include <vector>
-#include <iostream>
-
-class CustomString
-{
-public:
-  CustomString (const char* buffer) :
-    m_buffer(nullptr)
-  {
-    if (buffer)
-    {
-      auto l = strlen(buffer);
-      m_buffer = new char[1 + l];
-      strcpy(m_buffer, buffer);
-    }
-  }
-  
-  ~CustomString ()
-  {
-    delete[] m_buffer;
-  }
-  
-  const char*
-  GetBuffer ()
-  {
-    return m_buffer;
-  }
-  
-private:
-  char *m_buffer;
-};
-
-class NameAndAddress
-	{
-	public:
-		CustomString& GetName() { return *m_name; }
-		CustomString& GetAddress() { return *m_address; }
-		NameAndAddress(const char* N, const char* A) : m_name(new CustomString(N)), m_address(new CustomString(A))
-		{
-		}
-		~NameAndAddress()
-		{
-		}
-		
-	private:
-		CustomString* m_name;
-		CustomString* m_address;
-};
-
-typedef std::vector<NameAndAddress> People;
-
-int main (int argc, const char * argv[])
-{
-	People p;
-	p.push_back(NameAndAddress("Enrico","123 Main Street"));
-	p.push_back(NameAndAddress("Foo","10710 Johnson Avenue")); // Set break point at this line.
-	p.push_back(NameAndAddress("Arpia","6956 Florey Street"));
-	p.push_back(NameAndAddress("Apple","1 Infinite Loop")); // Set break point at this line.
-	p.push_back(NameAndAddress("Richard","9500 Gilman Drive"));
-	p.push_back(NameAndAddress("Bar","3213 Windsor Rd"));
-
-	for (int j = 0; j<p.size(); j++)
-	{
-		NameAndAddress guy = p[j];
-		std::cout << "Person " << j << " is named " << guy.GetName().GetBuffer() << " and lives at " << guy.GetAddress().GetBuffer() << std::endl; // Set break point at this line.
-	}
-
-	return 0;
-	
-}
-

diff  --git a/lldb/test/API/functionalities/type_completion/Makefile b/lldb/test/API/lang/cpp/complete-type-check/Makefile
similarity index 100%
rename from lldb/test/API/functionalities/type_completion/Makefile
rename to lldb/test/API/lang/cpp/complete-type-check/Makefile

diff  --git a/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py b/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
new file mode 100644
index 0000000000000..1059e03878010
--- /dev/null
+++ b/lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -0,0 +1,80 @@
+""" Tests SBType.IsTypeComplete on C++ types. """
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def assertComplete(self, typename):
+        """ Asserts that the type with the given name is complete. """
+        found_type = self.target().FindFirstType(typename)
+        self.assertTrue(found_type.IsValid())
+        self.assertTrue(found_type.IsTypeComplete())
+
+    def assertCompleteWithVar(self, typename):
+        """ Asserts that the type with the given name is complete. """
+        found_type = self.target().FindFirstType(typename)
+        self.assertTrue(found_type.IsValid())
+        self.assertTrue(found_type.IsTypeComplete())
+
+    def assertPointeeIncomplete(self, typename, variable):
+        """ Asserts that the pointee type behind the type with the given name
+        is not complete. The variable is used to find the type."""
+        found_type = self.target().FindFirstType(typename)
+        found_type = self.expect_expr(variable, result_type=typename).GetType()
+        self.assertTrue(found_type.IsPointerType())
+        pointee_type = found_type.GetPointeeType()
+        self.assertTrue(pointee_type.IsValid())
+        self.assertFalse(pointee_type.IsTypeComplete())
+
+    @no_debug_info_test
+    def test_forward_declarations(self):
+        """ Tests types of declarations that can be forward declared. """
+        self.build()
+        self.createTestTarget()
+
+        # Check record types with a definition.
+        self.assertCompleteWithVar("EmptyClass")
+        self.assertCompleteWithVar("DefinedClass")
+        self.assertCompleteWithVar("DefinedClassTypedef")
+        self.assertCompleteWithVar("DefinedTemplateClass<int>")
+
+        # Record types without a defining declaration are not complete.
+        self.assertPointeeIncomplete("FwdClass *", "fwd_class")
+        self.assertPointeeIncomplete("FwdClassTypedef *", "fwd_class_typedef")
+        self.assertPointeeIncomplete("FwdTemplateClass<> *", "fwd_template_class")
+
+        # A pointer type is complete even when it points to an incomplete type.
+        fwd_class_ptr = self.expect_expr("fwd_class", result_type="FwdClass *")
+        self.assertTrue(fwd_class_ptr.GetType().IsTypeComplete())
+
+    @no_debug_info_test
+    def test_builtin_types(self):
+        """ Tests builtin types and types derived from them. """
+        self.build()
+        self.createTestTarget()
+
+        # Void is complete.
+        void_type = self.target().FindFirstType("void")
+        self.assertTrue(void_type.IsValid())
+        self.assertTrue(void_type.IsTypeComplete())
+
+        # Builtin types are also complete.
+        int_type = self.target().FindFirstType("int")
+        self.assertTrue(int_type.IsValid())
+        self.assertTrue(int_type.IsTypeComplete())
+
+        # References to builtin types are also complete.
+        int_ref_type = int_type.GetReferenceType()
+        self.assertTrue(int_ref_type.IsValid())
+        self.assertTrue(int_ref_type.IsTypeComplete())
+
+        # Pointer types to basic types are always complete.
+        int_ptr_type = int_type.GetReferenceType()
+        self.assertTrue(int_ptr_type.IsValid())
+        self.assertTrue(int_ptr_type.IsTypeComplete())

diff  --git a/lldb/test/API/lang/cpp/complete-type-check/main.cpp b/lldb/test/API/lang/cpp/complete-type-check/main.cpp
new file mode 100644
index 0000000000000..307718ce74e55
--- /dev/null
+++ b/lldb/test/API/lang/cpp/complete-type-check/main.cpp
@@ -0,0 +1,36 @@
+struct EmptyClass {};
+struct DefinedClass {
+  int i;
+};
+typedef DefinedClass DefinedClassTypedef;
+
+struct FwdClass;
+typedef FwdClass FwdClassTypedef;
+
+template <typename T> struct DefinedTemplateClass {};
+template <> struct DefinedTemplateClass<int> {};
+
+template <typename T> struct FwdTemplateClass;
+template <> struct FwdTemplateClass<int>;
+
+enum class EnumClassFwd;
+
+enum DefinedEnum { Case1 };
+enum DefinedEnumClass { Case2 };
+
+EmptyClass empty_class;
+DefinedClass defined_class;
+DefinedClassTypedef defined_class_typedef;
+
+FwdClass *fwd_class;
+FwdClassTypedef *fwd_class_typedef;
+
+DefinedTemplateClass<int> defined_template_class;
+FwdTemplateClass<int> *fwd_template_class;
+
+EnumClassFwd *fwd_enum_class = nullptr;
+
+DefinedEnum defined_enum = Case1;
+DefinedEnumClass defined_enum_class = DefinedEnumClass::Case2;
+
+int main() {}

diff  --git a/lldb/test/API/lang/objc/complete-type-check/Makefile b/lldb/test/API/lang/objc/complete-type-check/Makefile
new file mode 100644
index 0000000000000..2f74200fd6f11
--- /dev/null
+++ b/lldb/test/API/lang/objc/complete-type-check/Makefile
@@ -0,0 +1,5 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS := -framework Foundation
+CFLAGS_EXTRAS := -fobjc-arc
+
+include Makefile.rules

diff  --git a/lldb/test/API/lang/objc/complete-type-check/TestObjCIsTypeComplete.py b/lldb/test/API/lang/objc/complete-type-check/TestObjCIsTypeComplete.py
new file mode 100644
index 0000000000000..7820d4cb80616
--- /dev/null
+++ b/lldb/test/API/lang/objc/complete-type-check/TestObjCIsTypeComplete.py
@@ -0,0 +1,39 @@
+""" Tests SBType.IsTypeComplete on Objective-C types. """
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @skipUnlessDarwin
+    @no_debug_info_test
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.m"))
+
+        # A class that is only forward declared is not complete.
+        incomplete = self.expect_expr("incomplete", result_type="IncompleteClass *")
+        self.assertTrue(incomplete.IsValid())
+        incomplete_class = incomplete.GetType().GetPointeeType()
+        self.assertTrue(incomplete_class.IsValid())
+        self.assertFalse(incomplete_class.IsTypeComplete())
+
+        # A class that has its interface fully declared is complete.
+        complete = self.expect_expr("complete", result_type="CompleteClass *")
+        self.assertTrue(complete.IsValid())
+        complete_class = complete.GetType().GetPointeeType()
+        self.assertTrue(complete_class.IsValid())
+        self.assertTrue(complete_class.IsTypeComplete())
+
+        # A class that has its interface fully declared and an implementation
+        # is also complete.
+        complete_with_impl = self.expect_expr("complete_impl",
+            result_type="CompleteClassWithImpl *")
+        self.assertTrue(complete_with_impl.IsValid())
+        complete_class_with_impl = complete_with_impl.GetType().GetPointeeType()
+        self.assertTrue(complete_class_with_impl.IsValid())
+        self.assertTrue(complete_class_with_impl.IsTypeComplete())

diff  --git a/lldb/test/API/lang/objc/complete-type-check/main.m b/lldb/test/API/lang/objc/complete-type-check/main.m
new file mode 100644
index 0000000000000..eb7b8a0fcd5c8
--- /dev/null
+++ b/lldb/test/API/lang/objc/complete-type-check/main.m
@@ -0,0 +1,19 @@
+#include <Foundation/Foundation.h>
+
+ at class IncompleteClass;
+
+ at interface CompleteClass : NSObject
+ at end
+
+ at interface CompleteClassWithImpl : NSObject
+ at end
+ at implementation CompleteClassWithImpl
+ at end
+
+IncompleteClass *incomplete = 0;
+CompleteClass *complete = 0;
+CompleteClassWithImpl *complete_impl = 0;
+
+int main() {
+  return 0; // break here
+}


        


More information about the lldb-commits mailing list