Please use GitHub pull requests for new patches. Phabricator shutdown timeline
Changeset View
Changeset View
Standalone View
Standalone View
source/Core/ValueObjectRegister.cpp
| Show All 12 Lines | |||||
| #include "lldb/Symbol/CompilerType.h" | #include "lldb/Symbol/CompilerType.h" | ||||
| #include "lldb/Symbol/TypeSystem.h" | #include "lldb/Symbol/TypeSystem.h" | ||||
| #include "lldb/Target/ExecutionContext.h" | #include "lldb/Target/ExecutionContext.h" | ||||
| #include "lldb/Target/Process.h" | #include "lldb/Target/Process.h" | ||||
| #include "lldb/Target/RegisterContext.h" | #include "lldb/Target/RegisterContext.h" | ||||
| #include "lldb/Target/StackFrame.h" | #include "lldb/Target/StackFrame.h" | ||||
| #include "lldb/Target/Target.h" | #include "lldb/Target/Target.h" | ||||
| #include "lldb/Utility/DataExtractor.h" | #include "lldb/Utility/DataExtractor.h" | ||||
| #include "lldb/Utility/Log.h" | |||||
| #include "lldb/Utility/Scalar.h" | #include "lldb/Utility/Scalar.h" | ||||
| #include "lldb/Utility/Status.h" | #include "lldb/Utility/Status.h" | ||||
| #include "lldb/Utility/Stream.h" | #include "lldb/Utility/Stream.h" | ||||
| #include "llvm/ADT/StringRef.h" | #include "llvm/ADT/StringRef.h" | ||||
| #include <assert.h> | #include <assert.h> | ||||
| #include <memory> | #include <memory> | ||||
| ▲ Show 20 Lines • Show All 222 Lines • ▼ Show 20 Lines | ValueObjectRegister::ValueObjectRegister(ExecutionContextScope *exe_scope, | ||||
| ConstructObject(reg_num); | ConstructObject(reg_num); | ||||
| } | } | ||||
| ValueObjectRegister::~ValueObjectRegister() {} | ValueObjectRegister::~ValueObjectRegister() {} | ||||
| CompilerType ValueObjectRegister::GetCompilerTypeImpl() { | CompilerType ValueObjectRegister::GetCompilerTypeImpl() { | ||||
| if (!m_compiler_type.IsValid()) { | if (!m_compiler_type.IsValid()) { | ||||
| ExecutionContext exe_ctx(GetExecutionContextRef()); | ExecutionContext exe_ctx(GetExecutionContextRef()); | ||||
| Target *target = exe_ctx.GetTargetPtr(); | if (auto *target = exe_ctx.GetTargetPtr()) { | ||||
| if (target) { | if (auto *exe_module = target->GetExecutableModulePointer()) { | ||||
| Module *exe_module = target->GetExecutableModulePointer(); | auto type_system_or_err = | ||||
| if (exe_module) { | |||||
| TypeSystem *type_system = | |||||
| exe_module->GetTypeSystemForLanguage(eLanguageTypeC); | exe_module->GetTypeSystemForLanguage(eLanguageTypeC); | ||||
| if (type_system) | if (auto err = type_system_or_err.takeError()) { | ||||
| m_compiler_type = type_system->GetBuiltinTypeForEncodingAndBitSize( | LLDB_LOG_ERROR( | ||||
JDevlieghere: As a general note: I think it's good practice to use `llvm::consumeError` when discarding the… | |||||
Not Done ReplyInline ActionsThis is not just good practice, it is actually required. An simply checking the bool-ness of the Expected object will not set the "checked" flag of the error object, and it will assert when it is destroyed (if asserts are enabled). labath: This is not just good practice, it is actually required. An simply checking the bool-ness of… | |||||
So what you're saying is do not define it in an if condition, but rather get the Expected and if it isn't valid then we consume the error, else use the type system. Is that right? xiaobai: So what you're saying is do not define it in an if condition, but rather get the Expected and… | |||||
Not Done ReplyInline ActionsYes, although technically, the value can still be defined in the if-condition, as its scope extends into the else clause. So the following code is perfectly valid (if slightly surprising): if (auto expected_foo = maybe_get_foo()) do_stuff(*expected_foo); else log(expected_foo.takeError()); labath: Yes, although technically, the value can still be defined in the if-condition, as its scope… | |||||
| lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_TYPES), | |||||
| std::move(err), "Unable to get CompilerType from TypeSystem"); | |||||
Not Done ReplyInline ActionsIf you returned a reference then you also wouldn't need this extra variable to avoid weird double-deref. labath: If you returned a reference then you also wouldn't need this extra variable to avoid weird… | |||||
| } else { | |||||
| m_compiler_type = | |||||
| type_system_or_err->GetBuiltinTypeForEncodingAndBitSize( | |||||
| m_reg_info.encoding, m_reg_info.byte_size * 8); | m_reg_info.encoding, m_reg_info.byte_size * 8); | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| } | |||||
| return m_compiler_type; | return m_compiler_type; | ||||
| } | } | ||||
| ConstString ValueObjectRegister::GetTypeName() { | ConstString ValueObjectRegister::GetTypeName() { | ||||
| if (m_type_name.IsEmpty()) | if (m_type_name.IsEmpty()) | ||||
| m_type_name = GetCompilerType().GetConstTypeName(); | m_type_name = GetCompilerType().GetConstTypeName(); | ||||
| return m_type_name; | return m_type_name; | ||||
| } | } | ||||
| ▲ Show 20 Lines • Show All 80 Lines • Show Last 20 Lines | |||||
As a general note: I think it's good practice to use llvm::consumeError when discarding the error to make it explicit that it's not handled. This also makes it easier down the line to handle the error, e.g. by logging it.