Skip to content

ASTScope: Remove "re-expansion" mechanism #34022

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 10 commits into from
Sep 23, 2020

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Sep 22, 2020

The subset of the AST that participates in unqualified lookup should be effectively immutable. Delayed parsing can fill in members of nominal types and extensions, as well as function bodies, but since we lazily expand scopes, and the relevant getter methods on IterableDeclContext and AbstractFunctionBody trigger parsing, ASTScope should always see a consistent view of the world.

@slavapestov slavapestov changed the title Astscope remove re expansion ASTScope: Remove "re-expansion" mechanism, and other simplifications Sep 22, 2020
@slavapestov slavapestov force-pushed the astscope-remove-re-expansion branch 2 times, most recently from 46d2245 to 4f9a684 Compare September 22, 2020 17:58
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov changed the title ASTScope: Remove "re-expansion" mechanism, and other simplifications ASTScope: Remove "re-expansion" mechanism Sep 22, 2020
Copy link
Contributor

@davidungar davidungar left a comment

Choose a reason for hiding this comment

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

I would rather keep the member count and put in an assertion as we discussed. The member count would only count explicit members. Otherwise, my feeling is that this change introduces more fragility than my comfort level.

@slavapestov
Copy link
Contributor Author

@davidungar I'm planning on switching the call to getMembers() to use getParsedMembers() instead. This evaluates a request which produces a once-allocated ArrayRef which is guaranteed to never change.

@slavapestov slavapestov force-pushed the astscope-remove-re-expansion branch from 4f9a684 to af627c2 Compare September 23, 2020 05:25
@slavapestov slavapestov force-pushed the astscope-remove-re-expansion branch from af627c2 to 4bbdb95 Compare September 23, 2020 05:26
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

@slavapestov slavapestov changed the base branch from master to main September 23, 2020 07:05
@slavapestov slavapestov merged commit d37318b into swiftlang:main Sep 23, 2020
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.

2 participants