Skip to content

Commit

Permalink
Don't type-erase the SymbolContextItem enumeration.
Browse files Browse the repository at this point in the history 
When we get the `resolve_scope` parameter from the SB API, it's a
`uint32_t`.  We then pass it through all of LLDB this way, as a uint32.
This is unfortunate, because it means the user of an API never actually
knows what they're dealing with.  We can call it something like
`resolve_scope` and have comments saying "this is a value from the
`SymbolContextItem` enumeration, but it makes more sense to just have it
actually *be* the correct type in the actual C++ type system to begin
with.  This way the person reading the code just knows what it is.

The reason to use integers instead of enumerations for flags is because
when you do bitwise operations on enumerations they get promoted to
integers, so it makes it tedious to constantly be casting them back
to the enumeration types, so I've introduced a macro to make this
happen magically.  By writing LLDB_MARK_AS_BITMASK_ENUM after defining
an enumeration, it will define overloaded operators so that the
returned type will be the original enum.  This should address all
the mechanical issues surrounding using rich enum types directly.

This way, we get a better debugger experience, and new users to
the codebase can get more easily acquainted with the codebase because
their IDE features can help them understand what the types mean.

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

llvm-svn: 345313
  • Loading branch information
Zachary Turner committed Oct 25, 2018
1 parent 970f38e commit 991e445
Show file tree
Hide file tree
Showing 37 changed files with 159 additions and 117 deletions.
6 changes: 3 additions & 3 deletions lldb/include/lldb/Core/Address.h
View file
Expand Up @@ -508,9 +508,9 @@ class Address {
///
/// @see SymbolContextScope::CalculateSymbolContext(SymbolContext*)
//------------------------------------------------------------------
uint32_t CalculateSymbolContext(
SymbolContext *sc,
uint32_t resolve_scope = lldb::eSymbolContextEverything) const;
uint32_t CalculateSymbolContext(SymbolContext *sc,
lldb::SymbolContextItem resolve_scope =
lldb::eSymbolContextEverything) const;

lldb::ModuleSP CalculateSymbolContextModule() const;

Expand Down
23 changes: 10 additions & 13 deletions lldb/include/lldb/Core/Module.h
View file
Expand Up @@ -816,10 +816,9 @@ class Module : public std::enable_shared_from_this<Module>,
///
/// @see SymbolContext::Scope
//------------------------------------------------------------------
uint32_t
ResolveSymbolContextForAddress(const Address &so_addr, uint32_t resolve_scope,
SymbolContext &sc,
bool resolve_tail_call_address = false);
uint32_t ResolveSymbolContextForAddress(
const Address &so_addr, lldb::SymbolContextItem resolve_scope,
SymbolContext &sc, bool resolve_tail_call_address = false);

//------------------------------------------------------------------
/// Resolve items in the symbol context for a given file and line.
Expand Down Expand Up @@ -862,10 +861,9 @@ class Module : public std::enable_shared_from_this<Module>,
///
/// @see SymbolContext::Scope
//------------------------------------------------------------------
uint32_t ResolveSymbolContextForFilePath(const char *file_path, uint32_t line,
bool check_inlines,
uint32_t resolve_scope,
SymbolContextList &sc_list);
uint32_t ResolveSymbolContextForFilePath(
const char *file_path, uint32_t line, bool check_inlines,
lldb::SymbolContextItem resolve_scope, SymbolContextList &sc_list);

//------------------------------------------------------------------
/// Resolve items in the symbol context for a given file and line.
Expand Down Expand Up @@ -909,10 +907,9 @@ class Module : public std::enable_shared_from_this<Module>,
///
/// @see SymbolContext::Scope
//------------------------------------------------------------------
uint32_t ResolveSymbolContextsForFileSpec(const FileSpec &file_spec,
uint32_t line, bool check_inlines,
uint32_t resolve_scope,
SymbolContextList &sc_list);
uint32_t ResolveSymbolContextsForFileSpec(
const FileSpec &file_spec, uint32_t line, bool check_inlines,
lldb::SymbolContextItem resolve_scope, SymbolContextList &sc_list);

void SetFileSpecAndObjectName(const FileSpec &file,
const ConstString &object_name);
Expand Down Expand Up @@ -1155,7 +1152,7 @@ class Module : public std::enable_shared_from_this<Module>,
//------------------------------------------------------------------
uint32_t ResolveSymbolContextForAddress(lldb::addr_t vm_addr,
bool vm_addr_is_file_addr,
uint32_t resolve_scope,
lldb::SymbolContextItem resolve_scope,
Address &so_addr, SymbolContext &sc);

void SymbolIndicesToSymbolContextList(Symtab *symtab,
Expand Down
16 changes: 7 additions & 9 deletions lldb/include/lldb/Core/ModuleList.h
View file
Expand Up @@ -495,26 +495,24 @@ class ModuleList {
/// &,uint32_t,SymbolContext&)
//------------------------------------------------------------------
uint32_t ResolveSymbolContextForAddress(const Address &so_addr,
uint32_t resolve_scope,
lldb::SymbolContextItem resolve_scope,
SymbolContext &sc) const;

//------------------------------------------------------------------
/// @copydoc Module::ResolveSymbolContextForFilePath (const char
/// *,uint32_t,bool,uint32_t,SymbolContextList&)
//------------------------------------------------------------------
uint32_t ResolveSymbolContextForFilePath(const char *file_path, uint32_t line,
bool check_inlines,
uint32_t resolve_scope,
SymbolContextList &sc_list) const;
uint32_t ResolveSymbolContextForFilePath(
const char *file_path, uint32_t line, bool check_inlines,
lldb::SymbolContextItem resolve_scope, SymbolContextList &sc_list) const;

//------------------------------------------------------------------
/// @copydoc Module::ResolveSymbolContextsForFileSpec (const FileSpec
/// &,uint32_t,bool,uint32_t,SymbolContextList&)
//------------------------------------------------------------------
uint32_t ResolveSymbolContextsForFileSpec(const FileSpec &file_spec,
uint32_t line, bool check_inlines,
uint32_t resolve_scope,
SymbolContextList &sc_list) const;
uint32_t ResolveSymbolContextsForFileSpec(
const FileSpec &file_spec, uint32_t line, bool check_inlines,
lldb::SymbolContextItem resolve_scope, SymbolContextList &sc_list) const;

//------------------------------------------------------------------
/// Gets the size of the module list.
Expand Down
2 changes: 1 addition & 1 deletion lldb/include/lldb/Symbol/CompileUnit.h
View file
Expand Up @@ -391,7 +391,7 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
//------------------------------------------------------------------
uint32_t ResolveSymbolContext(const FileSpec &file_spec, uint32_t line,
bool check_inlines, bool exact,
uint32_t resolve_scope,
lldb::SymbolContextItem resolve_scope,
SymbolContextList &sc_list);

//------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions lldb/include/lldb/Symbol/SymbolFile.h
View file
Expand Up @@ -155,11 +155,11 @@ class SymbolFile : public PluginInterface {
return CompilerDeclContext();
}
virtual uint32_t ResolveSymbolContext(const Address &so_addr,
uint32_t resolve_scope,
lldb::SymbolContextItem resolve_scope,
SymbolContext &sc) = 0;
virtual uint32_t ResolveSymbolContext(const FileSpec &file_spec,
uint32_t line, bool check_inlines,
uint32_t resolve_scope,
lldb::SymbolContextItem resolve_scope,
SymbolContextList &sc_list);
virtual uint32_t
FindGlobalVariables(const ConstString &name,
Expand Down
4 changes: 2 additions & 2 deletions lldb/include/lldb/Symbol/SymbolVendor.h
View file
Expand Up @@ -71,12 +71,12 @@ class SymbolVendor : public ModuleChild, public PluginInterface {
virtual Type *ResolveTypeUID(lldb::user_id_t type_uid);

virtual uint32_t ResolveSymbolContext(const Address &so_addr,
uint32_t resolve_scope,
lldb::SymbolContextItem resolve_scope,
SymbolContext &sc);

virtual uint32_t ResolveSymbolContext(const FileSpec &file_spec,
uint32_t line, bool check_inlines,
uint32_t resolve_scope,
lldb::SymbolContextItem resolve_scope,
SymbolContextList &sc_list);

virtual size_t FindGlobalVariables(const ConstString &name,
Expand Down
2 changes: 1 addition & 1 deletion lldb/include/lldb/Target/StackFrame.h
View file
Expand Up @@ -173,7 +173,7 @@ class StackFrame : public ExecutionContextScope,
/// A SymbolContext reference which includes the types of information
/// requested by resolve_scope, if they are available.
//------------------------------------------------------------------
const SymbolContext &GetSymbolContext(uint32_t resolve_scope);
const SymbolContext &GetSymbolContext(lldb::SymbolContextItem resolve_scope);

//------------------------------------------------------------------
/// Return the Canonical Frame Address (DWARF term) for this frame.
Expand Down
74 changes: 52 additions & 22 deletions lldb/include/lldb/lldb-enumerations.h
View file
Expand Up @@ -10,6 +10,36 @@
#ifndef LLDB_lldb_enumerations_h_
#define LLDB_lldb_enumerations_h_

#include <type_traits>

// Macro to enable bitmask operations on an enum. Without this, Enum | Enum
// gets promoted to an int, so you have to say Enum a = Enum(eFoo | eBar). If
// you mark Enum with LLDB_MARK_AS_BITMASK_ENUM(Enum), however, you can simply
// write Enum a = eFoo | eBar.
#define LLDB_MARK_AS_BITMASK_ENUM(Enum) \
inline Enum operator|(Enum a, Enum b) { \
return static_cast<Enum>( \
static_cast<std::underlying_type<Enum>::type>(a) | \
static_cast<std::underlying_type<Enum>::type>(b)); \
} \
inline Enum operator&(Enum a, Enum b) { \
return static_cast<Enum>( \
static_cast<std::underlying_type<Enum>::type>(a) & \
static_cast<std::underlying_type<Enum>::type>(b)); \
} \
inline Enum operator~(Enum a) { \
return static_cast<Enum>( \
~static_cast<std::underlying_type<Enum>::type>(a)); \
} \
inline Enum &operator|=(Enum &a, Enum b) { \
a = a | b; \
return a; \
} \
inline Enum &operator&=(Enum &a, Enum b) { \
a = a & b; \
return a; \
}

#ifndef SWIG
// With MSVC, the default type of an enum is always signed, even if one of the
// enumerator values is too large to fit into a signed integer but would
Expand Down Expand Up @@ -327,39 +357,40 @@ enum InputReaderGranularity {
//------------------------------------------------------------------
FLAGS_ENUM(SymbolContextItem){
eSymbolContextTarget = (1u << 0), ///< Set when \a target is requested from
///a query, or was located in query
///results
/// a query, or was located in query
/// results
eSymbolContextModule = (1u << 1), ///< Set when \a module is requested from
///a query, or was located in query
///results
/// a query, or was located in query
/// results
eSymbolContextCompUnit = (1u << 2), ///< Set when \a comp_unit is requested
///from a query, or was located in query
///results
/// from a query, or was located in
/// query results
eSymbolContextFunction = (1u << 3), ///< Set when \a function is requested
///from a query, or was located in query
///results
/// from a query, or was located in
/// query results
eSymbolContextBlock = (1u << 4), ///< Set when the deepest \a block is
///requested from a query, or was located
///in query results
/// requested from a query, or was located
/// in query results
eSymbolContextLineEntry = (1u << 5), ///< Set when \a line_entry is
///requested from a query, or was
///located in query results
/// requested from a query, or was
/// located in query results
eSymbolContextSymbol = (1u << 6), ///< Set when \a symbol is requested from
///a query, or was located in query
///results
/// a query, or was located in query
/// results
eSymbolContextEverything = ((eSymbolContextSymbol << 1) -
1u), ///< Indicates to try and lookup everything
///up during a routine symbol context
///query.
eSymbolContextVariable = (1u << 7) ///< Set when \a global or static
///variable is requested from a query, or
///was located in query results.
/// up during a routine symbol context
/// query.
eSymbolContextVariable = (1u << 7), ///< Set when \a global or static
/// variable is requested from a query,
/// or was located in query results.
///< eSymbolContextVariable is potentially expensive to lookup so it isn't
///included in
/// included in
///< eSymbolContextEverything which stops it from being used during frame PC
///lookups and
/// lookups and
///< many other potential address to symbol context lookups.
};
LLDB_MARK_AS_BITMASK_ENUM(SymbolContextItem)

FLAGS_ENUM(Permissions){ePermissionsWritable = (1u << 0),
ePermissionsReadable = (1u << 1),
Expand Down Expand Up @@ -1086,7 +1117,6 @@ enum TypeSummaryCapping {
eTypeSummaryCapped = true,
eTypeSummaryUncapped = false
};

} // namespace lldb

#endif // LLDB_lldb_enumerations_h_
3 changes: 2 additions & 1 deletion lldb/source/API/SBAddress.cpp
View file
Expand Up @@ -198,8 +198,9 @@ SBModule SBAddress::GetModule() {

SBSymbolContext SBAddress::GetSymbolContext(uint32_t resolve_scope) {
SBSymbolContext sb_sc;
SymbolContextItem scope = static_cast<SymbolContextItem>(resolve_scope);
if (m_opaque_ap->IsValid())
m_opaque_ap->CalculateSymbolContext(&sb_sc.ref(), resolve_scope);
m_opaque_ap->CalculateSymbolContext(&sb_sc.ref(), scope);
return sb_sc;
}

Expand Down
4 changes: 2 additions & 2 deletions lldb/source/API/SBFrame.cpp
View file
Expand Up @@ -112,7 +112,7 @@ SBSymbolContext SBFrame::GetSymbolContext(uint32_t resolve_scope) const {
SBSymbolContext sb_sym_ctx;
std::unique_lock<std::recursive_mutex> lock;
ExecutionContext exe_ctx(m_opaque_sp.get(), lock);

SymbolContextItem scope = static_cast<SymbolContextItem>(resolve_scope);
StackFrame *frame = nullptr;
Target *target = exe_ctx.GetTargetPtr();
Process *process = exe_ctx.GetProcessPtr();
Expand All @@ -121,7 +121,7 @@ SBSymbolContext SBFrame::GetSymbolContext(uint32_t resolve_scope) const {
if (stop_locker.TryLock(&process->GetRunLock())) {
frame = exe_ctx.GetFramePtr();
if (frame) {
sb_sym_ctx.SetSymbolContext(&frame->GetSymbolContext(resolve_scope));
sb_sym_ctx.SetSymbolContext(&frame->GetSymbolContext(scope));
} else {
if (log)
log->Printf("SBFrame::GetVariables () => error: could not "
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/API/SBModule.cpp
View file
Expand Up @@ -217,9 +217,9 @@ SBModule::ResolveSymbolContextForAddress(const SBAddress &addr,
uint32_t resolve_scope) {
SBSymbolContext sb_sc;
ModuleSP module_sp(GetSP());
SymbolContextItem scope = static_cast<SymbolContextItem>(resolve_scope);
if (module_sp && addr.IsValid())
module_sp->ResolveSymbolContextForAddress(addr.ref(), resolve_scope,
*sb_sc);
module_sp->ResolveSymbolContextForAddress(addr.ref(), scope, *sb_sc);
return sb_sc;
}

Expand Down
5 changes: 3 additions & 2 deletions lldb/source/API/SBTarget.cpp
View file
Expand Up @@ -660,11 +660,12 @@ SBSymbolContext
SBTarget::ResolveSymbolContextForAddress(const SBAddress &addr,
uint32_t resolve_scope) {
SBSymbolContext sc;
SymbolContextItem scope = static_cast<SymbolContextItem>(resolve_scope);
if (addr.IsValid()) {
TargetSP target_sp(GetSP());
if (target_sp)
target_sp->GetImages().ResolveSymbolContextForAddress(
addr.ref(), resolve_scope, sc.ref());
target_sp->GetImages().ResolveSymbolContextForAddress(addr.ref(), scope,
sc.ref());
}
return sc;
}
Expand Down
4 changes: 3 additions & 1 deletion lldb/source/Commands/CommandObjectSource.cpp
View file
Expand Up @@ -1210,7 +1210,9 @@ class CommandObjectSourceList : public CommandObjectParsed {
target->GetImages().FindModules(module_spec, matching_modules);
num_matches += matching_modules.ResolveSymbolContextForFilePath(
filename, 0, check_inlines,
eSymbolContextModule | eSymbolContextCompUnit, sc_list);
SymbolContextItem(eSymbolContextModule |
eSymbolContextCompUnit),
sc_list);
}
}
} else {
Expand Down
5 changes: 3 additions & 2 deletions lldb/source/Core/Address.cpp
View file
Expand Up @@ -779,8 +779,9 @@ bool Address::SectionWasDeletedPrivate() const {
m_section_wp.owner_before(empty_section_wp);
}

uint32_t Address::CalculateSymbolContext(SymbolContext *sc,
uint32_t resolve_scope) const {
uint32_t
Address::CalculateSymbolContext(SymbolContext *sc,
SymbolContextItem resolve_scope) const {
sc->Clear(false);
// Absolute addresses don't have enough information to reconstruct even their
// target.
Expand Down
6 changes: 3 additions & 3 deletions lldb/source/Core/Disassembler.cpp
View file
Expand Up @@ -429,9 +429,9 @@ bool Disassembler::PrintInstructions(Disassembler *disasm_ptr,
const Address &addr = inst->GetAddress();
ModuleSP module_sp(addr.GetModule());
if (module_sp) {
const uint32_t resolve_mask = eSymbolContextFunction |
eSymbolContextSymbol |
eSymbolContextLineEntry;
const SymbolContextItem resolve_mask = eSymbolContextFunction |
eSymbolContextSymbol |
eSymbolContextLineEntry;
uint32_t resolved_mask =
module_sp->ResolveSymbolContextForAddress(addr, resolve_mask, sc);
if (resolved_mask) {
Expand Down

0 comments on commit 991e445

Please sign in to comment.