[Name lookup] Enable shadowing for type lookup and of the Swift module. #21370
Conversation
… rather than co-opting NULL, which is a valid CanType type.
Tweak the shadowing rules in two ways: 1) For unqualified type lookup, apply shadowing rules. Without this, we would always get an ambiguity if there were two types with the same name, so this should be a strict improvement. 2) Allow a name introduced in any other module to shadow a name in the Swift standard library. This is (another) weak form of a more sensible, generalized rule that would use the import graph to describe shadowing. Together, these tweaks allow the Result type that was recently introduced in the standard library to exist without breaking source compatibility for Swift code that is already using a Result type. The user Result type will shadow (hide) the Swift one. The latter can be spelled Swift.Result if it is needed by such code. Fixes rdar://problem/46767892.
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
1 similar comment
|
@swift-ci please test source compatibility |
We’re going to need to use this in GenericSignature.cpp. NFC
Narrow the recently-introduced shadowing rule for types so that it only applies to non-member types. Member types lack a reasonable syntax for specifying precisely which module to look into, and there are a few use cases where the type checker will pick a type that would be shadowed by the new rule. Fixes a source-compatibility regression introduced by the shadowing rule.
When substituting type parameters to compute a canonical type within a given generic context, also handle the case where the type parameter is directly known to be a concrete type. The type checker won’t do this, but SourceKit does. Fixes a regression in SourceKit/DocSupport/doc_stdlib.swift.
|
The test failure was real (a bug in |
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
| @@ -1169,6 +1191,11 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC, | |||
| lookupInModule(&M, {}, Name, CurModuleResults, NLKind::UnqualifiedLookup, | |||
| resolutionKind, TypeResolver, DC, extraImports); | |||
|
|
|||
| // Always perform name shadowing for type lookup. | |||
| if (options.contains(Flags::TypeLookup)) { | |||
There was a problem hiding this comment.
This is iffy—it means that expression context lookup will behave differently than type context lookup. Or does that happen elsewhere?
There was a problem hiding this comment.
It does happen elsewhere. Most unqualified lookup goes through TypeChecker::lookupUnqualified, which (separately) triggers shadowing, so this is bringing things closer together in a duct-tape-and-paper-clip sort of way. There's more refactoring to do, but I'm holding out hope for this to be safe enough for Swift 5.0, hence the narrow applicability (to type lookup, to the Swift module, to non-member types);.
There was a problem hiding this comment.
All right, makes sense. Maybe worth a FIXME, though?
| @@ -44,6 +44,7 @@ | |||
| #include "llvm/Support/raw_ostream.h" | |||
| #include "llvm/Support/SaveAndRestore.h" | |||
| #include <algorithm> | |||
| #include "GenericSignatureBuilderImpl.h" | |||
There was a problem hiding this comment.
Nitpick: LLVM style says local headers go at the top.
There was a problem hiding this comment.
Ah, of course. Committing locally to fix at some point, but I'm not going to re-start CI for that right now.
|
All of the source-compatibility failures are UPASS's. |
Tweak the shadowing rules in two ways:
would always get an ambiguity if there were two types with the same name,
so this should be a strict improvement.
Swift standard library. This is (another) weak form of a more sensible,
generalized rule that would use the import graph to describe shadowing.
Together, these tweaks allow the Result type that was recently introduced in
the standard library to exist without breaking source compatibility for
Swift code that is already using a Result type. The user Result type will
shadow (hide) the Swift one. The latter can be spelled Swift.Result if it
is needed by such code.
Fixes rdar://problem/46767892.