-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Name lookup] Enable shadowing for type lookup and of the Swift module. #21370
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
Changes from all commits
aa5dc6c
9f61301
fc7eb0d
a2a663c
bbe3aa7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
// | ||
// GenericSignatureBuilderImpl.h | ||
// Swift | ||
// | ||
// Created by Doug Gregor on 12/17/18. | ||
// | ||
|
||
#ifndef SWIFT_AST_GENERIC_SIGNATURE_BUILDER_IMPL_H | ||
#define SWIFT_AST_GENERIC_SIGNATURE_BUILDER_IMPL_H | ||
|
||
#include "swift/AST/GenericSignatureBuilder.h" | ||
|
||
namespace swift { | ||
|
||
class GenericSignatureBuilder::ResolvedType { | ||
llvm::PointerUnion<PotentialArchetype *, Type> type; | ||
EquivalenceClass *equivClass; | ||
|
||
/// For a type that could not be resolved further unless the given | ||
/// equivalence class changes. | ||
ResolvedType(EquivalenceClass *equivClass) | ||
: type(), equivClass(equivClass) { } | ||
|
||
public: | ||
/// A specific resolved potential archetype. | ||
ResolvedType(PotentialArchetype *pa) | ||
: type(pa), equivClass(pa->getEquivalenceClassIfPresent()) { } | ||
|
||
/// A resolved type within the given equivalence class. | ||
ResolvedType(Type type, EquivalenceClass *equivClass) | ||
: type(type), equivClass(equivClass) { | ||
assert(type->isTypeParameter() == static_cast<bool>(equivClass) && | ||
"type parameters must have equivalence classes"); | ||
} | ||
|
||
/// Return an unresolved result, which could be resolved when we | ||
/// learn more information about the given equivalence class. | ||
static ResolvedType forUnresolved(EquivalenceClass *equivClass) { | ||
return ResolvedType(equivClass); | ||
} | ||
|
||
/// Return a result for a concrete type. | ||
static ResolvedType forConcrete(Type concreteType) { | ||
return ResolvedType(concreteType, nullptr); | ||
} | ||
|
||
/// Determine whether this result was resolved. | ||
explicit operator bool() const { return !type.isNull(); } | ||
|
||
/// Retrieve the dependent type. | ||
Type getDependentType(GenericSignatureBuilder &builder) const; | ||
|
||
/// Retrieve the concrete type, or a null type if this result doesn't store | ||
/// a concrete type. | ||
Type getAsConcreteType() const { | ||
assert(*this && "Doesn't contain any result"); | ||
if (equivClass) return Type(); | ||
return type.dyn_cast<Type>(); | ||
} | ||
|
||
/// Realize a potential archetype for this type parameter. | ||
PotentialArchetype *realizePotentialArchetype( | ||
GenericSignatureBuilder &builder); | ||
|
||
/// Retrieve the potential archetype, if already known. | ||
PotentialArchetype *getPotentialArchetypeIfKnown() const { | ||
return type.dyn_cast<PotentialArchetype *>(); | ||
} | ||
|
||
/// Retrieve the equivalence class into which a resolved type refers. | ||
EquivalenceClass *getEquivalenceClass( | ||
GenericSignatureBuilder &builder) const { | ||
assert(*this && "Only for resolved types"); | ||
if (equivClass) return equivClass; | ||
|
||
// Create the equivalence class now. | ||
return type.get<PotentialArchetype *>() | ||
->getOrCreateEquivalenceClass(builder); | ||
} | ||
|
||
/// Retrieve the equivalence class into which a resolved type refers. | ||
EquivalenceClass *getEquivalenceClassIfPresent() const { | ||
assert(*this && "Only for resolved types"); | ||
if (equivClass) return equivClass; | ||
|
||
// Create the equivalence class now. | ||
return type.get<PotentialArchetype *>()->getEquivalenceClassIfPresent(); | ||
} | ||
|
||
/// Retrieve the unresolved result. | ||
EquivalenceClass *getUnresolvedEquivClass() const { | ||
assert(!*this); | ||
return equivClass; | ||
} | ||
|
||
/// Return an unresolved type. | ||
/// | ||
/// This loses equivalence-class information that could be useful, which | ||
/// is unfortunate. | ||
UnresolvedType getUnresolvedType() const { | ||
return type; | ||
} | ||
}; | ||
|
||
} // end namepsace swift | ||
|
||
#endif // SWIFT_AST_GENERIC_SIGNATURE_BUILDER_IMPL_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -254,6 +254,24 @@ static void recordShadowedDeclsAfterSignatureMatch( | |
break; | ||
} | ||
|
||
// Prefer declarations in the any module over those in the standard | ||
// library module. | ||
if (auto swiftModule = ctx.getStdlibModule()) { | ||
if ((firstModule == swiftModule) != (secondModule == swiftModule)) { | ||
// If the second module is the standard library module, the second | ||
// declaration is shadowed by the first. | ||
if (secondModule == swiftModule) { | ||
shadowed.insert(secondDecl); | ||
continue; | ||
} | ||
|
||
// Otherwise, the first declaration is shadowed by the second. There is | ||
// no point in continuing to compare the first declaration to others. | ||
shadowed.insert(firstDecl); | ||
break; | ||
} | ||
} | ||
|
||
// Prefer declarations in an overlay to similar declarations in | ||
// the Clang module it customizes. | ||
if (firstDecl->hasClangNode() != secondDecl->hasClangNode()) { | ||
|
@@ -336,26 +354,33 @@ static void recordShadowedDecls(ArrayRef<ValueDecl *> decls, | |
} | ||
} | ||
|
||
// We need an interface type here. | ||
if (typeResolver) | ||
typeResolver->resolveDeclSignature(decl); | ||
CanType signature; | ||
|
||
// If the decl is currently being validated, this is likely a recursive | ||
// reference and we'll want to skip ahead so as to avoid having its type | ||
// attempt to desugar itself. | ||
if (!decl->hasValidSignature()) | ||
continue; | ||
if (!isa<TypeDecl>(decl)) { | ||
// We need an interface type here. | ||
if (typeResolver) | ||
typeResolver->resolveDeclSignature(decl); | ||
|
||
// FIXME: the canonical type makes a poor signature, because we don't | ||
// canonicalize away default arguments. | ||
auto signature = decl->getInterfaceType()->getCanonicalType(); | ||
// If the decl is currently being validated, this is likely a recursive | ||
// reference and we'll want to skip ahead so as to avoid having its type | ||
// attempt to desugar itself. | ||
if (!decl->hasValidSignature()) | ||
continue; | ||
|
||
// FIXME: The type of a variable or subscript doesn't include | ||
// enough context to distinguish entities from different | ||
// constrained extensions, so use the overload signature's | ||
// type. This is layering a partial fix upon a total hack. | ||
if (auto asd = dyn_cast<AbstractStorageDecl>(decl)) | ||
signature = asd->getOverloadSignatureType(); | ||
// FIXME: the canonical type makes a poor signature, because we don't | ||
// canonicalize away default arguments. | ||
signature = decl->getInterfaceType()->getCanonicalType(); | ||
|
||
// FIXME: The type of a variable or subscript doesn't include | ||
// enough context to distinguish entities from different | ||
// constrained extensions, so use the overload signature's | ||
// type. This is layering a partial fix upon a total hack. | ||
if (auto asd = dyn_cast<AbstractStorageDecl>(decl)) | ||
signature = asd->getOverloadSignatureType(); | ||
} else if (decl->getDeclContext()->isTypeContext()) { | ||
// Do not apply shadowing rules for member types. | ||
continue; | ||
} | ||
|
||
// Record this declaration based on its signature. | ||
auto &known = collisions[signature]; | ||
|
@@ -1169,6 +1194,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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does happen elsewhere. Most unqualified lookup goes through There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All right, makes sense. Maybe worth a FIXME, though? |
||
removeShadowedDecls(CurModuleResults, &M); | ||
} | ||
|
||
for (auto VD : CurModuleResults) | ||
Results.push_back(LookupResultEntry(VD)); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
public enum Result<Value, Error> { | ||
case success(Value) | ||
case failure(Error) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
public class RootClass { | ||
} | ||
|
||
public class SubClass: RootClass { | ||
public typealias Member = Int | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// RUN: %empty-directory(%t) | ||
// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/MemberTypesInClasses.swift | ||
// RUN: %target-swift-frontend -typecheck %s -I %t -verify | ||
|
||
import MemberTypesInClasses | ||
|
||
protocol P { | ||
associatedtype Member | ||
} | ||
|
||
extension RootClass: P { | ||
typealias Member = SubClass.Member | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
// RUN: %empty-directory(%t) | ||
// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/HasResult.swift | ||
// RUN: %target-swift-frontend -typecheck %s -I %t -verify | ||
|
||
import HasResult | ||
|
||
func foo() -> Result<Int, Error> { | ||
return Result<Int, Error>.success(42) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: LLVM style says local headers go at the top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, of course. Committing locally to fix at some point, but I'm not going to re-start CI for that right now.