Skip to content

Eliminate unnecessary SwiftASTContext fallback in GetIndexOfChildMemb… #3889

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ class SwiftLanguageRuntimeStub {
return {};
}

llvm::Optional<size_t> GetIndexOfChildMemberWithName(
std::pair<bool, llvm::Optional<size_t>> GetIndexOfChildMemberWithName(
CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes) {
STUB_LOG();
Expand Down Expand Up @@ -2209,7 +2209,8 @@ llvm::Optional<std::string> SwiftLanguageRuntime::GetEnumCaseName(
FORWARD(GetEnumCaseName, type, data, exe_ctx);
}

llvm::Optional<size_t> SwiftLanguageRuntime::GetIndexOfChildMemberWithName(
std::pair<bool, llvm::Optional<size_t>>
SwiftLanguageRuntime::GetIndexOfChildMemberWithName(
CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes) {
FORWARD(GetIndexOfChildMemberWithName, type, name, exe_ctx,
Expand Down
12 changes: 11 additions & 1 deletion lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,17 @@ class SwiftLanguageRuntime : public LanguageRuntime {
const DataExtractor &data,
ExecutionContext *exe_ctx);

llvm::Optional<size_t> GetIndexOfChildMemberWithName(
/// Behaves like the CompilerType::GetIndexOfChildMemberWithName()
/// except for the more nuanced return value.
///
/// \returns {false, {}} on error.
//
/// \returns {true, {}} if the member exists, but it is an enum case
/// without payload. Enum cases without payload
/// don't have an index.
///
/// \returns {true, {num_idexes}} on success.
std::pair<bool, llvm::Optional<size_t>> GetIndexOfChildMemberWithName(
CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1073,26 +1073,32 @@ GetTypeFromTypeRef(TypeSystemSwiftTypeRef &ts,
return ts.RemangleAsType(dem, node);
}

static llvm::Optional<size_t>
static std::pair<bool, llvm::Optional<size_t>>
findFieldWithName(const std::vector<swift::reflection::FieldInfo> &fields,
llvm::StringRef name, std::vector<uint32_t> &child_indexes,
uint32_t offset = 0) {
llvm::StringRef name, bool is_enum,
std::vector<uint32_t> &child_indexes, uint32_t offset = 0) {
uint32_t index = 0;
bool is_nonpayload_enum_case = false;
auto it = std::find_if(fields.begin(), fields.end(), [&](const auto &field) {
// A nonnull TypeRef is required for enum cases, where it represents cases
// that have a payload. In other types it will be true anyway.
if (field.TR == nullptr)
return false;
if (name != field.Name) {
++index;
// A nonnull TypeRef is required for enum cases, where it represents cases
// that have a payload. In other types it will be true anyway.
if (field.TR)
++index;
return false;
}
if (is_enum)
is_nonpayload_enum_case = (field.TR == nullptr);
return true;
});
// Not found.
if (it == fields.end())
return {};
return {false, {}};
// Found, but no index to report.
if (is_nonpayload_enum_case)
return {true, {}};
child_indexes.push_back(offset + index);
return child_indexes.size();
return {true, child_indexes.size()};
}

static llvm::Optional<std::string>
Expand Down Expand Up @@ -1136,21 +1142,22 @@ llvm::Optional<std::string> SwiftLanguageRuntimeImpl::GetEnumCaseName(
return {};
}

llvm::Optional<size_t> SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
std::pair<bool, llvm::Optional<size_t>>
SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes) {
LLDB_SCOPED_TIMER();
auto *ts =
llvm::dyn_cast_or_null<TypeSystemSwiftTypeRef>(type.GetTypeSystem());
if (!ts)
return {};
return {false, {}};

using namespace swift::reflection;
// Try the static type metadata.
const TypeRef *tr = nullptr;
auto *ti = GetSwiftRuntimeTypeInfo(type, exe_ctx->GetFramePtr(), &tr);
if (!ti)
return {};
return {false, {}};
switch (ti->getKind()) {
case TypeInfoKind::Record: {
// Structs and Tuples.
Expand All @@ -1160,7 +1167,7 @@ llvm::Optional<size_t> SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
case RecordKind::ThickFunction:
// There are two fields, `function` and `context`, but they're not exposed
// by lldb.
return 0;
return {true, {0}};
case RecordKind::OpaqueExistential:
// `OpaqueExistential` is documented as:
// An existential is a three-word buffer followed by value metadata...
Expand All @@ -1170,17 +1177,17 @@ llvm::Optional<size_t> SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
uint32_t index;
if (name.take_back().getAsInteger(10, index) && index < 3) {
child_indexes.push_back(index);
return child_indexes.size();
return {true, child_indexes.size()};
}
}
return findFieldWithName(rti->getFields(), name, child_indexes, 3);
return findFieldWithName(rti->getFields(), name, false, child_indexes, 3);
default:
return findFieldWithName(rti->getFields(), name, child_indexes);
return findFieldWithName(rti->getFields(), name, false, child_indexes);
}
}
case TypeInfoKind::Enum: {
auto *eti = llvm::cast<EnumTypeInfo>(ti);
return findFieldWithName(eti->getCases(), name, child_indexes);
return findFieldWithName(eti->getCases(), name, true, child_indexes);
}
case TypeInfoKind::Reference: {
// Objects.
Expand All @@ -1207,20 +1214,21 @@ llvm::Optional<size_t> SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
break;
auto *super_tr = builder.lookupSuperclass(current_tr);
uint32_t offset = super_tr ? 1 : 0;
if (auto size = findFieldWithName(record_ti->getFields(), name,
child_indexes, offset))
return size;
auto found_size = findFieldWithName(record_ti->getFields(), name, false,
child_indexes, offset);
if (found_size.first)
return found_size;
current_tr = super_tr;
child_indexes.push_back(0);
}
child_indexes.clear();
return {};
return {false, {}};
}
}
}
default:
// FIXME: Implement more cases.
return {};
return {false, {}};
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class SwiftLanguageRuntimeImpl {
const DataExtractor &data,
ExecutionContext *exe_ctx);

llvm::Optional<size_t> GetIndexOfChildMemberWithName(
std::pair<bool, llvm::Optional<size_t>> GetIndexOfChildMemberWithName(
CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes);

Expand Down
27 changes: 16 additions & 11 deletions lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2802,27 +2802,29 @@ size_t TypeSystemSwiftTypeRef::GetIndexOfChildMemberWithName(
child_indexes));
if (auto *exe_scope = exe_ctx->GetBestExecutionContextScope())
if (auto *runtime =
SwiftLanguageRuntime::Get(exe_scope->CalculateProcess()))
if (auto index_size = runtime->GetIndexOfChildMemberWithName(
GetCanonicalType(type), name, exe_ctx, omit_empty_base_classes,
child_indexes)) {
SwiftLanguageRuntime::Get(exe_scope->CalculateProcess())) {
auto found_numidx = runtime->GetIndexOfChildMemberWithName(
GetCanonicalType(type), name, exe_ctx, omit_empty_base_classes,
child_indexes);
if (found_numidx.first) {
size_t index_size = found_numidx.second.getValueOr(0);
#ifndef NDEBUG
// This block is a custom VALIDATE_AND_RETURN implementation to support
// checking the return value, plus the by-ref `child_indexes`.
if (!GetSwiftASTContext())
return *index_size;
return index_size;
auto ast_type = ReconstructType(type);
if (!ast_type)
return *index_size;
return index_size;
std::vector<uint32_t> ast_child_indexes;
auto ast_index_size =
GetSwiftASTContext()->GetIndexOfChildMemberWithName(
ast_type, name, exe_ctx, omit_empty_base_classes,
ast_child_indexes);
// The runtime has more info than the AST. No useful validation can be
// done.
if (*index_size > ast_index_size)
return *index_size;
if (index_size > ast_index_size)
return index_size;

auto fail = [&]() {
auto join = [](const auto &v) {
Expand All @@ -2841,17 +2843,20 @@ size_t TypeSystemSwiftTypeRef::GetIndexOfChildMemberWithName(
assert(false &&
"TypeSystemSwiftTypeRef diverges from SwiftASTContext");
};
if (*index_size != ast_index_size)
if (index_size != ast_index_size)
fail();
for (unsigned i = 0; i < *index_size; ++i)
for (unsigned i = 0; i < index_size; ++i)
if (child_indexes[i] < ast_child_indexes[i])
// When the runtime may know know about more children. When this
// happens, indexes will be larger. But if an index is smaller, that
// means the runtime has dropped info somehow.
fail();
#endif
return *index_size;
return index_size;
}
// If we're here, the runtime didn't find type info.
assert(!found_numidx.first);
}

LLDB_LOGF(GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES),
"Using SwiftASTContext::GetIndexOfChildMemberWithName fallback for "
Expand Down
34 changes: 24 additions & 10 deletions lldb/test/Shell/Swift/Inputs/No.swiftmodule.swift
Original file line number Diff line number Diff line change
@@ -1,22 +1,36 @@
import NoSwiftmoduleHelper

// The struct is resolved using type metadata and the Swift runtime.
struct s { let i = 0 }
struct S { let i = 0 }

func useTypeFromOtherModule(x: S2) {
// break here
}

enum NoPayload {
case first
case second
}

enum WithPayload {
case empty
case with(i: Int)
}

func f<T>(_ t: T) {
let number = 1 // CHECK-DAG: (Int) number = 1
let array = [1, 2, 3] // CHECK-DAG: ([Int]) array = 3 values
let string = "hello" // CHECK-DAG: (String) string = "hello"
let tuple = (0, 1) // CHECK-DAG: (Int, Int) tuple = (0 = 0, 1 = 1)
let strct = s() // CHECK-DAG: strct = (i = 0)
let strct2 = S2() // CHECK-DAG: strct2 = {}{{$}}
let generic = t // CHECK-DAG: (Int) generic = 23
let generic_tuple = (t, t) // CHECK-DAG: generic_tuple = (0 = 23, 1 = 23)
let word = 0._builtinWordValue // CHECK-DAG: word = 0
let number = 1 // CHECK-DAG: (Int) number {{=}} 1
let array = [1, 2, 3] // CHECK-DAG: ([Int]) array {{=}} 3 values
let string = "hello" // CHECK-DAG: (String) string {{=}} "hello"
let tuple = (0, 1) // CHECK-DAG: (Int, Int) tuple {{=}} (0 = 0, 1 = 1)
let strct = S() // CHECK-DAG: strct {{=}} (i = 0)
let strct2 = S2() // CHECK-DAG: strct2 {{=}} {}{{$}}
let generic = t // CHECK-DAG: (Int) generic {{=}} 23
let generic_tuple = (t, t) // CHECK-DAG: generic_tuple {{=}} (0 = 23, 1 = 23)
let word = 0._builtinWordValue // CHECK-DAG: word {{=}} 0
let enum1 = NoPayload.second // CHECK-DAG: enum1 {{=}}
// FIXME: Fails in swift::reflection::NoPayloadEnumTypeInfo::projectEnumValue: .second
let enum2 = WithPayload.with(i:42) // CHECK-DAG: enum2 {{=}} with
// CHECK-DAG: i {{=}} 42
print(number)
useTypeFromOtherModule(x: S2())
}
Expand Down
1 change: 1 addition & 0 deletions lldb/test/Shell/Swift/No.swiftmodule.test
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ run
fr var
up
fr var
quit