Skip to content

[nfc][cxx-interop] Mark lazy member loading complete only after members are loaded. #40336

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 1 commit into from
Dec 1, 2021

Conversation

zoecarver
Copy link
Contributor

This will be slightly more performant because we don't have to walk through all members multiple times (even though we cache all the member lookups).

@zoecarver zoecarver added the c++ interop Feature: Interoperability with C++ label Nov 30, 2021
@@ -989,15 +989,15 @@ void IterableDeclContext::loadAllMembers() const {
if (!hasLazyMembers())
return;

// Don't try to load all members re-entrant-ly.
FirstDeclAndLazyMembers.setInt(false);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@CodaFi maybe I'm confused about what's going on here, but it seems like this logic is not correct. Shouldn't we mark the members as lazily complete after we load them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, after investigating some test failures, I see why this is needed now. Otherwise we get into a re-entrant loop where we keep adding the same members.

@zoecarver zoecarver force-pushed the set-lazy-complete-after-load branch from 10212a5 to f3b7a40 Compare November 30, 2021 20:28
@zoecarver
Copy link
Contributor Author

I just re-factored this patch so it only touches the clang importer. Sorry for the noise.

@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test.

@zoecarver zoecarver merged commit d08ecf3 into swiftlang:main Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant