[5.5][CSSimplify] Allow overload choices with missing labels to be considered for diagnostics #37137
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…new-branch [5.5] Add support for release/5.5 branch in update-checkout script
Combine IsNotRecommended with NotRecommendedReason and improve the names of the existing cases to more clearly identify the cases they apply to. Now all not-recommended completions are required to have a reason.
…Pointer rdar://72164992
…mprovement-5.5 [5.5][completion] Clarify and simplify not-recommended state
Destructors can't be run on the main actor since they may be called from a asynchronous context. Because we are the only ones with a handle to the class, we can touch Main-Actor protected properties from anywhere safely. We can't touch static properties though since those will still be alive and accessible from outside of just this dying instance.
Updated the tests for non-global-actor protected deinit. We can't access main-actor synchronous functions, main-actor static properties, but can access non-static main-actor stored properties in the deinit.
[5.5] ClangImporter: add some error-recovery code path for TypeBase::wrapInPointer
When allowing errors there's various cases where an invalid type is used during serialization: - Invalid explicit conformances on an extension - Superclass with invalid generic type Add checks to skip these to avoid crashing. Resolves rdar://75379780.
… concrete as well Otherwise, when we resolve the subject type while adding the requirement to the new signature, we assert that it is not a dependent type. This fixes a regression from 977d3a7. Fixes rdar://problem/76750100.
[5.5][Serialization] Add some checks for invalid types
…’s not on top-level We fail to resolve the extended type if an extension is not declared at the top level. This causes us to diagnose a lookup failure on `Self`. If the extended type is a protocol, we thus end up with a nominal Self type that’s not a class, violating the assertion that’s currently in place. To fix the crasher, convert the assertion to an `if` condition and issue a generic "cannot find type 'Self' in scope" in the else case. Cherry-picking 78d6fb2 from the `main` branch.
[AsyncLet] reimplemented with new ABI and builtins [AsyncLet] remove runChild; it is now asyncLetStart [AsyncLet] ending an async let cancels the task [AsyncLet] remove some commented out code [AsyncLet] silence issue on windows [Concurrency] Store child record when async let child task spawned [AsyncLet] reimplemented with new ABI and builtins [AsyncLet] remove runChild; it is now asyncLetStart [AsyncLet] ending an async let cancels the task [AsyncLet] remove some commented out code
… let child task spawned"
Narrow fix for rdar://76860623. (cherry picked from commit 4d6f07e)
Introduce flags `-enable-actor-data-race-checks` and `-disable-actor-data-race-checks` to enable/disable emission of code that checks that we are on the correct actor. Default to `false` for now but make it easy to enable in the future. (cherry picked from commit 19a7fa6)
…t-implied-by-concrete-5.5 GSB: When rebuilding a signature, drop layout requirements implied by concrete as well [5.5]
Add a new parameter attribute `@_implicitSelfCapture` that disables the requirement to explicitly use `self.` to refer to a member of `self` in an escaping closure. Part of rdar://76927008. (cherry picked from commit abfc9bc)
This saves us from needing to re-match args to params in CSApply and is also useful for a forthcoming change migrating code completion in argument position to use the solver-based typeCheckForCodeCompletion api. rdar://76581093 (cherry picked from commit c57c403)
…or-deinit Cherry-Pick: [Concurrency] Don't propagate global actor-ness to deinit
…aner-abi-asynclet
…-flags-5.5 [5.5] [SILGen] Put actor data-race checking behind a flag.
There are a number of occurances that create implicit `Switch`s by passing `SourceLoc()` for all location paramters. Refactor those occurances out to a separate `createImplicit` method that automatically fills the locations with invalid source locations.
Cherry-Pick 5.5: [TypeChecker] Fix assertion failure when using Self in extension that’s not on top-level
…wiftlang#36982) Me, sobbing: "Look, you can't just point at an empty struct and call it a RandomNumberGenerator." Swift 5.4, pointing at anything: "RNG."
When a function declaration has no body (e.g. because it’s a protocol requirement), we construct the range to replace by the `async` keyword as follows: - Start: One character after the closing `)` (or potentially the `throws` keyword if it exists) - End: Last token in the function declaration Since the last token in the function declaration is the `)`, we end up with a range that has `End < Start`, which crashes when trying to print the range. If the function has no body, we should just use the range’s start location as the end location to construct an empty range. Fixes rdar://76677035
(cherry picked from commit 29b3411fbc05c46ea53598207a563139f9364d75)
…or-printing-simplified-async-resume-functions Demangling: Add option for printing simplified async resume functions
`ContextualFailure` is the main beneficiary of additional information associated with `ContextualType` element because it no longer has to query solution for "purpose" of the contextual information. Resolves: rdar://68795727 (cherry picked from commit 71372bc)
[5.5][Concurrency] Fix await fix-it location
[5.5][Diagnostics] Avoid relying on solution for contextual purpose information
Not-filtering solutions causes unacceptable slownesses in some cases. For now, filter solutions as normal typechecking does to restore the performance. rdar://76714968 (cherry picked from commit 8480318)
[5.5][TypeChecker] PreCheck: Don't strip away tuple/paren from subscripts …
…ave members Since 9ba892c we always transform `CurrentType` in `ASTPrinter` to be an interface type. This causes issues when the variable type is a generic parameter type. Previously we had `CurrentType` set to a `PrimaryArchetypeType`. With the fix in d93ae06, we are mapping the archetype out of context to a `GenericParamType`. A `GenericParamType`, however, can’t have members, so we’re hitting an assertion when creating a `TypeTransformContext`. Since the entire type transformation in `printTransformedTypeWithOptions` is only affecting type members, we should be able to safely skip over the transformation if `CurrentType` can’t have any members. Fixes rdar://76750555 [SR-14497]
Consider the following example.
```swift
protocol FontStyle {}
struct FontStyleOne: FontStyle {}
extension FontStyle where Self == FontStyleOne {
static var one: FontStyleOne { FontStyleOne() }
}
func foo<T: FontStyle>(x: T) {}
func case1() {
foo(x: .#^COMPLETE^#)
}
```
With SE-0299 accepted, we should be suggesting `.one`.
For that, we need to consider the extension applied when performing the unresolved dot code completion with a primary archetype that conforms to `FontStyle`.
However, in the following case, which performs an unresolved dot completion on the same base type, we don't want to suggest `.one` because that would require `T == FontStyleOne`, which we can’t assume.
```swift
func case2<T: FontStyle>(x: T) {
x.#^COMPLETE_2^#
}
```
Since the constraint system cannot tell us how it came up with the archetype, we need to apply a heuristic to differentiate between the two cases.
What seems to work fine in most cases, is to determine if `T` referes to a generic parameter that is visible from the decl context we are completing in (i.e. the decl context we are completing in is a child context of the context that `T` is declared in). If it is not, then `T` cannot be the type of a variable we are completing on. Thus, we are in the first case and we should consider all extensions of `FontStyle` because we can further specialize `T` by picking a more concrete type.
Otherwise `T` may be the type of a variable we are completing on and we should be conservative and only suggest those extensions whose requirements are fulfilled by `T`.
Since this is just a heuristic, there are some corner cases, where we aren’t suggesting constrainted extensions although we should. For example, in the following example the call to `testRecursive` doesn’t use `T` and we should thus suggest `one`. But by the rules described above we detect that `T` is accessible at the call and thus don’t apply extension whose requirements aren’t satisfied.
```swift
func testRecursive<T: FontStyle>(_ style: T) {
testRecursive(.#^COMPLETE_RECURSIVE_GENERIC^#)
}
```
Similar completion issues also occurred without SE-0299 in more complicated, generic scenarios.
Resolves rdar://74958497 and SR-12973
…d generic type
In the following test case, we are crashing while building the generic signature of `someGenericFunc`, potentially invoked on `model` in line 11.
```swift
struct MyBinding<BindingOuter> {
func someGenericFunc<BindingInner>(x: BindingInner) {}
}
struct MyTextField<TextFieldOuter> {
init<TextFieldInner>(text: MyBinding<TextFieldInner>) {}
}
struct EncodedView {
func foo(model: MyBinding<String>) {
let _ = MyTextField<String>(text: model)
}
}
```
Because we know that `model` has type `MyBinding<TextFieldInner>`, we substitute the `BindingOuter` generic parameter by `TextFieldInner`. Thus, `someGenericFunc` has the signature `<TextFieldInner /* substitutes BindingOuter */, BindingInner>`. `TextFieldInner` and `BindingOuter` both have `depth = 1`, `index = 0`. Thus the verification in `GenericSignatureBuilder` is failing.
After discussion with Slava, the root issue appears to be that we shouldn’t be calling `subst` on a `GenericFunctionType` at all. Instead we should be using `substGenericArgs` which doesn’t attempt to rebuild a generic signature, but instead builds a non-generic function type.
--------------------------------------------------------------------------------
We slightly regress in code completion results by showing two `collidingGeneric` twice in the following case.
```swift
protocol P1 {
func collidingGeneric<T>(x: T)
}
protocol P2 {
func collidingGeneric<T>(x: T)
}
class C : P1, P2 {
#^COMPLETE^#
}
```
Previously, we were representing the type of `collidingGeneric` by a generic function type with generic param `T` that doesn’t have any restrictions. Since we are now using `substGenericArgs` instead of `subst`, we receive a non-generic function type that represents `T` as an archetype. And since that archetype is different for the two function signatures, we show the result twice in code completion.
One could also argue that showing the result twice is intended (or at least acceptable) behaviour since, the two protocol may name their generic params differently. E.g. in
```swift
protocol P1 {
func collidingGeneric<S>(x: S)
}
protocol P2 {
func collidingGeneric<T>(x: T)
}
class C : P1, P2 {
#^COMPLETE^#
}
```
we might be expected to show the following two results
```
func collidingGeneric<S>(x: S)
func collidingGeneric<T>(x: T)
```
Resolves rdar://76711477 [SR-14495]
…-members [5.5][SourceKit] Don’t transform type when printing if CurrentType can’t have members
…constrained-extension [5.5][CodeComplete] Show completions from constrained protocol extension
[5.5][CodeCompletion] Peform complete `filterSolutions` in code completion
…pped value mismatch. If there's an error in property wrapper application, the backing property wrapper type request will return a null type, which would cause the compiler to crash because most error recovery code expects ErrorType. If the wrapper application has an error, use the interface type of the parameter instead.
Cross-module incremental builds require a stable source of fingerprint information for iterable decl contexts. This is provided by the incremental frontends when they produce partial swift module files. Embedded in these files is a table of fingerprints, which are consumed by merge-modules to construct a module-wide dependency graph that is then serialized into the final merged swift module file. Unfortunately, the implementation here iterated through the files in the module and asked for the first fingerprint that would load for a particular iterable decl context. If (more likely, when) the DeclID for that serialized iterable decl context collided with another DeclID in the wrong file, we would load that fingerprint instead. Locate up to the module-scope context for an iterable decl context and only load the fingerprint from there. This ensures that the fingerprints in the partial modules matches the fingerprints in the merged modules. rdar://77005039
…mapping-5.5 [5.5] SILModule: track opened archetypes per function.
…n(block)` We already have special logic to extrac the closure for closures with capture lists, add the same kind of logic for closures that are marked `@convention(block)` etc. Resolves rdar://75301524 [SR-14328]
…params-mismatch [5.5][Sema] Fix crash when retrieving typeContextInfo for a partially bound generic type
This changes how ReflectionContext reads machO reflection sections by reading them individually instead of as one big memory block spanning from the first to the last section (and including whatever else is in between). This change will enable an optimization on LLDB's side where, if we're reading read-only data, we read from the file-cache instead of the child process, which should speed up debugging when working with remote processes. (cherry picked from commit b54d426)
…iagnostic-crash [5.5][Property Wrappers] Fix a diagnostic crash when a parameter has a wrapped value mismatch.
When there are symbols 'Label' and 'label', if a user type 'Lab', 'Label' should be prioritized. rdar://77164709 (cherry picked from commit 75ff948)
Read machO sections section by section
…ion-rdar77164709 [5.5][CodeCompletion] Boost exact case-sensitive prefix match
…on-to-async [5.5][Refactoring] Support refactoring to async if callback is `@convention(block)`
(cherry picked from commit 957e05c)
…red for diagnostics Let's make use of a newly added "disable for performance" flag to allow solver to consider overload choices where the only issue is missing one or more labels - this makes it for a much better diagnostic experience without any performance impact for valid code. (cherry picked from commit f36ecf2)
AnthonyLatsis
added
🍒 release cherry pick
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-pick of #37115
Introduce a new constraint flag - "disable for performance" and use it to
allow solver to consider overload choices where the only issue is
missing one or more labels - this makes it for a much better
diagnostic experience without any performance impact for valid code.
This is a workaround for the problem where
simplifyAppliedOverloadsImpldoes filtering during constraint generation and disabled choices stay that
way permanently since there is no rollback mechanism for changes to
constraint system that happen before solver scopes are involved.