Skip to content

Always Import FieldDecls from Records #40760

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 2 commits into from
Jan 8, 2022
Merged

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Jan 7, 2022

When we switched this path over to use lazy member loading, a
subsequent commit introduced a check that members were canonical. In
certain extremely twisty scenarios involving multiple modular frameworks
pulling in non-modular headers, this check can fail which results in us
silently dropping these members on the floor.

It's ultimately correct to only pull in the canonical members, but real-world
examples of "system" frameworks evading modularity diagnostics exist so this
is strictly a regression.

Drop the canonicity check for FieldDecls.

rdar://86740970

When we switched this path over to use lazy member loading, a
subsequent commit introduced a check that members were canonical. In
certain extremely twisty scenarios involving multiple modular frameworks
pulling in non-modular headers, this check can fail which results in us
silently dropping these members on the floor.

It's ultimately correct to only pull in the canonical members, but real-world
examples of "system" frameworks evading modularity diagnostics exist so this
is strictly a regression.

Drop the canonicity check for FieldDecls.

rdar://86740970
@CodaFi CodaFi requested a review from DougGregor January 7, 2022 08:23
@CodaFi
Copy link
Contributor Author

CodaFi commented Jan 7, 2022

@swift-ci test

@CodaFi
Copy link
Contributor Author

CodaFi commented Jan 7, 2022

I have been trying for hours to get the stars to align to test for this and I have failed...

@swift-ci
Copy link
Contributor

swift-ci commented Jan 7, 2022

Build failed
Swift Test OS X Platform
Git Sha - 8d2bf20

@DougGregor
Copy link
Member

@swift-ci please smoke test macOS

@CodaFi
Copy link
Contributor Author

CodaFi commented Jan 7, 2022

@swift-ci smoke test

@DougGregor
Copy link
Member

Thank for you finding and fixing this. Silly me, thinking the world is as it should be.

@CodaFi
Copy link
Contributor Author

CodaFi commented Jan 7, 2022

@swift-ci smoke test

@CodaFi
Copy link
Contributor Author

CodaFi commented Jan 8, 2022

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