Skip to content

[CS] Fix a couple of use-after-frees #82596

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
Jun 29, 2025

Conversation

hamishknight
Copy link
Contributor

@hamishknight hamishknight commented Jun 28, 2025

Make sure we call simplifyType for the opened type bindings in computeSubstitutions to ensure holes get converted to UnresolvedType. Also make sure we use the resolved type in NonOptionalUnwrapFailure::diagnoseAsError since diagnostics can outlive the ConstraintSystem itself if we have a diagnostic transaction for e.g typeCheckParameterDefault, so make sure we don't try to use a solver-allocated type as an argument.

rdar://154553285

Make sure we call `simplifyType` for the opened type bindings to
ensure holes get converted to UnresolvedType.

rdar://154553285
…ror`

Diagnostics can outlive the ConstraintSystem itself if we have a
diagnostic transaction for e.g `typeCheckParameterDefault`, make sure
we don't try to use a solver-allocated type as an argument.
@hamishknight hamishknight changed the title [CS] Avoid escaping solver-allocated types in computeSubstitutions [CS] Fix a couple of use-after-frees Jun 28, 2025
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@hamishknight
Copy link
Contributor Author

@swift-ci please test source compatibility

@@ -3880,7 +3880,8 @@ bool NonOptionalUnwrapFailure::diagnoseAsError() {
diagnostic = diag::invalid_force_unwrap;

auto range = getSourceRange();
emitDiagnostic(diagnostic, BaseType).highlight(range).fixItRemove(range.End);
emitDiagnostic(diagnostic, resolveType(BaseType))
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 move resolveType to the constructor of this diagnostic instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it can probably be done with a pile of templating logic, I can look into that as a follow up

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, thank you!

@hamishknight hamishknight enabled auto-merge June 28, 2025 19:00
@hamishknight hamishknight merged commit 54a8a86 into swiftlang:main Jun 29, 2025
7 checks passed
@hamishknight hamishknight deleted the solve-escape branch June 29, 2025 01: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