Skip to content

Commit a0a26b8

Browse files
committed
Require explicit statement of all Copyable/Escapable requirements for conditional Copyable/Escapable conformances.
As specified by the SE-0446 acceptance, extensions that declare a type's conditional `Copyable` or `Escapable` ability must reiterate explicitly all of the `Copyable` and/or `Escapable` requirements, whether required or not required (by e.g. `~Copyable`) that were suppressed in the original type declaration.
1 parent b906219 commit a0a26b8

18 files changed

+224
-55
lines changed

include/swift/AST/Decl.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2094,7 +2094,12 @@ class ExtensionDecl final : public GenericContext, public Decl,
20942094

20952095
/// Does this extension add conformance to an invertible protocol for the
20962096
/// extended type?
2097-
bool isAddingConformanceToInvertible() const;
2097+
///
2098+
/// Returns \c nullopt if the extension does not add conformance to any
2099+
/// invertible protocol. Returns one of the invertible protocols being
2100+
/// conformed to otherwise.
2101+
std::optional<InvertibleProtocolKind>
2102+
isAddingConformanceToInvertible() const;
20982103

20992104
/// If this extension represents an imported Objective-C category, returns the
21002105
/// category's name. Otherwise returns the empty identifier.

include/swift/AST/DiagnosticsSema.def

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7850,6 +7850,16 @@ ERROR(extension_conforms_to_invertible_and_others, none,
78507850
ERROR(invertible_conformance_other_source_file,none,
78517851
"conformance to '%0' must occur in the same source file as %kind1",
78527852
(StringRef, const ValueDecl *))
7853+
ERROR(inverse_conditional_must_be_fully_explicit,none,
7854+
"conditional conformance to %0 must explicitly "
7855+
"state whether %1 is required to conform to %2 or not",
7856+
(const ProtocolDecl *, Type, const ProtocolDecl *))
7857+
NOTE(make_inverse_conditional_non_requirement_explicit,none,
7858+
"specify that the conformance does not require %0 to conform to %1",
7859+
(Type, const ProtocolDecl *))
7860+
NOTE(make_inverse_conditional_requirement_explicit,none,
7861+
"specify that the conformance requires %0 to conform to %1",
7862+
(Type, const ProtocolDecl *))
78537863

78547864
// -- older ones below --
78557865
ERROR(noncopyable_parameter_requires_ownership, none,

include/swift/AST/TypeCheckRequests.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2118,7 +2118,8 @@ class InferredGenericSignatureRequest :
21182118
WhereClauseOwner,
21192119
SmallVector<Requirement, 2>,
21202120
SmallVector<TypeBase *, 2>,
2121-
SourceLoc, bool, bool),
2121+
SourceLoc, ExtensionDecl *,
2122+
bool),
21222123
RequestFlags::Uncached> {
21232124
public:
21242125
using SimpleRequest::SimpleRequest;
@@ -2134,7 +2135,8 @@ class InferredGenericSignatureRequest :
21342135
WhereClauseOwner whereClause,
21352136
SmallVector<Requirement, 2> addedRequirements,
21362137
SmallVector<TypeBase *, 2> inferenceSources,
2137-
SourceLoc loc, bool isExtension, bool allowInverses) const;
2138+
SourceLoc loc, ExtensionDecl *forExtension,
2139+
bool allowInverses) const;
21382140

21392141
public:
21402142
/// Inferred generic signature requests don't have source-location info.

include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ SWIFT_REQUEST(TypeChecker, InferredGenericSignatureRequest,
206206
WhereClauseOwner,
207207
SmallVector<Requirement, 2>,
208208
SmallVector<TypeBase *, 2>,
209-
SourceLoc, bool, bool),
209+
SourceLoc, ExtensionDecl *, bool),
210210
Uncached, NoLocationInfo)
211211
SWIFT_REQUEST(TypeChecker, DistributedModuleIsAvailableRequest,
212212
bool(const ValueDecl *), Cached, NoLocationInfo)

lib/AST/ASTPrinter.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -995,6 +995,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
995995
SwapSelfAndDependentMemberType = 8,
996996
PrintInherited = 16,
997997
PrintInverseRequirements = 32,
998+
PrintExplicitInvertibleRequirements = 64,
998999
};
9991000

10001001
/// The default generic signature flags for printing requirements.
@@ -1763,7 +1764,18 @@ void PrintAST::printGenericSignature(
17631764
SmallVector<Requirement, 2> requirements;
17641765
SmallVector<InverseRequirement, 2> inverses;
17651766

1766-
if (flags & PrintInverseRequirements) {
1767+
if (flags & PrintExplicitInvertibleRequirements) {
1768+
// Collect the inverted requirements…
1769+
SmallVector<Requirement, 2> filteredRequirements;
1770+
genericSig->getRequirementsWithInverses(filteredRequirements, inverses);
1771+
llvm::erase_if(inverses, [&](InverseRequirement inverse) -> bool {
1772+
return !inverseFilter(inverse);
1773+
});
1774+
// …and also all of the unfiltered requirements, including the ones for
1775+
// invertible protocols.
1776+
requirements.append(genericSig.getRequirements().begin(),
1777+
genericSig.getRequirements().end());
1778+
} else if (flags & PrintInverseRequirements) {
17671779
genericSig->getRequirementsWithInverses(requirements, inverses);
17681780
llvm::erase_if(inverses, [&](InverseRequirement inverse) -> bool {
17691781
return !inverseFilter(inverse);
@@ -3115,7 +3127,7 @@ void PrintAST::printExtension(ExtensionDecl *decl) {
31153127
// in such an extension. We need to print the whole signature:
31163128
// extension S: Copyable where T: Copyable
31173129
if (decl->isAddingConformanceToInvertible())
3118-
genSigFlags &= ~PrintInverseRequirements;
3130+
genSigFlags |= PrintExplicitInvertibleRequirements;
31193131

31203132
printGenericSignature(genericSig,
31213133
genSigFlags,

lib/AST/Decl.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1992,7 +1992,8 @@ Type ExtensionDecl::getExtendedType() const {
19921992
return ErrorType::get(ctx);
19931993
}
19941994

1995-
bool ExtensionDecl::isAddingConformanceToInvertible() const {
1995+
std::optional<InvertibleProtocolKind>
1996+
ExtensionDecl::isAddingConformanceToInvertible() const {
19961997
const unsigned numEntries = getInherited().size();
19971998
for (unsigned i = 0; i < numEntries; ++i) {
19981999
InheritedTypeRequest request{this, i, TypeResolutionStage::Structural};
@@ -2010,10 +2011,10 @@ bool ExtensionDecl::isAddingConformanceToInvertible() const {
20102011

20112012
if (inheritedTy)
20122013
if (auto kp = inheritedTy->getKnownProtocol())
2013-
if (getInvertibleProtocolKind(*kp))
2014-
return true;
2014+
if (auto kind = getInvertibleProtocolKind(*kp))
2015+
return kind;
20152016
}
2016-
return false;
2017+
return std::nullopt;
20172018
}
20182019

20192020
bool Decl::isObjCImplementation() const {

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -776,15 +776,15 @@ InferredGenericSignatureRequest::evaluate(
776776
WhereClauseOwner whereClause,
777777
SmallVector<Requirement, 2> addedRequirements,
778778
SmallVector<TypeBase *, 2> inferenceSources,
779-
SourceLoc loc, bool isExtension, bool allowInverses) const {
779+
SourceLoc loc, ExtensionDecl *forExtension, bool allowInverses) const {
780780
GenericSignature parentSig(parentSigImpl);
781781

782782
SmallVector<GenericTypeParamType *, 4> genericParams(
783783
parentSig.getGenericParams().begin(),
784784
parentSig.getGenericParams().end());
785785

786786
unsigned numOuterParams = genericParams.size();
787-
if (isExtension) {
787+
if (forExtension) {
788788
numOuterParams = 0;
789789
}
790790

@@ -889,6 +889,68 @@ InferredGenericSignatureRequest::evaluate(
889889
SmallVector<StructuralRequirement, 2> defaults;
890890
InverseRequirement::expandDefaults(ctx, paramTypes, defaults);
891891
applyInverses(ctx, paramTypes, inverses, defaults, errors);
892+
893+
// Any remaining implicit defaults in a conditional inverse requirement
894+
// extension must be made explicit.
895+
if (forExtension) {
896+
auto invertibleProtocol = forExtension->isAddingConformanceToInvertible();
897+
// FIXME: to workaround a reverse condfail, always infer the requirements if
898+
// the extension is in a swiftinterface file. This is temporary and should
899+
// be removed soon. (rdar://130424971)
900+
if (auto *sf = forExtension->getOutermostParentSourceFile()) {
901+
if (sf->Kind == SourceFileKind::Interface
902+
&& !ctx.LangOpts.hasFeature(Feature::SE427NoInferenceOnExtension)) {
903+
invertibleProtocol = std::nullopt;
904+
}
905+
}
906+
if (invertibleProtocol) {
907+
for (auto &def : defaults) {
908+
// Check whether a corresponding explicit requirement was provided.
909+
for (auto &req : requirements) {
910+
// An explicit requirement can match the default exactly.
911+
if (req.req.getCanonical() == def.req.getCanonical()) {
912+
goto next;
913+
}
914+
915+
// Disregard requirements on other parameters.
916+
if (!req.req.getFirstType()->isEqual(def.req.getFirstType())) {
917+
continue;
918+
}
919+
920+
// Or it can be implied by a requirement on something that's inherently
921+
// copyable.
922+
if (req.req.getKind() == RequirementKind::Superclass) {
923+
// classes are currently always escapable and copyable
924+
goto next;
925+
}
926+
if (req.req.getKind() == RequirementKind::Layout) {
927+
// layout constraints currently always imply escapable and copyable
928+
goto next;
929+
}
930+
if (req.req.getKind() == RequirementKind::Conformance
931+
&& req.req.getProtocolDecl()
932+
->inheritsFrom(def.req.getProtocolDecl())) {
933+
goto next;
934+
}
935+
936+
// A same-type constraint removes the ability for the copyability
937+
// to vary independently at all.
938+
if (req.req.getKind() == RequirementKind::SameType) {
939+
goto next;
940+
}
941+
}
942+
ctx.Diags.diagnose(loc,diag::inverse_conditional_must_be_fully_explicit,
943+
ctx.getProtocol(getKnownProtocolKind(*invertibleProtocol)),
944+
def.req.getFirstType(),
945+
def.req.getProtocolDecl());
946+
next:;
947+
}
948+
// Don't actually apply the inferred requirements since they should be
949+
// stated explicitly.
950+
defaults.clear();
951+
}
952+
}
953+
892954
requirements.append(defaults);
893955

894956
auto &rewriteCtx = ctx.getRewriteContext();
@@ -954,7 +1016,7 @@ InferredGenericSignatureRequest::evaluate(
9541016
if (attempt == 0) {
9551017
machine->computeRequirementDiagnostics(errors, inverses, loc);
9561018
diagnoseRequirementErrors(ctx, errors,
957-
(isExtension || !genericParamList)
1019+
(forExtension || !genericParamList)
9581020
? AllowConcreteTypePolicy::All
9591021
: AllowConcreteTypePolicy::AssocTypes);
9601022
}
@@ -985,7 +1047,7 @@ InferredGenericSignatureRequest::evaluate(
9851047
std::move(machine));
9861048
}
9871049

988-
if (genericParamList && !isExtension) {
1050+
if (genericParamList && !forExtension) {
9891051
for (auto genericParam : result.getInnermostGenericParams()) {
9901052
auto reduced = result.getReducedType(genericParam);
9911053

lib/Sema/TypeCheckAttr.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3492,7 +3492,7 @@ SerializeAttrGenericSignatureRequest::evaluate(Evaluator &evaluator,
34923492
/*addedRequirements=*/{},
34933493
/*inferenceSources=*/{},
34943494
attr->getLocation(),
3495-
/*isExtension=*/false,
3495+
/*forExtension=*/nullptr,
34963496
/*allowInverses=*/true};
34973497

34983498
auto specializedSig = evaluateOrDefault(Ctx.evaluator, request,
@@ -6078,7 +6078,7 @@ bool resolveDifferentiableAttrDerivativeGenericSignature(
60786078
/*addedRequirements=*/{},
60796079
/*inferenceSources=*/{},
60806080
attr->getLocation(),
6081-
/*isExtension=*/false,
6081+
/*forExtension=*/nullptr,
60826082
/*allowInverses=*/true};
60836083

60846084
// Compute generic signature for derivative functions.

lib/Sema/TypeCheckGeneric.cpp

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ OpaqueResultTypeRequest::evaluate(Evaluator &evaluator,
119119
/*addedRequirements=*/{},
120120
/*inferenceSources=*/{},
121121
repr->getLoc(),
122-
/*isExtension=*/false,
122+
/*forExtension=*/nullptr,
123123
/*allowInverses=*/true};
124124

125125
interfaceSignature = evaluateOrDefault(
@@ -689,7 +689,6 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
689689
SmallVector<TypeBase *, 2> inferenceSources;
690690
SmallVector<Requirement, 2> extraReqs;
691691
SourceLoc loc;
692-
bool inferInvertibleReqs = true;
693692

694693
if (auto VD = dyn_cast<ValueDecl>(GC->getAsDecl())) {
695694
loc = VD->getLoc();
@@ -787,20 +786,6 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
787786
} else if (auto *ext = dyn_cast<ExtensionDecl>(GC)) {
788787
loc = ext->getLoc();
789788

790-
// If the extension introduces conformance to invertible protocol IP, do not
791-
// infer any conditional requirements that the generic parameters to conform
792-
// to invertible protocols. This forces people to write out the conditions.
793-
inferInvertibleReqs = !ext->isAddingConformanceToInvertible();
794-
795-
// FIXME: to workaround a reverse condfail, always infer the requirements if
796-
// the extension is in a swiftinterface file. This is temporary and should
797-
// be removed soon. (rdar://130424971)
798-
if (auto *sf = ext->getOutermostParentSourceFile()) {
799-
if (sf->Kind == SourceFileKind::Interface
800-
&& !ctx.LangOpts.hasFeature(Feature::SE427NoInferenceOnExtension))
801-
inferInvertibleReqs = true;
802-
}
803-
804789
collectAdditionalExtensionRequirements(ext->getExtendedType(), extraReqs);
805790

806791
auto *extendedNominal = ext->getExtendedNominal();
@@ -838,8 +823,8 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
838823
parentSig.getPointer(),
839824
genericParams, WhereClauseOwner(GC),
840825
extraReqs, inferenceSources, loc,
841-
/*isExtension=*/isa<ExtensionDecl>(GC),
842-
/*allowInverses=*/inferInvertibleReqs};
826+
/*forExtension=*/dyn_cast<ExtensionDecl>(GC),
827+
/*allowInverses=*/true};
843828
return evaluateOrDefault(ctx.evaluator, request,
844829
GenericSignatureWithError()).getPointer();
845830
}

lib/Sema/TypeChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ swift::handleSILGenericParams(GenericParamList *genericParams,
516516
/*parentSig=*/nullptr,
517517
nestedList.back(), WhereClauseOwner(),
518518
{}, {}, genericParams->getLAngleLoc(),
519-
/*isExtension=*/false,
519+
/*forExtension=*/nullptr,
520520
allowInverses};
521521
return evaluateOrDefault(DC->getASTContext().evaluator, request,
522522
GenericSignatureWithError()).getPointer();

0 commit comments

Comments
 (0)