Skip to content

[index] Improvements on how conformances are recorded #19043

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
Aug 29, 2018

Conversation

akyrtzi
Copy link
Contributor

@akyrtzi akyrtzi commented Aug 29, 2018

Query for the local conformances of a DeclContext and record conformances of
functions that are defined in other types as 'implicit' overrides, with
the type that introduces the conformance as container symbol.

This allows more accurate index data, and avoids needing to query for all
protocol requirements, which is expensive to calculate.

Query for the local conformances of a DeclContext and record conformances of
functions that are defined in other types as 'implicit' overrides, with
the type that introduces the conformance as container symbol.

This allows more accurate index data, and avoids needing to query for all
protocol requirements, which is expensive to calculate.
@akyrtzi
Copy link
Contributor Author

akyrtzi commented Aug 29, 2018

@swift-ci smoke test

@akyrtzi
Copy link
Contributor Author

akyrtzi commented Aug 29, 2018

@swift-ci smoke test

Copy link
Contributor

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

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

LGTM, with one suggestion.

for (auto Overriden: getOverriddenDecls(D, /*IncludeProtocolReqs=*/!isSystemModule)) {
if (addRelation(Info, (SymbolRoleSet) SymbolRole::RelationOverrideOf, Overriden))
return false;
for (auto Overriden: getOverriddenDecls(D, /*IncludeProtocolReqs=*/false)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should change the default value for IncludeProtocolReqs in getOverriddenDecls to false or remove the default value entirely to avoid accidentally adding this kind of code in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately the migrator also depends on this (in APIDiffMigratorPass::getRelatedDiffItems()), I intend to remove it after the migrator is changed as well (rdar://43844914)

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