Skip to content

[Serialization] Add flag to force unsafe recovery from some xref failures #66186

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
May 30, 2023

Conversation

xymus
Copy link
Contributor

@xymus xymus commented May 26, 2023

Intro a deserialization mode controlled by the flag -experimental-force-workaround-broken-modules to attempt unsafe recovery from deserialization failures caused by project issues.

The one issue handled at this time is when a type moves from one module to another. With this new mode the compiler may be able to pick a matching type in a different module. This is risky to use, but may help in a pinch for a client to fix and issue in a library over which they have no control.

…ures

Intro a deserialization mode controlled by the flag
`-experimental-force-workaround-broken-modules` to attempt unsafe
recovery from deserialization failures caused by project issues.

The one issue handled at this time is when a type moves from one module
to another. With this new mode the compiler may be able to pick a
matching type in a different module. This is risky to use, but may help
in a pinch for a client to fix and issue in a library over which they
have no control.
@xymus xymus requested review from bnbarham, elsh, artemcm and tshortli May 26, 2023 21:39
@xymus
Copy link
Contributor Author

xymus commented May 26, 2023

@swift-ci Please smoke test

Comment on lines 1965 to 1971
llvm::handleAllErrors(std::move(error),
[&](const ModularizationError &modularError) {
modularError.diagnose(this, DiagnosticBehavior::Remark);
});
getContext().Diags.diagnose(getSourceLoc(),
diag::modularization_issue_worked_around,
foundIn);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to make this a warning with the remark instead attached as a note?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I inverted both diagnostics, using the warning as the main message with the current remark as a note.

Use the `attempting forced recovery` diagnostic as main warning to which
we attach other messages as notes. Also mention the flag in the
diagnostic to reinforce that the flag is active.
@xymus xymus force-pushed the force-workaround branch from e233f52 to d50f20e Compare May 26, 2023 22:28
@xymus
Copy link
Contributor Author

xymus commented May 26, 2023

@swift-ci Please smoke test

@xymus xymus merged commit 814ca43 into swiftlang:main May 30, 2023
@xymus xymus deleted the force-workaround branch May 30, 2023 15:55
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