Skip to content

Ask the external provider for type info first in visitAnyNominalTypeRef #61081

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 1 commit into from
Oct 4, 2022
Merged

Ask the external provider for type info first in visitAnyNominalTypeRef #61081

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 27 additions & 13 deletions stdlib/public/Reflection/TypeLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2226,29 +2226,43 @@ class LowerType
}

const TypeInfo *visitAnyNominalTypeRef(const TypeRef *TR) {
auto QueryExternalTypeInfoProvider = [&]() -> const TypeInfo * {
if (ExternalTypeInfo) {
std::string MangledName;
if (auto N = dyn_cast<NominalTypeRef>(TR))
MangledName = N->getMangledName();
else if (auto BG = dyn_cast<BoundGenericTypeRef>(TR))
MangledName = BG->getMangledName();
if (!MangledName.empty())
if (auto *imported = ExternalTypeInfo->getTypeInfo(MangledName))
return imported;
}
return nullptr;
};

auto FD = TC.getBuilder().getFieldTypeInfo(TR);
if (FD == nullptr || FD->isStruct()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it were safer to just move it into the if (FD == nullptr || FD->isStruct()) condition?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would we want ExternalTypeInfo to be consulted when FD->isStruct() is true? In other words, should external lookup happen only when FD == nullptr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there are two situations where we want to ask the external provider:

  • We don't have any metadata for the type, ask the external provider.
  • We have metadata for the type and it's a builtin (builtins are structs), ask the external provider as it may have more accurate information.

Copy link
Contributor

Choose a reason for hiding this comment

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

in those two situations, should they be checked after TC.getBuilder().getFieldTypeInfo(TR)?

Copy link
Contributor

Choose a reason for hiding this comment

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

currently this change checks before finding out whether either of those two cases apply.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup! Just updated the patch :)

// Maybe this type is opaque -- look for a builtin
// descriptor to see if we at least know its size
// and alignment.
if (auto ImportedTypeDescriptor = TC.getBuilder().getBuiltinTypeInfo(TR))
if (auto ImportedTypeDescriptor =
TC.getBuilder().getBuiltinTypeInfo(TR)) {
// This might be an external type we treat as opaque (like C structs),
// the external type info provider might have better type information,
// so ask it first.
if (auto External = QueryExternalTypeInfoProvider())
return External;

return TC.makeTypeInfo<BuiltinTypeInfo>(TC.getBuilder(),
ImportedTypeDescriptor);
}

// Otherwise, we're out of luck.
if (FD == nullptr) {
if (ExternalTypeInfo) {
// Ask the ExternalTypeInfo. It may be a Clang-imported type.
std::string MangledName;
if (auto N = dyn_cast<NominalTypeRef>(TR))
MangledName = N->getMangledName();
else if (auto BG = dyn_cast<BoundGenericTypeRef>(TR))
MangledName = BG->getMangledName();
if (!MangledName.empty())
if (auto *imported = ExternalTypeInfo->getTypeInfo(MangledName))
return imported;
}
// If we still have no type info ask the external provider.
if (auto External = QueryExternalTypeInfoProvider())
return External;

// If the external provider also fails we're out of luck.
DEBUG_LOG(fprintf(stderr, "No TypeInfo for nominal type: "); TR->dump());
return nullptr;
}
Expand Down