[Lldb-commits] [lldb] [lldb] Use early returns in TypeSystemClang::IsPossibleDynamicType (PR #137630)

[Pavel Labath via lldb-commits]

================
@@ -3579,129 +3579,108 @@ bool TypeSystemClang::IsPossibleDynamicType(lldb::opaque_compiler_type_t type,
                                             CompilerType *dynamic_pointee_type,
                                             bool check_cplusplus,
                                             bool check_objc) {
+  if (dynamic_pointee_type)
+    dynamic_pointee_type->Clear();
+  if (!type)
+    return false;
+
+  auto set_dynamic_pointee_type = [&](clang::QualType type) {
+    if (dynamic_pointee_type)
+      dynamic_pointee_type->SetCompilerType(weak_from_this(),
+                                            type.getAsOpaquePtr());
+  };
+
   clang::QualType pointee_qual_type;
-  if (type) {
-    clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
-    bool success = false;
-    const clang::Type::TypeClass type_class = qual_type->getTypeClass();
-    switch (type_class) {
-    case clang::Type::Builtin:
-      if (check_objc &&
-          llvm::cast<clang::BuiltinType>(qual_type)->getKind() ==
-              clang::BuiltinType::ObjCId) {
-        if (dynamic_pointee_type)
-          dynamic_pointee_type->SetCompilerType(weak_from_this(), type);
-        return true;
-      }
-      break;
+  clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
+  switch (qual_type->getTypeClass()) {
+  case clang::Type::Builtin:
+    if (check_objc && llvm::cast<clang::BuiltinType>(qual_type)->getKind() ==
+                          clang::BuiltinType::ObjCId) {
+      set_dynamic_pointee_type(qual_type);
+      return true;
+    }
+    return false;
 
-    case clang::Type::ObjCObjectPointer:
-      if (check_objc) {
-        if (const auto *objc_pointee_type =
-                qual_type->getPointeeType().getTypePtrOrNull()) {
-          if (const auto *objc_object_type =
-                  llvm::dyn_cast_or_null<clang::ObjCObjectType>(
-                      objc_pointee_type)) {
-            if (objc_object_type->isObjCClass())
-              return false;
-          }
-        }
-        if (dynamic_pointee_type)
-          dynamic_pointee_type->SetCompilerType(
-              weak_from_this(),
-              llvm::cast<clang::ObjCObjectPointerType>(qual_type)
-                  ->getPointeeType()
-                  .getAsOpaquePtr());
-        return true;
+  case clang::Type::ObjCObjectPointer:
+    if (!check_objc)
+      return false;
+    if (const auto *objc_pointee_type =
+            qual_type->getPointeeType().getTypePtrOrNull()) {
+      if (const auto *objc_object_type =
+              llvm::dyn_cast_or_null<clang::ObjCObjectType>(
+                  objc_pointee_type)) {
+        if (objc_object_type->isObjCClass())
+          return false;
       }
-      break;
+    }
+    set_dynamic_pointee_type(
+        llvm::cast<clang::ObjCObjectPointerType>(qual_type)->getPointeeType());
+    return true;
 
-    case clang::Type::Pointer:
-      pointee_qual_type =
-          llvm::cast<clang::PointerType>(qual_type)->getPointeeType();
-      success = true;
-      break;
+  case clang::Type::Pointer:
+    pointee_qual_type =
+        llvm::cast<clang::PointerType>(qual_type)->getPointeeType();
+    break;
 
-    case clang::Type::LValueReference:
-    case clang::Type::RValueReference:
-      pointee_qual_type =
-          llvm::cast<clang::ReferenceType>(qual_type)->getPointeeType();
-      success = true;
-      break;
+  case clang::Type::LValueReference:
+  case clang::Type::RValueReference:
+    pointee_qual_type =
+        llvm::cast<clang::ReferenceType>(qual_type)->getPointeeType();
+    break;
 
+  default:
+    return false;
+  }
+
+  // Check to make sure what we are pointing to is a possible dynamic C++ type
+  // We currently accept any "void *" (in case we have a class that has been
+  // watered down to an opaque pointer) and virtual C++ classes.
+  switch (pointee_qual_type.getCanonicalType()->getTypeClass()) {
+  case clang::Type::Builtin:
+    switch (llvm::cast<clang::BuiltinType>(pointee_qual_type)->getKind()) {
+    case clang::BuiltinType::UnknownAny:
+    case clang::BuiltinType::Void:
+      set_dynamic_pointee_type(pointee_qual_type);
+      return true;
     default:
-      break;
+      return false;
     }
 
-    if (success) {
-      // Check to make sure what we are pointing too is a possible dynamic C++
-      // type We currently accept any "void *" (in case we have a class that
-      // has been watered down to an opaque pointer) and virtual C++ classes.
-      const clang::Type::TypeClass pointee_type_class =
-          pointee_qual_type.getCanonicalType()->getTypeClass();
-      switch (pointee_type_class) {
-      case clang::Type::Builtin:
-        switch (llvm::cast<clang::BuiltinType>(pointee_qual_type)->getKind()) {
-        case clang::BuiltinType::UnknownAny:
-        case clang::BuiltinType::Void:
-          if (dynamic_pointee_type)
-            dynamic_pointee_type->SetCompilerType(
-                weak_from_this(), pointee_qual_type.getAsOpaquePtr());
-          return true;
-        default:
-          break;
-        }
-        break;
-
-      case clang::Type::Record:
-        if (check_cplusplus) {
-          clang::CXXRecordDecl *cxx_record_decl =
-              pointee_qual_type->getAsCXXRecordDecl();
-          if (cxx_record_decl) {
-            bool is_complete = cxx_record_decl->isCompleteDefinition();
-
-            if (is_complete)
-              success = cxx_record_decl->isDynamicClass();
-            else {
-              if (std::optional<ClangASTMetadata> metadata =
-                      GetMetadata(cxx_record_decl))
-                success = metadata->GetIsDynamicCXXType();
-              else {
-                is_complete = GetType(pointee_qual_type).GetCompleteType();
-                if (is_complete)
-                  success = cxx_record_decl->isDynamicClass();
-                else
-                  success = false;
-              }
-            }
+  case clang::Type::Record: {
+    if (!check_cplusplus)
+      return false;
+    clang::CXXRecordDecl *cxx_record_decl =
+        pointee_qual_type->getAsCXXRecordDecl();
+    if (!cxx_record_decl)
+      return false;
 
-            if (success) {
-              if (dynamic_pointee_type)
-                dynamic_pointee_type->SetCompilerType(
-                    weak_from_this(), pointee_qual_type.getAsOpaquePtr());
-              return true;
-            }
-          }
-        }
-        break;
+    bool success;
+    if (cxx_record_decl->isCompleteDefinition())
+      success = cxx_record_decl->isDynamicClass();
+    else if (std::optional<ClangASTMetadata> metadata =
+                 GetMetadata(cxx_record_decl))
+      success = metadata->GetIsDynamicCXXType();
----------------
labath wrote:

This is the problematic part -- since we stopped searching for definitions eagerly, `ClangASTMetadata::GetIsDynamicCXXType` does not (always) hold the correct answer to this question.

https://github.com/llvm/llvm-project/pull/137630