Skip to content

Conversation

hamishknight
Copy link
Contributor

Reapply #36028 with an even stricter set of assertions to ensure we never have a solution with errors or holes in CSApply. Depends on #84279.

@hamishknight
Copy link
Contributor Author

@swift-ci please test source compatibility

@hamishknight
Copy link
Contributor Author

@swift-ci please SourceKit stress test

@slavapestov
Copy link
Contributor

Nice!

@hamishknight
Copy link
Contributor Author

After this I'd like to try and see if we can replace UnresolvedType with ErrorType

@xedin
Copy link
Contributor

xedin commented Sep 14, 2025

Thanks! I will take a look at this one and others on monday.

@hamishknight
Copy link
Contributor Author

Oh fun, TIL we allow PlaceholderTypes in interface types for e.g FuncDecls, specifically so we can diagnose them later in MiscDiagnostics and suggest replacing with the inferred return type. That explains this existing SILGen crasher:

protocol a {
  func b() -> _
}
do {
  func c(d : a) {
    let e = d.b
  }
}

But it also means that the following crashes with this change:

func foo() -> _ {}
foo()

Looks like we need to at least record a fix to stop the constraint system from attempting to apply the solution here (as well as ensuring we also reject PlaceholderTypes in FuncDecls with no body)

@hamishknight
Copy link
Contributor Author

Fixing the above case in #84345

hamishknight and others added 5 commits September 17, 2025 16:11
Using `*` previously meant we'd invoke undefined behavior.
It's not actually clear these are necessary anymore, and in one case
it actually makes a diagnostic worse.
And ensure we don't end up with invalid key paths in CSApply.
I believe these code paths could only be reached by re-typechecking
invalid code in the old CSDiag implementation.
Enforce that we don't ever encounter solutions that contain holes or
error types.
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@hamishknight
Copy link
Contributor Author

@swift-ci please test Linux

@hamishknight
Copy link
Contributor Author

@swift-ci please smoke test Linux

@hamishknight
Copy link
Contributor Author

swiftlang/swift-package-manager#9153

@swift-ci please smoke test Linux

@hamishknight hamishknight merged commit deab5f7 into swiftlang:main Sep 18, 2025
4 of 5 checks passed
@hamishknight hamishknight deleted the csapply-cleanup branch September 18, 2025 13:26
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