Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[lldb][TypeSystemClang][NFC] Clean up TypeSystemClang::GetBitSize #100674

Merged
merged 4 commits into from
Jul 26, 2024
Merged

[lldb][TypeSystemClang][NFC] Clean up TypeSystemClang::GetBitSize #100674

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0f98205
[lldb][TypeSystemClang][NFC] Clean up TypeSystemClang::GetBitSize
Michael137 Jul 26, 2024
b4a595b
fixup! reduce scope of variable
Michael137 Jul 26, 2024
a031b2e
fixup! uint32_t -> uint64_t
Michael137 Jul 26, 2024
2212ebd
fixup! reduce scope of process variable
Michael137 Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 58 additions & 56 deletions lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4725,67 +4725,69 @@ TypeSystemClang::GetFloatTypeSemantics(size_t byte_size) {
return llvm::APFloatBase::Bogus();
}

std::optional<uint64_t>
TypeSystemClang::GetObjCBitSize(QualType qual_type,
ExecutionContextScope *exe_scope) {
assert(qual_type->isObjCObjectOrInterfaceType());
ExecutionContext exe_ctx(exe_scope);
Process *process = exe_ctx.GetProcessPtr();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: you could move this into the if right below.

if (process) {
if (ObjCLanguageRuntime *objc_runtime =
ObjCLanguageRuntime::Get(*process)) {
if (std::optional<uint64_t> bit_size =
objc_runtime->GetTypeBitSize(GetType(qual_type)))
return *bit_size;
}
} else {
static bool g_printed = false;
if (!g_printed) {
StreamString s;
DumpTypeDescription(qual_type.getAsOpaquePtr(), s);

llvm::outs() << "warning: trying to determine the size of type ";
llvm::outs() << s.GetString() << "\n";
llvm::outs() << "without a valid ExecutionContext. this is not "
"reliable. please file a bug against LLDB.\n";
llvm::outs() << "backtrace:\n";
llvm::sys::PrintStackTrace(llvm::outs());
llvm::outs() << "\n";
Comment on lines +4746 to +4752
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than printing to stdout, this should use the global diagnostics (Debugger::ReportWarning). The function takes an optional std::once_flag to do the same thing you're doing here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I was really tempted to just outright remove this tbh

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind if I keep this as-is for now? To keep this change as NFC as possible? (I'm just moving this code around)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine as long as it gets either fixed or removed eventually.

g_printed = true;
}
}

return getASTContext().getTypeSize(qual_type) +
getASTContext().getTypeSize(getASTContext().ObjCBuiltinClassTy);
}

std::optional<uint64_t>
TypeSystemClang::GetBitSize(lldb::opaque_compiler_type_t type,
ExecutionContextScope *exe_scope) {
if (GetCompleteType(type)) {
clang::QualType qual_type(GetCanonicalQualType(type));
const clang::Type::TypeClass type_class = qual_type->getTypeClass();
switch (type_class) {
case clang::Type::Record:
if (GetCompleteType(type))
return getASTContext().getTypeSize(qual_type);
else
return std::nullopt;
break;
if (!GetCompleteType(type))
return std::nullopt;

case clang::Type::ObjCInterface:
case clang::Type::ObjCObject: {
ExecutionContext exe_ctx(exe_scope);
Process *process = exe_ctx.GetProcessPtr();
if (process) {
if (ObjCLanguageRuntime *objc_runtime =
ObjCLanguageRuntime::Get(*process)) {
if (std::optional<uint64_t> bit_size =
objc_runtime->GetTypeBitSize(GetType(qual_type)))
return *bit_size;
}
} else {
static bool g_printed = false;
if (!g_printed) {
StreamString s;
DumpTypeDescription(type, s);

llvm::outs() << "warning: trying to determine the size of type ";
llvm::outs() << s.GetString() << "\n";
llvm::outs() << "without a valid ExecutionContext. this is not "
"reliable. please file a bug against LLDB.\n";
llvm::outs() << "backtrace:\n";
llvm::sys::PrintStackTrace(llvm::outs());
llvm::outs() << "\n";
g_printed = true;
}
}
}
[[fallthrough]];
default:
const uint32_t bit_size = getASTContext().getTypeSize(qual_type);
if (bit_size == 0) {
if (qual_type->isIncompleteArrayType())
return getASTContext().getTypeSize(
qual_type->getArrayElementTypeNoTypeQual()
->getCanonicalTypeUnqualified());
}
if (qual_type->isObjCObjectOrInterfaceType())
return bit_size +
getASTContext().getTypeSize(getASTContext().ObjCBuiltinClassTy);
// Function types actually have a size of 0, that's not an error.
if (qual_type->isFunctionProtoType())
return bit_size;
if (bit_size)
return bit_size;
}
clang::QualType qual_type(GetCanonicalQualType(type));
const clang::Type::TypeClass type_class = qual_type->getTypeClass();
switch (type_class) {
case clang::Type::FunctionProto:
case clang::Type::Record:
return getASTContext().getTypeSize(qual_type);
case clang::Type::ObjCInterface:
case clang::Type::ObjCObject:
return GetObjCBitSize(qual_type, exe_scope);
case clang::Type::IncompleteArray: {
const uint64_t bit_size = getASTContext().getTypeSize(qual_type);
if (bit_size == 0)
return getASTContext().getTypeSize(
qual_type->getArrayElementTypeNoTypeQual()
->getCanonicalTypeUnqualified());

return bit_size;
}
default:
if (const uint64_t bit_size = getASTContext().getTypeSize(qual_type))
return bit_size;
}

return std::nullopt;
}

Expand Down
3 changes: 3 additions & 0 deletions lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1172,6 +1172,9 @@ class TypeSystemClang : public TypeSystem {
/// on creation of a new instance.
void LogCreation() const;

std::optional<uint64_t> GetObjCBitSize(clang::QualType qual_type,
ExecutionContextScope *exe_scope);

// Classes that inherit from TypeSystemClang can see and modify these
std::string m_target_triple;
std::unique_ptr<clang::ASTContext> m_ast_up;
Expand Down
Loading