Skip to content

Invert the ownership between SwiftASTContext and TypeSystemSwiftTypeR… #3408

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

Conversation

adrian-prantl
Copy link

…ef (NFC)

This patch introduces a new subclass SwiftASTContextForModules, which
is now owned by TypeSystemSwiftTypeRef and initialized lazily. The
TypeSystemSwiftTypeRef that corresponds to the scratch context is
still owned by SwiftASTContextForExpressions.

rdar://81717792

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

…ef (NFC)

This patch introduces a new subclass SwiftASTContextForModules, which
is now owned by TypeSystemSwiftTypeRef and initialized lazily. The
TypeSystemSwiftTypeRef that corresponds to the scratch context is
still owned by SwiftASTContextForExpressions.

rdar://81717792
@adrian-prantl
Copy link
Author

@swift-ci test

Comment on lines +2853 to +2856
module_holder ? module_holder : &ts->GetTypeSystemSwiftTypeRef(),
module_holder ? module_holder->GetSwiftASTContext()
: ts->GetSwiftASTContext(),
dem, mangled_name.GetStringRef());

Choose a reason for hiding this comment

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

I remember these ternaries, I wanted to get back to them to remove them. There are two callers of SwiftLanguageRuntimeImpl::GetTypeRef(), and in both cases, they pass in a type, and a TypeSystemSwiftTypeRef derived from that type. Then here, it's derived again.

In other words, the both caller and callee are doing type.GetTypeSystem()->GetTypeSystemSwiftTypeRef() (ignoring the cast for simplicity). I think these ternaries can be removed.

Copy link
Author

Choose a reason for hiding this comment

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

I will do this in a follow-up commit.

Copy link

@augusto2112 augusto2112 left a comment

Choose a reason for hiding this comment

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

LGTM

@adrian-prantl adrian-prantl merged commit 6133bb7 into swiftlang:stable/20210726 Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants