-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[GSB] Canonicalize conformance access paths on-the-fly. #13424
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
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 |
---|---|---|
|
@@ -900,12 +900,71 @@ namespace { | |
using GSBConstraint = GenericSignatureBuilder::Constraint<T>; | ||
} // end anonymous namespace | ||
|
||
/// Determine whether there is a conformance of the given | ||
/// subject type to the given protocol within the given set of explicit | ||
/// requirements. | ||
static bool hasConformanceInSignature(ArrayRef<Requirement> requirements, | ||
Type subjectType, | ||
ProtocolDecl *proto) { | ||
// Make sure this requirement exists in the requirement signature. | ||
for (const auto& req: requirements) { | ||
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. "auto &" maybe? 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. You're referring to the spacing? 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. (I'll fix the spacing in a separate PR) |
||
if (req.getKind() == RequirementKind::Conformance && | ||
req.getFirstType()->isEqual(subjectType) && | ||
req.getSecondType()->castTo<ProtocolType>()->getDecl() | ||
== proto) { | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/// Check whether the given requirement source has any non-canonical protocol | ||
/// requirements in it. | ||
static bool hasNonCanonicalSelfProtocolRequirement( | ||
const RequirementSource *source, | ||
ProtocolDecl *conformingProto) { | ||
for (; source; source = source->parent) { | ||
// Only look at protocol requirements. | ||
if (!source->isProtocolRequirement()) | ||
continue; | ||
|
||
// If we don't already have a requirement signature for this protocol, | ||
// build one now. | ||
auto inProto = source->getProtocolDecl(); | ||
if (!inProto->isRequirementSignatureComputed()) { | ||
inProto->computeRequirementSignature(); | ||
assert(inProto->isRequirementSignatureComputed() && | ||
"couldn't compute requirement signature?"); | ||
} | ||
|
||
// Check whether the given requirement is in the requirement signature. | ||
if (!source->usesRequirementSignature && | ||
!hasConformanceInSignature(inProto->getRequirementSignature(), | ||
source->getStoredType(), conformingProto)) | ||
return true; | ||
|
||
// Update the conforming protocol for the rest of the search. | ||
conformingProto = inProto; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/// Retrieve the best requirement source from the list | ||
static const RequirementSource * | ||
getBestRequirementSource(ArrayRef<GSBConstraint<ProtocolDecl *>> constraints) { | ||
getBestCanonicalRequirementSource( | ||
ArrayRef<GSBConstraint<ProtocolDecl *>> constraints) { | ||
const RequirementSource *bestSource = nullptr; | ||
for (const auto &constraint : constraints) { | ||
auto source = constraint.source; | ||
|
||
// If there is a non-canonical protocol requirement next to the root, | ||
// skip this requirement source. | ||
if (hasNonCanonicalSelfProtocolRequirement(source, constraint.value)) | ||
continue; | ||
|
||
// Check whether this is better than our best source. | ||
if (!bestSource || source->compare(bestSource) < 0) | ||
bestSource = source; | ||
} | ||
|
@@ -934,27 +993,6 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath( | |
typedef GenericSignatureBuilder::RequirementSource RequirementSource; | ||
ConformanceAccessPath path; | ||
|
||
#ifndef NDEBUG | ||
// Local function to determine whether there is a conformance of the given | ||
// subject type to the given protocol within the given set of explicit | ||
// requirements. | ||
auto hasConformanceInSignature = [&](ArrayRef<Requirement> requirements, | ||
Type subjectType, | ||
ProtocolDecl *proto) -> bool { | ||
// Make sure this requirement exists in the requirement signature. | ||
for (const auto& req: requirements) { | ||
if (req.getKind() == RequirementKind::Conformance && | ||
req.getFirstType()->isEqual(subjectType) && | ||
req.getSecondType()->castTo<ProtocolType>()->getDecl() | ||
== proto) { | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
}; | ||
#endif | ||
|
||
// Local function to construct the conformance access path from the | ||
// requirement. | ||
std::function<void(ArrayRef<Requirement>, const RequirementSource *, | ||
|
@@ -1003,13 +1041,6 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath( | |
return; | ||
} | ||
|
||
// Canonicalize this step with respect to the requirement signature. | ||
if (!inProtocol->isRequirementSignatureComputed()) { | ||
inProtocol->computeRequirementSignature(); | ||
assert(inProtocol->isRequirementSignatureComputed() && | ||
"missing signature"); | ||
} | ||
|
||
// Get a generic signature for the protocol's signature. | ||
auto inProtoSig = inProtocol->getGenericSignature(); | ||
auto &inProtoSigBuilder = *inProtoSig->getGenericSignatureBuilder(); | ||
|
@@ -1031,7 +1062,7 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath( | |
assert(conforms != equivClass->conformsTo.end()); | ||
|
||
// Compute the root type, canonicalizing it w.r.t. the protocol context. | ||
auto conformsSource = getBestRequirementSource(conforms->second); | ||
auto conformsSource = getBestCanonicalRequirementSource(conforms->second); | ||
assert(conformsSource != source || !requirementSignatureProto); | ||
Type localRootType = conformsSource->getRootType(); | ||
localRootType = inProtoSig->getCanonicalTypeInContext(localRootType); | ||
|
@@ -1079,7 +1110,7 @@ ConformanceAccessPath GenericSignature::getConformanceAccessPath( | |
}; | ||
|
||
// Canonicalize the root type. | ||
auto source = getBestRequirementSource(conforms->second); | ||
auto source = getBestCanonicalRequirementSource(conforms->second); | ||
Type rootType = source->getRootType()->getCanonicalType(this); | ||
|
||
// Build the path. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
// RUN: %target-swift-frontend -assume-parsing-unqualified-ownership-sil -primary-file %s -emit-ir > %t.ll | ||
// RUN: %FileCheck %s < %t.ll | ||
|
||
|
||
// SR-6200: canonicalizing a conformance access path that was built | ||
// without the requirement signature. | ||
public struct Valid<V> {} | ||
|
||
extension Valid where V: ValidationSuite {} | ||
|
||
public protocol Validatable {} | ||
|
||
extension Validatable { | ||
public func tested() {} | ||
|
||
// CHECK-LABEL: define{{.*}}_T023conformance_access_path11ValidatablePAAE6testedyqd__m2by_t9InputTypeQyd__RszAA15ValidationSuiteRd__lF | ||
public func tested<S: ValidationSuite>(by suite: S.Type) where S.InputType == Self { | ||
// CHECK: [[S_AS_VALIDATION_SUITE:%[0-9]+]] = load i8*, i8** %S.ValidationSuite | ||
// CHECK-NEXT: [[S_VALIDATOR_BASE:%.*]] = bitcast i8* [[S_AS_VALIDATION_SUITE]] to i8** | ||
// CHECK-NEXT: [[S_VALIDATABLE_ADDR:%[0-9]+]] = getelementptr inbounds i8*, i8** [[S_VALIDATOR_BASE]], i32 1 | ||
// CHECK-NEXT: [[S_VALIDATABLE_FN_RAW:%[0-9]+]] = load i8*, i8** [[S_VALIDATABLE_ADDR]] | ||
// CHECK-NEXT: [[S_VALIDATABLE_FN:%[0-9]+]] = bitcast i8* [[S_VALIDATABLE_FN_RAW]] to i8** (%swift.type*, %swift.type*, i8**)* | ||
// CHECK-NEXT: call i8** [[S_VALIDATABLE_FN]](%swift.type* %Self, %swift.type* %S, i8** %S.Validator) | ||
tested() | ||
} | ||
} | ||
|
||
public protocol Validator { | ||
associatedtype InputType: Validatable | ||
} | ||
|
||
public protocol ValidationSuite: Validator { | ||
associatedtype InputType: Validatable | ||
} |
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.
Does this need to be public?
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.
Yeah, it's used from a static function. It's
const
, so there's no concern about anyone messing with the state.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.
... although this makes me wonder if I could back-patch the value when it is found to map into the requirement signature, to prevent future work. Feels icky.