Skip to content

[5.8 🍒] TypeCheckType: Fix existential any migration diagnostics for extra-modular protocols #65160

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

Closed
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
46 changes: 29 additions & 17 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -549,11 +549,12 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
/// Whether the existential of this protocol conforms to itself.
ExistentialConformsToSelf : 1,

/// Whether the \c ExistentialRequiresAny bit is valid.
ExistentialRequiresAnyValid : 1,
/// Whether the \c HasSelfOrAssociatedTypeRequirements bit is valid.
HasSelfOrAssociatedTypeRequirementsValid : 1,

/// Whether the existential of this protocol must be spelled with \c any.
ExistentialRequiresAny : 1,
/// Whether this protocol has \c Self or associated type requirements.
/// See \c hasSelfOrAssociatedTypeRequirements() for clarification.
HasSelfOrAssociatedTypeRequirements : 1,

/// True if the protocol has requirements that cannot be satisfied (e.g.
/// because they could not be imported from Objective-C).
Expand Down Expand Up @@ -4651,19 +4652,19 @@ class ProtocolDecl final : public NominalTypeDecl {
Bits.ProtocolDecl.ExistentialConformsToSelf = result;
}

/// Returns the cached result of \c existentialRequiresAny or \c None if it
/// hasn't yet been computed.
Optional<bool> getCachedExistentialRequiresAny() {
if (Bits.ProtocolDecl.ExistentialRequiresAnyValid)
return Bits.ProtocolDecl.ExistentialRequiresAny;
/// Returns the cached result of \c hasSelfOrAssociatedTypeRequirements or
/// \c None if it hasn't yet been computed.
Optional<bool> getCachedHasSelfOrAssociatedTypeRequirements() {
if (Bits.ProtocolDecl.HasSelfOrAssociatedTypeRequirementsValid)
return Bits.ProtocolDecl.HasSelfOrAssociatedTypeRequirements;

return None;
}

/// Caches the result of \c existentialRequiresAny
void setCachedExistentialRequiresAny(bool requiresAny) {
Bits.ProtocolDecl.ExistentialRequiresAnyValid = true;
Bits.ProtocolDecl.ExistentialRequiresAny = requiresAny;
/// Caches the result of \c hasSelfOrAssociatedTypeRequirements
void setCachedHasSelfOrAssociatedTypeRequirements(bool value) {
Bits.ProtocolDecl.HasSelfOrAssociatedTypeRequirementsValid = true;
Bits.ProtocolDecl.HasSelfOrAssociatedTypeRequirements = value;
}

bool hasLazyRequirementSignature() const {
Expand All @@ -4684,7 +4685,7 @@ class ProtocolDecl final : public NominalTypeDecl {
friend class RequirementSignatureRequestGSB;
friend class ProtocolRequiresClassRequest;
friend class ExistentialConformsToSelfRequest;
friend class ExistentialRequiresAnyRequest;
friend class HasSelfOrAssociatedTypeRequirementsRequest;
friend class InheritedProtocolsRequest;
friend class PrimaryAssociatedTypesRequest;
friend class ProtocolRequirementsRequest;
Expand Down Expand Up @@ -4783,9 +4784,20 @@ class ProtocolDecl final : public NominalTypeDecl {
/// Does this protocol require a self-conformance witness table?
bool requiresSelfConformanceWitnessTable() const;

/// Determine whether an existential type must be explicitly prefixed
/// with \c any. \c any is required if any of the members contain
/// an associated type, or if \c Self appears in non-covariant position.
/// Determine whether this protocol has `Self` or associated type
/// requirements.
///
/// This is true if one of the following conditions is met for this protocol
/// or an inherited protocol:
/// - The protocol has an associated type requirement.
/// - `Self` appears in non-covariant position in the type signature of a
/// value requirement.
bool hasSelfOrAssociatedTypeRequirements() const;

/// Determine whether an existential type constrained by this protocol must
/// be written using `any` syntax.
///
/// \Note This method takes language feature state into account.
bool existentialRequiresAny() const;

/// Returns a list of protocol requirements that must be assessed to
Expand Down
4 changes: 2 additions & 2 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,8 @@ class ExistentialConformsToSelfRequest :

/// Determine whether an existential type conforming to this protocol
/// requires the \c any syntax.
class ExistentialRequiresAnyRequest :
public SimpleRequest<ExistentialRequiresAnyRequest,
class HasSelfOrAssociatedTypeRequirementsRequest :
public SimpleRequest<HasSelfOrAssociatedTypeRequirementsRequest,
bool(ProtocolDecl *),
RequestFlags::SeparatelyCached> {
public:
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ SWIFT_REQUEST(TypeChecker, EnumRawTypeRequest,
Type(EnumDecl *), Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, ExistentialConformsToSelfRequest,
bool(ProtocolDecl *), SeparatelyCached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, ExistentialRequiresAnyRequest,
SWIFT_REQUEST(TypeChecker, HasSelfOrAssociatedTypeRequirementsRequest,
bool(ProtocolDecl *), SeparatelyCached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, ExtendedTypeRequest, Type(ExtensionDecl *), Cached,
NoLocationInfo)
Expand Down
13 changes: 11 additions & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5684,9 +5684,18 @@ bool ProtocolDecl::existentialConformsToSelf() const {
ExistentialConformsToSelfRequest{const_cast<ProtocolDecl *>(this)}, true);
}

bool ProtocolDecl::existentialRequiresAny() const {
bool ProtocolDecl::hasSelfOrAssociatedTypeRequirements() const {
return evaluateOrDefault(getASTContext().evaluator,
ExistentialRequiresAnyRequest{const_cast<ProtocolDecl *>(this)}, true);
HasSelfOrAssociatedTypeRequirementsRequest{
const_cast<ProtocolDecl *>(this)},
true);
}

bool ProtocolDecl::existentialRequiresAny() const {
if (getASTContext().LangOpts.hasFeature(Feature::ExistentialAny))
return true;

return hasSelfOrAssociatedTypeRequirements();
}

ArrayRef<AssociatedTypeDecl *>
Expand Down
17 changes: 10 additions & 7 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,28 +259,31 @@ void ExistentialConformsToSelfRequest::cacheResult(bool value) const {
}

//----------------------------------------------------------------------------//
// existentialRequiresAny computation.
// hasSelfOrAssociatedTypeRequirementsRequest computation.
//----------------------------------------------------------------------------//

void ExistentialRequiresAnyRequest::diagnoseCycle(DiagnosticEngine &diags) const {
void HasSelfOrAssociatedTypeRequirementsRequest::diagnoseCycle(
DiagnosticEngine &diags) const {
auto decl = std::get<0>(getStorage());
diags.diagnose(decl, diag::circular_protocol_def, decl->getName());
}

void ExistentialRequiresAnyRequest::noteCycleStep(DiagnosticEngine &diags) const {
void HasSelfOrAssociatedTypeRequirementsRequest::noteCycleStep(
DiagnosticEngine &diags) const {
auto requirement = std::get<0>(getStorage());
diags.diagnose(requirement, diag::kind_declname_declared_here,
DescriptiveDeclKind::Protocol, requirement->getName());
}

Optional<bool> ExistentialRequiresAnyRequest::getCachedResult() const {
Optional<bool>
HasSelfOrAssociatedTypeRequirementsRequest::getCachedResult() const {
auto decl = std::get<0>(getStorage());
return decl->getCachedExistentialRequiresAny();
return decl->getCachedHasSelfOrAssociatedTypeRequirements();
}

void ExistentialRequiresAnyRequest::cacheResult(bool value) const {
void HasSelfOrAssociatedTypeRequirementsRequest::cacheResult(bool value) const {
auto decl = std::get<0>(getStorage());
decl->setCachedExistentialRequiresAny(value);
decl->setCachedHasSelfOrAssociatedTypeRequirements(value);
}

//----------------------------------------------------------------------------//
Expand Down
11 changes: 3 additions & 8 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -707,13 +707,8 @@ ExistentialConformsToSelfRequest::evaluate(Evaluator &evaluator,
return true;
}

bool
ExistentialRequiresAnyRequest::evaluate(Evaluator &evaluator,
ProtocolDecl *decl) const {
auto &ctx = decl->getASTContext();
if (ctx.LangOpts.hasFeature(Feature::ExistentialAny))
return true;

bool HasSelfOrAssociatedTypeRequirementsRequest::evaluate(
Evaluator &evaluator, ProtocolDecl *decl) const {
// ObjC protocols do not require `any`.
if (decl->isObjC())
return false;
Expand All @@ -736,7 +731,7 @@ ExistentialRequiresAnyRequest::evaluate(Evaluator &evaluator,

// Check whether any of the inherited protocols require `any`.
for (auto proto : decl->getInheritedProtocols()) {
if (proto->existentialRequiresAny())
if (proto->hasSelfOrAssociatedTypeRequirements())
return true;
}

Expand Down
8 changes: 6 additions & 2 deletions lib/Sema/TypeCheckType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4779,6 +4779,10 @@ class ExistentialTypeVisitor
if (T->isInvalid())
return;

if (Ctx.LangOpts.hasFeature(Feature::ImplicitSome)) {
return;
}

// Compute the type repr to attach 'any' to.
TypeRepr *replaceRepr = T;
// Insert parens in expression context for '(any P).self'
Expand Down Expand Up @@ -4812,7 +4816,7 @@ class ExistentialTypeVisitor

auto comp = T->getComponentRange().back();
if (auto *proto = dyn_cast_or_null<ProtocolDecl>(comp->getBoundDecl())) {
if (proto->existentialRequiresAny() && !Ctx.LangOpts.hasFeature(Feature::ImplicitSome)) {
if (proto->existentialRequiresAny()) {
Ctx.Diags.diagnose(comp->getNameLoc(),
diag::existential_requires_any,
proto->getDeclaredInterfaceType(),
Expand All @@ -4829,7 +4833,7 @@ class ExistentialTypeVisitor
if (type->isConstraintType()) {
auto layout = type->getExistentialLayout();
for (auto *protoDecl : layout.getProtocols()) {
if (!protoDecl->existentialRequiresAny() || Ctx.LangOpts.hasFeature(Feature::ImplicitSome))
if (!protoDecl->existentialRequiresAny())
continue;

Ctx.Diags.diagnose(comp->getNameLoc(),
Expand Down
8 changes: 4 additions & 4 deletions lib/Serialization/Deserialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3894,14 +3894,14 @@ class DeclDeserializer {
StringRef blobData) {
IdentifierID nameID;
DeclContextID contextID;
bool isImplicit, isClassBounded, isObjC, existentialRequiresAny;
bool isImplicit, isClassBounded, isObjC, hasSelfOrAssocTypeRequirements;
uint8_t rawAccessLevel;
unsigned numInheritedTypes;
ArrayRef<uint64_t> rawInheritedAndDependencyIDs;

decls_block::ProtocolLayout::readRecord(scratch, nameID, contextID,
isImplicit, isClassBounded, isObjC,
existentialRequiresAny,
hasSelfOrAssocTypeRequirements,
rawAccessLevel, numInheritedTypes,
rawInheritedAndDependencyIDs);

Expand Down Expand Up @@ -3929,8 +3929,8 @@ class DeclDeserializer {

ctx.evaluator.cacheOutput(ProtocolRequiresClassRequest{proto},
std::move(isClassBounded));
ctx.evaluator.cacheOutput(ExistentialRequiresAnyRequest{proto},
std::move(existentialRequiresAny));
ctx.evaluator.cacheOutput(HasSelfOrAssociatedTypeRequirementsRequest{proto},
std::move(hasSelfOrAssocTypeRequirements));

if (auto accessLevel = getActualAccessLevel(rawAccessLevel))
proto->setAccess(*accessLevel);
Expand Down
2 changes: 1 addition & 1 deletion lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3827,7 +3827,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
const_cast<ProtocolDecl *>(proto)
->requiresClass(),
proto->isObjC(),
proto->existentialRequiresAny(),
proto->hasSelfOrAssociatedTypeRequirements(),
rawAccessLevel, numInherited,
inheritedAndDependencyTypes);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func testUnlabeledParamMatching(i: Int, fn: ((Int) -> Int) -> Void) {

// When "fuzzy matching is disabled, this will fail.
func forwardMatchFailure( // expected-note{{declared here}}
onError: ((Error) -> Void)? = nil,
onError: ((any Error) -> Void)? = nil,
onCompletion: (Int) -> Void
) { }

Expand Down
29 changes: 20 additions & 9 deletions test/type/explicit_existential.swift
Original file line number Diff line number Diff line change
Expand Up @@ -207,17 +207,9 @@ extension MyError {
}
}

struct Wrapper {
typealias E = Error
}

func typealiasMemberReferences(metatype: Wrapper.Type) {
let _: Wrapper.E.Protocol = metatype.E.self
let _: (any Wrapper.E).Type = metatype.E.self
}

func testAnyTypeExpr() {
let _: (any P).Type = (any P).self
let _: (any P1 & P2).Type = (any P1 & P2).self

func test(_: (any P).Type) {}
test((any P).self)
Expand Down Expand Up @@ -281,6 +273,12 @@ enum EE : Equatable, any Empty { // expected-error {{raw type 'any Empty' is not
case hack
}

// Protocols from a serialized module (the standard library).
do {
let _: Decodable
let _: Codable
}

func testAnyFixIt() {
struct ConformingType : HasAssoc {
typealias Assoc = Int
Expand All @@ -302,6 +300,19 @@ func testAnyFixIt() {
// expected-error@+2 {{use of protocol 'HasAssoc' as a type must be written 'any HasAssoc'}}{{10-18=(any HasAssoc)}}
// expected-error@+1 {{use of protocol 'HasAssoc' as a type must be written 'any HasAssoc'}}{{30-38=(any HasAssoc)}}
let _: HasAssoc.Protocol = HasAssoc.self
do {
struct Wrapper {
typealias HasAssocAlias = HasAssoc
}
let wrapperMeta: Wrapper.Type
// FIXME: Both of these fix-its are wrong.
// 1. 'any' is attached to 'HasAssocAlias' instead of 'Wrapper.HasAssocAlias'
// 2. What is the correct fix-it for the initializer?
//
// expected-error@+2:20 {{use of 'Wrapper.HasAssocAlias' (aka 'HasAssoc') as a type must be written 'any Wrapper.HasAssocAlias' (aka 'any HasAssoc')}}{{20-33=(any HasAssocAlias)}}
// expected-error@+1:57 {{use of 'Wrapper.HasAssocAlias' (aka 'HasAssoc') as a type must be written 'any Wrapper.HasAssocAlias' (aka 'any HasAssoc')}}{{57-70=(any HasAssocAlias)}}
let _: Wrapper.HasAssocAlias.Protocol = wrapperMeta.HasAssocAlias.self
}
// expected-error@+1 {{use of protocol 'HasAssoc' as a type must be written 'any HasAssoc'}}{{11-19=any HasAssoc}}
let _: (HasAssoc).Protocol = (any HasAssoc).self
// expected-error@+1 {{use of protocol 'HasAssoc' as a type must be written 'any HasAssoc'}}{{10-18=(any HasAssoc)}}
Expand Down
15 changes: 15 additions & 0 deletions test/type/explicit_existential_multimodule1.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend %s -swift-version 5 -emit-module -DM -module-name M -emit-module-path %t/M.swiftmodule -enable-upcoming-feature ExistentialAny
// RUN: %target-swift-frontend %s -verify -swift-version 5 -typecheck -I %t

// Test that the feature state used to compile a module is not reflected in
// the module file. The feature must not be enforced on protocols originating in
// a different module just because that module was compiled with the feature
// enabled.

#if M
public protocol P {}
#else
import M
func test(_: P) {}
#endif
15 changes: 15 additions & 0 deletions test/type/explicit_existential_multimodule2.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend %s -swift-version 5 -emit-module -DM -module-name M -emit-module-path %t/M.swiftmodule
// RUN: %target-swift-frontend %s -verify -swift-version 5 -typecheck -I %t -enable-upcoming-feature ExistentialAny

// Test that a protocol that requires 'any' *only* when the feature is enabled
// is diagnosed as expected when said protocol originates in a different module.
// In other words, test that deserialization does not affect 'any' migration
// diagnostics.

#if M
public protocol P {}
#else
import M
func test(_: P) {} // expected-error {{use of protocol 'P' as a type must be written 'any P'}}
#endif
30 changes: 21 additions & 9 deletions test/type/explicit_existential_swift6.swift
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,9 @@ extension MyError {
}
}

struct Wrapper {
typealias E = Error
}

func typealiasMemberReferences(metatype: Wrapper.Type) {
let _: Wrapper.E.Protocol = metatype.E.self
let _: (any Wrapper.E).Type = metatype.E.self
}

func testAnyTypeExpr() {
let _: (any P).Type = (any P).self
let _: (any P1 & P2).Type = (any P1 & P2).self

func test(_: (any P).Type) {}
test((any P).self)
Expand Down Expand Up @@ -305,6 +297,13 @@ enum EE : Equatable, any Empty { // expected-error {{raw type 'any Empty' is not
case hack
}

// Protocols from a serialized module (the standard library).
do {
// expected-error@+1 {{use of protocol 'Decodable' as a type must be written 'any Decodable'}}
let _: Decodable
// expected-error@+1 2 {{use of 'Codable' (aka 'Decodable & Encodable') as a type must be written 'any Codable' (aka 'any Decodable & Encodable')}}
let _: Codable
}

func testAnyFixIt() {
struct ConformingType : HasAssoc {
Expand All @@ -327,6 +326,19 @@ func testAnyFixIt() {
// expected-error@+2 {{use of protocol 'HasAssoc' as a type must be written 'any HasAssoc'}}{{10-18=(any HasAssoc)}}
// expected-error@+1 {{use of protocol 'HasAssoc' as a type must be written 'any HasAssoc'}}{{30-38=(any HasAssoc)}}
let _: HasAssoc.Protocol = HasAssoc.self
do {
struct Wrapper {
typealias HasAssocAlias = HasAssoc
}
let wrapperMeta: Wrapper.Type
// FIXME: Both of these fix-its are wrong.
// 1. 'any' is attached to 'HasAssocAlias' instead of 'Wrapper.HasAssocAlias'
// 2. What is the correct fix-it for the initializer?
//
// expected-error@+2:20 {{use of 'Wrapper.HasAssocAlias' (aka 'HasAssoc') as a type must be written 'any Wrapper.HasAssocAlias' (aka 'any HasAssoc')}}{{20-33=(any HasAssocAlias)}}
// expected-error@+1:57 {{use of 'Wrapper.HasAssocAlias' (aka 'HasAssoc') as a type must be written 'any Wrapper.HasAssocAlias' (aka 'any HasAssoc')}}{{57-70=(any HasAssocAlias)}}
let _: Wrapper.HasAssocAlias.Protocol = wrapperMeta.HasAssocAlias.self
}
// expected-error@+1 {{use of protocol 'HasAssoc' as a type must be written 'any HasAssoc'}}{{11-19=any HasAssoc}}
let _: (HasAssoc).Protocol = (any HasAssoc).self
// expected-error@+1 {{use of protocol 'HasAssoc' as a type must be written 'any HasAssoc'}}{{10-18=(any HasAssoc)}}
Expand Down