Skip to content

[Incremental compilation]: Merge pull request #32219 from davidungar/quick-dependency-bug-fix #32230

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

Conversation

davidungar
Copy link
Contributor

@davidungar davidungar commented Jun 6, 2020

Fixes a regression from 5.2. Undo a dependency optimization that causes miscompiles.

Explanation: In order to realize a benefit from type-body-fingerprints, code was added to omit a dependency wherein a file A depended upon a type defined in that same file A. However, if that same type is also used subsequently to its definition in the same file A, the omission causes miscompiles because a use of that type in file B depends upon file A. The omission causes file A to be insensitive to changes in the type, and thus causes the use in file B to be insensitive to changes in the type. In the future, refinements in the dependency system may well allow us to reenable this optimization, but for now, we need to disable the optimization in order to prevent miscompilations.

Origination: d09c906 Fed 27, 11:38 pm
Risk: Zero, because all it does is add dependencies. It effectively puts us back to pre-type-body-fingerprints.
Reviewed by: Slava Pestov
Testing: CI PR tests + the perturbation tester
Radar:rdar://64074744

…ug-fix

Simplest fix to a dependency bug.
@davidungar davidungar requested a review from a team as a code owner June 6, 2020 22:08
@davidungar
Copy link
Contributor Author

@swift-ci please test

@davidungar
Copy link
Contributor Author

@slavapestov , @CodaFi , @DougGregor
Thanks to Slava and then my own experiments confirming it, I’m sure that the predicate at the end of AbstractSourceFileDependencyGraph.cpp causes miscompiles today. So I decided to disable it in master, right away. I would also like to nominate it for 5.3, right away. Would one of you care to review #32230 ?

@davidungar
Copy link
Contributor Author

@swift-ci please nominate

@theblixguy theblixguy added the r5.3 label Jun 7, 2020
@tkremenek tkremenek merged commit 1ebb558 into swiftlang:release/5.3 Jun 9, 2020
@AnthonyLatsis AnthonyLatsis added swift 5.3 🍒 release cherry pick Flag: Release branch cherry picks labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants