Skip to content

More ASTScope cleanups #34076

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 3 commits into from
Sep 25, 2020
Merged

Conversation

slavapestov
Copy link
Contributor

No description provided.

If we're searching for a declaration with a given name, the name
should be entirely encapsulated inside the DeclConsumer.

Otherwise, there might not be a specific name at all, if we're
performing code completion for example (once LookupVisibleDecls
starts to use ASTScope, anyway).
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

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.

All this code was there originally to help me debug these tough bugs that happened when the AST changed out from under ASTScopes.

I didn't quite remember this code when we spoke about it earlier today.

I don't mind deleting it, but it would sure be great if we could add a flag somewhere that would prevent the creation of any new explicit DECLs after the first ASTScope lookup. Is there any possibility of that?

@slavapestov slavapestov merged commit f436b60 into swiftlang:main Sep 25, 2020
@slavapestov
Copy link
Contributor Author

I’ll think about that along with the other assertions I promised you I would add.

@davidungar
Copy link
Contributor

I’ll think about that along with the other assertions I promised you I would add.

Thanks. It you figure out a good way do it, it would be so cool to just replace the other assertions. No need to add most of them if we can do that.

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