Skip to content

[lldb] Wire-up TypeSystemSwiftTypeRef::GetNumChildren #2200

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

Merged
merged 13 commits into from
Dec 9, 2020
Merged

[lldb] Wire-up TypeSystemSwiftTypeRef::GetNumChildren #2200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b82ad6d
[lldb] Wire-up TypeSystemSwiftTypeRef::GetNumChildren
kastiglione Dec 8, 2020
79f7869
add EnumTypeInfo handling in GetNumChildren
kastiglione Dec 8, 2020
3331e29
fallback to SwiftASTContext for unsupported cases
kastiglione Dec 8, 2020
9e41188
use fallback for any failed data dependency
kastiglione Dec 8, 2020
e2381ff
handle OpaqueExistential's three payload fields
kastiglione Dec 9, 2020
58a76c3
tweak comment
kastiglione Dec 9, 2020
6449eb1
call VALIDATE_AND_RETURN only when not using SwiftASTContext
kastiglione Dec 9, 2020
92d02fe
hardcode function types to zero children
kastiglione Dec 9, 2020
3b030b8
change Equivalent<uint32_t> to assert >=
kastiglione Dec 9, 2020
9af8222
log GetNumChildren fallback
kastiglione Dec 9, 2020
1cb9b37
use Equivalent(Optional<T>, Optional<T>)
kastiglione Dec 9, 2020
dc3fcf9
tweak substrs in TestLibraryResilient.py
kastiglione Dec 9, 2020
a5d4945
move function type handling into SwiftLanguageRuntimeImpl
kastiglione Dec 9, 2020
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
38 changes: 32 additions & 6 deletions lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1508,10 +1508,9 @@ template <> bool Equivalent<ConstString>(ConstString l, ConstString r) {
return l == r;
}

/// Version taylored to GetBitSize & friends.
template <>
bool Equivalent<llvm::Optional<uint64_t>>(llvm::Optional<uint64_t> l,
llvm::Optional<uint64_t> r) {
/// Version tailored to GetBitSize & friends.
template <typename T>
bool Equivalent(llvm::Optional<T> l, llvm::Optional<T> r) {
if (l == r)
return true;
// There are situations where SwiftASTContext incorrectly returns
Expand All @@ -1526,6 +1525,12 @@ bool Equivalent<llvm::Optional<uint64_t>>(llvm::Optional<uint64_t> l,
return false;
}

// Introduced for `GetNumChildren`.
template <typename T>
bool Equivalent(llvm::Optional<T> l, T r) {
return Equivalent(l, llvm::Optional<T>(r));
}

} // namespace
#endif

Expand Down Expand Up @@ -2065,13 +2070,34 @@ lldb::Encoding TypeSystemSwiftTypeRef::GetEncoding(opaque_compiler_type_t type,
lldb::Format TypeSystemSwiftTypeRef::GetFormat(opaque_compiler_type_t type) {
return m_swift_ast_context->GetFormat(ReconstructType(type));
}

uint32_t
TypeSystemSwiftTypeRef::GetNumChildren(opaque_compiler_type_t type,
bool omit_empty_base_classes,
const ExecutionContext *exe_ctx) {
return m_swift_ast_context->GetNumChildren(ReconstructType(type),
omit_empty_base_classes, exe_ctx);
if (exe_ctx)
if (auto *exe_scope = exe_ctx->GetBestExecutionContextScope())
if (auto *runtime =
SwiftLanguageRuntime::Get(exe_scope->CalculateProcess()))
if (auto num_children =
runtime->GetNumChildren(GetCanonicalType(type), nullptr)) {
// Use a lambda to intercept and unwrap the `Optional` return value.
return [&]() {
auto impl = [&]() { return num_children; };
VALIDATE_AND_RETURN(
impl, GetNumChildren, type,
(ReconstructType(type), omit_empty_base_classes, exe_ctx));
}().getValue();
}

LLDB_LOGF(GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES),
"Using SwiftASTContext::GetNumChildren fallback for type %s",
AsMangledName(type));

return m_swift_ast_context->GetNumChildren(
ReconstructType(type), omit_empty_base_classes, exe_ctx);
}

uint32_t TypeSystemSwiftTypeRef::GetNumFields(opaque_compiler_type_t type) {
return m_swift_ast_context->GetNumFields(ReconstructType(type));
}
Expand Down
19 changes: 17 additions & 2 deletions lldb/source/Target/SwiftLanguageRuntimeDynamicTypeResolution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,8 +1012,23 @@ SwiftLanguageRuntimeImpl::GetNumChildren(CompilerType type,
// Structs and Tuples.
if (auto *rti =
llvm::dyn_cast_or_null<swift::reflection::RecordTypeInfo>(ti)) {
auto fields = rti->getFields();
return fields.size();
switch (rti->getRecordKind()) {
case swift::reflection::RecordKind::ThickFunction:
// There are two fields, `function` and `context`, but they're not exposed
// by lldb.
return 0;
case swift::reflection::RecordKind::OpaqueExistential:
// `OpaqueExistential` is documented as:
// An existential is a three-word buffer followed by value metadata...
// The buffer is exposed as children named `payload_data_{0,1,2}`, and
// the number of fields are increased to match.
return rti->getNumFields() + 3;
default:
return rti->getNumFields();
}
}
if (auto *eti = llvm::dyn_cast_or_null<swift::reflection::EnumTypeInfo>(ti)) {
return eti->getNumPayloadCases();
}
// FIXME: Implement more cases.
return {};
Expand Down
2 changes: 1 addition & 1 deletion ...test/API/lang/swift/implementation_only_imports/library_resilient/TestLibraryResilient.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def cleanup():
# This test is deliberately checking what the user will see, rather than
# the structure provided by the Python API, in order to test the recovery.
self.expect("fr var", substrs=[
"(SomeLibrary.ContainsTwoInts) container = (other = 10)",
"(SomeLibrary.ContainsTwoInts) container = {", "other = 10",
Copy link
Author

@kastiglione kastiglione Dec 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is perhaps indicative of issues that need to be solved in this PR, or in follow ups.

ContainsTwoInts is defined as:

public struct ContainsTwoInts {
  internal var wrapped: TwoInts
  public var other: Int
}

This was previously formatted as container = (other = 10) but with these changes it's formatted as:

container = {
  other = 10
}

The same data is printed, but the difference is (presumably) that ShouldPrintAsOneLiner is no longer true. Note that despite there being two children in the type, lldb prints only one, the public property other.

Based on my debugging of other discrepancies, my guess is that GetNumChildren now returns 2 instead of 1, which then causes ShouldPrintAsOneLiner to return false.

What I think should be possible now, is for lldb to print both properties, wrapped and other. I'll take a look tomorrow to see if the TypeRef implementation of GetChildCompilerTypeAtIndex is excluding wrapped to mirror the behavior of SwiftASTContext. If that is the case, it seems like something we should fix, showing private properties too would be an improvement.

Copy link

Choose a reason for hiding this comment

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

Sounds great! Thanks for the thoughtful analysis.

"(Int) simple = 1"])
self.expect("e container", substrs=["(SomeLibrary.ContainsTwoInts)", "other = 10"])
self.expect("e container.wrapped", error=True, substrs=["value of type 'ContainsTwoInts' has no member 'wrapped'"])