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

Fix indexing crasher with implicit objcImpl inits #79207

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

beccadax
Copy link
Contributor

@beccadax beccadax commented Feb 7, 2025

Implicit initializers are given a source location within the type they belong to. This works poorly for @objc @implementation classes, because the class they belong to is imported and so those SourceLocs are in a different source buffer from the extension they’re inside, breaking an invariant enforced by index-while-building features.

Fix these SourceLocs to come from the implementation context, so they’ll come from the extension for an objcImpl class and the type itself otherwise.

@beccadax
Copy link
Contributor Author

beccadax commented Feb 7, 2025

@swift-ci please test

@beccadax
Copy link
Contributor Author

beccadax commented Feb 7, 2025

@bnbarham Would especially appreciate your input, as I'm not very familiar with indexing.

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

Thanks @beccadax! Seems reasonable to me. My only comment would be that instead of printing the index symbols from swift-ide-test after already indexing using index-store-path, we could just print the record and check that instead.

And I'm assuming this is a similar crash to #78835, but where the buffer ID is actually valid (as we're currently indexing the Swift file, but getting a location outside it)?

Implicit initializers are given a source location within the type they belong to. This works poorly for @objc @implementation classes, because the class they belong to is imported and so those SourceLocs are in a different source buffer from the extension they’re inside, breaking an invariant enforced by index-while-building features.

Fix these SourceLocs to come from the implementation context, so they’ll come from the extension for an objcImpl class and the type itself otherwise.
@beccadax
Copy link
Contributor Author

beccadax commented Feb 7, 2025

@swift-ci please test

@beccadax
Copy link
Contributor Author

beccadax commented Feb 7, 2025

@bnbarham Yes, it's a very similar crash—just caused by seeing a source location belonging to e.g. buffer 42 when we're indexing buffer 13, rather than seeing a source location for 13 when we're indexing the nonexistent -1.

@beccadax beccadax merged commit f393962 into swiftlang:main Feb 11, 2025
5 checks passed
@beccadax beccadax deleted the objcquious-indexes branch February 12, 2025 00:24
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