We noticed that a project failed to build with prepared overloads enabled,
but built successfully with prepared overloads disabled.
It turns out that the code clearly should never have been accepted in the
first place, because the type checker was making an arbitrary choice between
two nominal type declarations with the same name.
Further inspection revealed this was because of a FIXME added in 2013 which
was far too broad. Tighten up the logic here to only disambiguate if at least
one of the two declarations is a type alias, and it has the same underlying
type as the other one. A choice between unrelated nominal type declarations
should always be ambiguous.
This still isn't perfect, because we might have two generic type aliases
that are referenced in both solutions with different substitution maps,
in which case we will still erroneously pick the first one. But this
is better than the old logic, at least.
Fixes rdar://165863775.
We've had a load-bearing optimization that avoids rebuilding
the generic signature of the protocol extension to avoid a
cycle involving a typealias in the extension that's mentioned
as a requirement in the protocol, for example:
```
public protocol P3 {
associatedtype A
}
public protocol P4: P3 where A == B {}
extension P4 {
public typealias B = String
}
```
What was happening is that we're inside a
StructuralRequirementsRequest, so we're performing TypeResolution
in the Structural stage. When we encounter this typealias that's
within a protocol extension, we'd end up accidentally requesting
the interface type of that alias through
`TypeChecker::substMemberTypeWithBase`
(via `aliasDecl->getUnderlyingType()`).
What we should do instead is call `aliasDecl->getStructuralType()`
and skip the substitution during that stage.
There happened to already be code doing this, but it was only
kicking in if the DeclContext `isa<ProtocolDecl>`, which excludes
extensions of a protocol. I see no reason why extensions of a
protocol should be excluded, so I assume it was unintentional.
Thus, the fix boils down to using `DeclContext::getSelfProtocolDecl`
which does include extensions of protocols. With this fix, the
optimization is no longer load-bearing on the example above.
See the history of this hack in f747121080
or rdar://problem/129540617
When a DeclRefTypeRepr is bound to a known declaration, the exact DeclNameRef used to create it is erased. This means that we no longer know exactly which module selector was used in the source code, so repeating the lookup with the same instance might produce different results.
Avoid this problem by reusing the bound decl instead of trying to look up the same declaration again.
Fixes rdar://164647850.
If we have a custom attribute on a type that does a qualified lookup
into the same type, we need to be able to expand member attribute
macros for that type. As such, the check to see if we already have
a nominal for the attribute would hit a cycle. Limit this check such
that it only applies to local vars, which is the only case where it
actually matters.
rdar://163961797
In code like the following:
```
protocol P { associatedtype A: Hashable }
protocol Q { associatedtype A: Comparable }
func fn<T: P & Q>(_: T) where T.A == Int { … }
```
`T.A` is actually the union of `P.A` and `Q.A`—it satisfies both associated types and has both of their constraints. This means it doesn’t actually make sense to apply a module selector to `A`—even if `P` and `Q` are in different modules, `T.A` always represents both of the declarations, not one or the other. We therefore now ban module selectors in this position, since they don’t actually jibe with the nature of a generic signature.
This justification technically doesn’t hold for *every* member type of a generic parameter—a member type can refer to a concrete typealias in a protocol extension, for instance—but in those situations, you can disambiguate (and add module selectors) by writing `P.A` or `Q.A` instead of `T.A`, so we’re not really worried about this limitation.
Lookups like Builtin::Int64 were failing because BuiltinUnit rejected all unqualified lookups. Make it allow unqualified lookups with a module selector.
Missed this in my previous patch, avoid relying on the `in` location
for closures with parameters and capture lists. Instead, form scopes
that start at the first element of the body. This fixes a crasher
uncovered by the fuzzer.
We don't properly support peer declarations in local contexts, as such
if we have both a property wrapper and macro with a given name on a
local var, let's prefer the property wrapper.
rdar://148782639
Filter out any duplicate notes to help cut down on the noise for
request cycle diagnostics. Some of the note locations here still aren't
great, but this at least stops us from repeating them for each
intermediate request.
These are tests that fail in the next commit without this flag. This
does not add -verify-ignore-unrelated to all tests with -verify, only
the ones that would fail without it. This is NFC since this flag is
currently a no-op.
The changes from https://github.com/swiftlang/swift/pull/84259/ caused a
regression in which declarations from an outer scope could be shadowed
inappropriately by member declarations from a module that has not been
imported. This fix addresses the issue by refactoring `resolveDeclRefExpr()` so
that it performs lookups in two passes. First, it attempts to resolve the decl
ref using the complete lookup algorithm and standard name lookup options, which
will ignore members from modules that haven't been imported if
`MemberImportVisibility` is enabled. Then, if no results were found it re-runs
the full lookup algorithm again with `NameLookupFlags::IgnoreMissingImports`
included to find additional results that ought to be diagnosed as unavailable.
This insures that the unavailable results are not considered until after the
main lookup has already failed.
Resolves rdar://161078015.
@c @implementation relies on matching the original C declaration. The
lookup for the original C declaration was doing the wrong kind of
lookup, meaning that it could only find the C declaration if it came
through a bridging header, and not through a normal module import.
Using unqualified lookup here finds the name appropriately.
Clarify the diagnostics here as well to not talk about umbrella and
bridging headers.
Fixes rdar://161909754.
`TypeChecker::resolveDeclRefExpr()` would leave error nodes in the AST when
performing fallback name lookups with `MemberImportVisibility`. When running in
migration mode for `MemberImportVisibility`, these error nodes would then cause
the compiler to either diagnose a missing error or to crash during SILGen.
Instead of restricting name lookup during `TypeChecker::resolveDeclRefExpr()`,
allow it to find candidates with missing imports and then diagnose them if
necessary before forming a resolve decl ref.
Resolves rdar://154361861.
The diagnostic pass that checks whether expressions reference member
declarations that have not been imported failed to handle key path expressions.
Resolves rdar://159093481.
The migration to `MemberImportVisibility` can be performed mechanically by
adding missing import declarations, so offer automatic migration for the
feature.
Resolves rdar://151931597.
The was never invoked because inaccessibility due to SPI protection level is
always diagnosed before missing imports are diagnosed. The functionality could
therefore not be tested and should be removed.
Omit an explicit access level from `MemberImportVisibility` fix-its under the
following conditions:
- `InternalImportsByDefault` is enabled.
- The required import needs an `internal` access level or lower.
- The module is not yet imported explicitly in any other file.
Resolves rdar://149577615.
Unlike in Swift, Obj-C allows method overrides to be declared in extensions
(categories), even outside of the module that defines the type that is being
extended. When MemberImportVisibility is enabled, these overrides must be
filtered out to prevent them from hijacking name lookup and causing the
compiler to insist that the module that defines the extension be imported.
Resolves rdar://145329988.
