Skip to content
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

[Parse] Avoid delayed member parsing for type decl with missing brace #32191

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Jun 4, 2020

Cache empty member list so that 'IterableDeclContext::loadAllMembers()'
doesn't perform delayed member parsing.

Fixes: rdar://problem/63921896

Cache empty member list so that 'IterableDeclContext::loadAllMembers()'
doesn't perform delayed member parsing.

Fixes: rdar://problem/63921896
@rintaro
Copy link
Member Author

rintaro commented Jun 4, 2020

@swift-ci Please smoke test

@rintaro rintaro requested review from nkcsgexi and benlangmuir June 4, 2020 21:36
LBLoc = RBLoc = PreviousLoc;

// Cache the empty result to prevent delayed parsing.
Context.evaluator.cacheOutput(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we change the evaluator to handle this rather than poking the result into the cache? This currently depends on the cache not really being a cache, but instead global mutable state.

The refactoring of parseMemberDeclList still seems like a readability win to me though.

Copy link
Member Author

@rintaro rintaro Jun 4, 2020

Choose a reason for hiding this comment

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

Although we can use getBrace().Start == getBrace().End as the indicator of "missing member brace", I thought it's a little brittle.

I implement it like this because "eager member parsing" is doing cacheOutput() as well.
https://github.com/apple/swift/blob/09ea5fd1f7fd61ed2ce8be380f1efc3c120a5ba6/lib/Parse/ParseDecl.cpp#L4441-L4458

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I guess this doesn't make it worse in that case.

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