-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb] Use early returns in TypeSystemClang::IsPossibleDynamicType #137630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Remove a couple of levels of indentation before I need to add another one to fix a bug.
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesRemove a couple of levels of indentation before I need to add another one to fix a bug. Full diff: https://github.com/llvm/llvm-project/pull/137630.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 59292f4b24af3..0dd9d8f75504e 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -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();
+ else if (GetType(pointee_qual_type).GetCompleteType())
+ success = cxx_record_decl->isDynamicClass();
+ else
+ success = false;
- case clang::Type::ObjCObject:
- case clang::Type::ObjCInterface:
- if (check_objc) {
- if (dynamic_pointee_type)
- dynamic_pointee_type->SetCompilerType(
- weak_from_this(), pointee_qual_type.getAsOpaquePtr());
- return true;
- }
- break;
+ if (success)
+ set_dynamic_pointee_type(pointee_qual_type);
+ return success;
+ }
- default:
- break;
- }
+ case clang::Type::ObjCObject:
+ case clang::Type::ObjCInterface:
+ if (check_objc) {
+ set_dynamic_pointee_type(pointee_qual_type);
+ return true;
}
+ break;
+
+ default:
+ break;
}
- if (dynamic_pointee_type)
- dynamic_pointee_type->Clear();
return false;
}
|
else if (std::optional<ClangASTMetadata> metadata = | ||
GetMetadata(cxx_record_decl)) | ||
success = metadata->GetIsDynamicCXXType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the problematic part -- since we stopped searching for definitions eagerly, ClangASTMetadata::GetIsDynamicCXXType
does not (always) hold the correct answer to this question.
…lvm#137630) Remove a couple of levels of indentation before I need to add another one to fix a bug.
…lvm#137630) Remove a couple of levels of indentation before I need to add another one to fix a bug.
…lvm#137630) Remove a couple of levels of indentation before I need to add another one to fix a bug.
…lvm#137630) Remove a couple of levels of indentation before I need to add another one to fix a bug.
…lvm#137630) Remove a couple of levels of indentation before I need to add another one to fix a bug.
…lvm#137630) Remove a couple of levels of indentation before I need to add another one to fix a bug.
Remove a couple of levels of indentation before I need to add another one to fix a bug.