Skip to content

[GSB] Start inferring same-type requirements from inherited type declarations #9064

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 96 additions & 16 deletions lib/AST/GenericSignatureBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,31 @@ static unsigned sourcePathLength(const RequirementSource *source) {
return count;
}

/// Check whether we have a NestedTypeNameMatch/ProtocolRequirement requirement
/// pair, in which case we prefer the RequirementSignature.
///
/// This is part of staging out NestedTypeNameMatch requirement sources in
/// favor of something more principled.
static int isNestedTypeNameMatchAndRequirementSignaturePair(
const RequirementSource *lhs,
const RequirementSource *rhs) {
if (lhs->getRoot()->kind == RequirementSource::NestedTypeNameMatch &&
rhs->isProtocolRequirement())
return +1;

if (rhs->getRoot()->kind == RequirementSource::NestedTypeNameMatch &&
lhs->isProtocolRequirement())
return -1;

return 0;
}

int RequirementSource::compare(const RequirementSource *other) const {
// FIXME: Egregious hack while we phase out NestedTypeNameMatch
if (int compare =
isNestedTypeNameMatchAndRequirementSignaturePair(this, other))
return compare;

// Prefer the derived option, if there is one.
bool thisIsDerived = this->isDerivedRequirement();
bool otherIsDerived = other->isDerivedRequirement();
Expand Down Expand Up @@ -2459,7 +2483,7 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement(

// Collect all of the inherited associated types and typealiases in the
// inherited protocols (recursively).
DenseMap<DeclName, TinyPtrVector<TypeDecl *>> inheritedTypeDecls;
llvm::MapVector<DeclName, TinyPtrVector<TypeDecl *>> inheritedTypeDecls;
{
Proto->walkInheritedProtocols(
[&](ProtocolDecl *inheritedProto) -> TypeWalker::Action {
Expand Down Expand Up @@ -2523,17 +2547,58 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement(
return result;
};

// Form an unsubstituted type referring to the given type declaration,
// for use in an inferred same-type requirement.
auto formUnsubstitutedType = [&](TypeDecl *typeDecl) -> Type {
if (auto assocType = dyn_cast<AssociatedTypeDecl>(typeDecl)) {
return DependentMemberType::get(
assocType->getProtocol()->getSelfInterfaceType(),
assocType);
}

if (auto typealias = dyn_cast<TypeAliasDecl>(typeDecl)) {
// Resolve the underlying type, if we haven't done so yet.
if (!typealias->hasInterfaceType()) {
getLazyResolver()->resolveDeclSignature(typealias);
}

return typealias->getUnderlyingTypeLoc().getType();
}

return Type();
};

// An an inferred same-type requirement between the two type declarations
// within this protocol or a protocol it inherits.
auto addInferredSameTypeReq = [&](TypeDecl *first, TypeDecl *second) {
Type firstType = formUnsubstitutedType(first);
if (!firstType) return;

Type secondType = formUnsubstitutedType(second);
if (!secondType) return;

auto inferredSameTypeSource =
FloatingRequirementSource::viaProtocolRequirement(
Source, Proto, WrittenRequirementLoc(), /*inferred=*/true);

addRequirement(
Requirement(RequirementKind::SameType, firstType, secondType),
inferredSameTypeSource, Proto->getParentModule(),
&protocolSubMap);
};

// Add requirements for each of the associated types.
for (auto Member : getProtocolMembers(Proto)) {
if (auto AssocType = dyn_cast<AssociatedTypeDecl>(Member)) {
if (auto assocTypeDecl = dyn_cast<AssociatedTypeDecl>(Member)) {
// Add requirements placed directly on this associated type.
Type assocType = DependentMemberType::get(concreteSelf, AssocType);
Type assocType = DependentMemberType::get(concreteSelf, assocTypeDecl);
auto assocResult =
addInheritedRequirements(AssocType, assocType, Source, protoModule);
addInheritedRequirements(assocTypeDecl, assocType, Source, protoModule);
if (isErrorResult(assocResult))
return assocResult;

if (auto WhereClause = AssocType->getTrailingWhereClause()) {
// Add requirements from this associated type's where clause.
if (auto WhereClause = assocTypeDecl->getTrailingWhereClause()) {
for (auto &req : WhereClause->getRequirements()) {
auto innerSource =
FloatingRequirementSource::viaProtocolRequirement(
Expand All @@ -2543,30 +2608,33 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement(
}

// Check whether we inherited any types with the same name.
auto knownInherited = inheritedTypeDecls.find(AssocType->getFullName());
auto knownInherited =
inheritedTypeDecls.find(assocTypeDecl->getFullName());
if (knownInherited == inheritedTypeDecls.end()) continue;

bool shouldWarnAboutRedeclaration =
Source->kind == RequirementSource::RequirementSignatureSelf &&
AssocType->getDefaultDefinitionLoc().isNull();
for (auto inheritedType : knownInherited->getSecond()) {
assocTypeDecl->getDefaultDefinitionLoc().isNull();
for (auto inheritedType : knownInherited->second) {
// If we have inherited associated type...
if (auto inheritedAssocTypeDecl =
dyn_cast<AssociatedTypeDecl>(inheritedType)) {
// FIXME: Wire up same-type constraint.
// Infer a same-type requirement among the same-named associated
// types.
addInferredSameTypeReq(assocTypeDecl, inheritedAssocTypeDecl);

// Complain about the first redeclaration.
if (shouldWarnAboutRedeclaration) {
auto inheritedFromProto = inheritedAssocTypeDecl->getProtocol();
auto fixItWhere = getProtocolWhereLoc();
Diags.diagnose(AssocType,
Diags.diagnose(assocTypeDecl,
diag::inherited_associated_type_redecl,
AssocType->getFullName(),
assocTypeDecl->getFullName(),
inheritedFromProto->getDeclaredInterfaceType())
.fixItInsertAfter(
fixItWhere.first,
getAssociatedTypeReqs(AssocType, fixItWhere.second))
.fixItRemove(AssocType->getSourceRange());
fixItWhere.first,
getAssociatedTypeReqs(assocTypeDecl, fixItWhere.second))
.fixItRemove(assocTypeDecl->getSourceRange());

Diags.diagnose(inheritedAssocTypeDecl, diag::decl_declared_here,
inheritedAssocTypeDecl->getFullName());
Expand All @@ -2592,11 +2660,13 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement(
bool shouldWarnAboutRedeclaration =
Source->kind == RequirementSource::RequirementSignatureSelf;

for (auto inheritedType : knownInherited->getSecond()) {
for (auto inheritedType : knownInherited->second) {
// If we have inherited associated type...
if (auto inheritedAssocTypeDecl =
dyn_cast<AssociatedTypeDecl>(inheritedType)) {
// FIXME: Wire up same-type constraint.
// Infer a same-type requirement between the typealias' underlying
// type and the inherited associated type.
addInferredSameTypeReq(inheritedAssocTypeDecl, typealias);

// Warn that one should use where clauses for this.
if (shouldWarnAboutRedeclaration) {
Expand Down Expand Up @@ -2626,6 +2696,16 @@ ConstraintResult GenericSignatureBuilder::addConformanceRequirement(
}
}

// Infer same-type requirements among inherited type declarations.
for (auto &entry : inheritedTypeDecls) {
if (entry.second.size() < 2) continue;

auto firstDecl = entry.second.front();
for (auto otherDecl : ArrayRef<TypeDecl *>(entry.second).slice(1)) {
addInferredSameTypeReq(firstDecl, otherDecl);
}
}

return ConstraintResult::Resolved;
}

Expand Down
34 changes: 34 additions & 0 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7427,6 +7427,40 @@ void TypeChecker::validateDeclForNameLookup(ValueDecl *D) {
validateAccessibility(assocType);
break;
}
case DeclKind::TypeAlias: {
auto typealias = cast<TypeAliasDecl>(D);
if (typealias->getUnderlyingTypeLoc().getType())
return;

// Perform earlier validation of typealiases in protocols.
if (auto proto = dyn_cast<ProtocolDecl>(dc)) {
if (!typealias->getGenericParams()) {
ProtocolRequirementTypeResolver resolver(proto);
TypeResolutionOptions options;

if (typealias->isBeingValidated()) return;

typealias->setIsBeingValidated();
SWIFT_DEFER { typealias->setIsBeingValidated(false); };

validateAccessibility(typealias);
if (typealias->getFormalAccess() <= Accessibility::FilePrivate)
options |= TR_KnownNonCascadingDependency;

if (validateType(typealias->getUnderlyingTypeLoc(),
typealias, options, &resolver)) {
typealias->setInvalid();
typealias->getUnderlyingTypeLoc().setInvalidType(Context);
}

typealias->setUnderlyingType(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if it does have a generic parameter list? How is this different from the typealias case in validateDecl()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We punt on generic typealiases in protocols, which is (sadly) not new. It's different from the typealias case in validateDecl() because it's using the ProtocolRequirementTypeResolver. The fallback is just validateDecl().

typealias->getUnderlyingTypeLoc().getType());

return;
}
}
LLVM_FALLTHROUGH;
}

default:
validateDecl(D);
Expand Down
2 changes: 1 addition & 1 deletion test/Generics/protocol_type_aliases.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ protocol Q3: P3 {
protocol P3_1 {
typealias T = Float // expected-error{{typealias 'T' requires types 'P3.T' (aka 'Int') and 'Float' to be the same}}
}
protocol Q3_1: P3, P3_1 {}
protocol Q3_1: P3, P3_1 {} // expected-error{{generic signature requires types 'Float'}}

// FIXME: these shouldn't be necessary to trigger the errors above, but are, due to
// the 'recursive decl validation' FIXME in GenericSignatureBuilder.cpp.
Expand Down
17 changes: 16 additions & 1 deletion test/Generics/requirement_inference.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func inferSuperclassRequirement2<T : Canidae>(_ v: U<T>) {}
// ----------------------------------------------------------------------------

protocol P3 {
associatedtype P3Assoc : P2
associatedtype P3Assoc : P2 // expected-note{{declared here}}
}

protocol P4 {
Expand Down Expand Up @@ -389,3 +389,18 @@ protocol P27b {
associatedtype A
associatedtype B where A == X26<B>
}

// ----------------------------------------------------------------------------
// Inference of associated type relationships within a protocol hierarchy
// ----------------------------------------------------------------------------

struct X28 : P2 {
func p1() { }
}

// CHECK-LABEL: .P28@
// CHECK-NEXT: Requirement signature: <Self where Self : P3, Self.P3Assoc == X28>
// CHECK-NEXT: Canonical requirement signature: <τ_0_0 where τ_0_0 : P3, τ_0_0.P3Assoc == X28>
protocol P28: P3 {
typealias P3Assoc = X28 // expected-warning{{typealias overriding associated type}}
}