Skip to content

[AST/Sema] NonCopyableGenerics: Address some of the request circularity issues #71659

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 16 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7c153e8
[Sema] InvertibleAnnotationRequest: Resolve requirements only if cons…
xedin Feb 14, 2024
d1b4a0c
[Sema] InvertibleAnnotationRequest: Only resolve types of requirement…
xedin Feb 14, 2024
b51600d
[AST] Add a way to check whether TypeRepr is an inverse of the given …
xedin Feb 15, 2024
09ab12d
[AST] Simplify `ProtocolDecl::requiresInvertible` check
xedin Feb 15, 2024
3257ff4
[AST] Move `hasInverseMarking` into `ProtocolDecl` and produce a loc …
xedin Feb 15, 2024
ed8cc09
[AST] InvertibleAnnotationRequest: Remove logic associated with proto…
xedin Feb 15, 2024
355e02d
[AST] Route inverse marking detection through `TypeDecl::hasInverseMa…
xedin Feb 16, 2024
937d9d0
[AST] Route `hasInverse` through `hasInverseMarking` and simplify it
xedin Feb 16, 2024
6e0cfb5
[AST] NonCopyableGenerics: Remove a few uses for `getMarking(...).get…
xedin Feb 16, 2024
b643cd8
[AST/Sema] NonCopyableGenerics: Remove all but one direct use of `get…
xedin Feb 16, 2024
cc72581
[AST] NoCopyableGenerics: Remove last use of `getMarking(...)`
xedin Feb 16, 2024
15b79c5
[AST/Sema] NFC: Remove obsolete `getMarking(...)` and `InvertibleAnno…
xedin Feb 16, 2024
68b49b6
[AST] NonCopyableGenerics: Don't resolve requirements in `NominalType…
xedin Feb 16, 2024
c81db8e
[AST] NonCopyableGenerics: Expand `hasInverseMarking` to support asso…
xedin Feb 19, 2024
b09aad8
[Tests] NFC: Adjust previous fixed test-case
xedin Feb 19, 2024
b6af269
[AST] Remove `TypeDecl::hasInverse` and move `canBe{Copyable, Escapab…
xedin Feb 19, 2024
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
63 changes: 38 additions & 25 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "swift/AST/IfConfigClause.h"
#include "swift/AST/Import.h"
#include "swift/AST/Initializer.h"
#include "swift/AST/InverseMarking.h"
#include "swift/AST/LayoutConstraint.h"
#include "swift/AST/LifetimeAnnotation.h"
#include "swift/AST/ReferenceCounting.h"
Expand Down Expand Up @@ -3214,30 +3215,6 @@ class TypeDecl : public ValueDecl {
};
};

/// "Does a conformance for Copyable exist for this type declaration?"
///
/// This doesn't mean that all instance of this type are Copyable, because
/// if a conditional conformance to Copyable exists, this method will return
/// true.
///
/// If you need a more precise answer, ask this Decl's corresponding
/// Type if it `isCopyable` instead of using this.
CanBeInvertible::Result canBeCopyable() const;

/// "Does a conformance for Escapable exist for this type declaration?"
///
/// This doesn't mean that all instance of this type are Escapable, because
/// if a conditional conformance to Escapable exists, this method will return
/// true.
///
/// If you need a more precise answer, ask this Decl's corresponding
/// Type if it `isEscapable` instead of using this.
CanBeInvertible::Result canBeEscapable() const;

/// Determine how the given invertible protocol was written on this TypeDecl,
/// if at all.
InverseMarking getMarking(InvertibleProtocolKind ip) const;

static bool classof(const Decl *D) {
return D->getKind() >= DeclKind::First_TypeDecl &&
D->getKind() <= DeclKind::Last_TypeDecl;
Expand All @@ -3255,7 +3232,7 @@ class TypeDecl : public ValueDecl {
}
};

/// A type declaration that can have generic parameters attached to it. Because
/// A type declaration that have generic parameters attached to it. Because
/// it has these generic parameters, it is always a DeclContext.
class GenericTypeDecl : public GenericContext, public TypeDecl {
public:
Expand Down Expand Up @@ -3930,6 +3907,10 @@ class AssociatedTypeDecl : public TypeDecl {
TypeDecl::getOverriddenDecl());
}

/// Determine whether this type has ~<target>` stated as
/// one of its inherited types.
InverseMarking::Mark hasInverseMarking(InvertibleProtocolKind target) const;

/// Retrieve the set of associated types overridden by this associated
/// type.
llvm::TinyPtrVector<AssociatedTypeDecl *> getOverriddenDecls() const;
Expand Down Expand Up @@ -4367,6 +4348,34 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
/// Returns null if the type is a class, or does not have a declared `deinit`.
DestructorDecl *getValueTypeDestructor();

/// "Does a conformance for Copyable exist for this type declaration?"
///
/// This doesn't mean that all instance of this type are Copyable, because
/// if a conditional conformance to Copyable exists, this method will return
/// true.
///
/// If you need a more precise answer, ask this Decl's corresponding
/// Type if it `isCopyable` instead of using this.
CanBeInvertible::Result canBeCopyable() const;

/// "Does a conformance for Escapable exist for this type declaration?"
///
/// This doesn't mean that all instance of this type are Escapable, because
/// if a conditional conformance to Escapable exists, this method will return
/// true.
///
/// If you need a more precise answer, ask this Decl's corresponding
/// Type if it `isEscapable` instead of using this.
CanBeInvertible::Result canBeEscapable() const;

/// Determine whether this type has `: <target>` stated explicitly in
/// its inheritance clause.
bool hasMarking(InvertibleProtocolKind target) const;

/// Determine whether this type has ~<target>` stated on
/// itself, one of its inherited types or `Self` requirements.
InverseMarking::Mark hasInverseMarking(InvertibleProtocolKind target) const;

// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) {
return D->getKind() >= DeclKind::First_NominalTypeDecl &&
Expand Down Expand Up @@ -5222,6 +5231,10 @@ class ProtocolDecl final : public NominalTypeDecl {
/// protocol.
bool inheritsFrom(const ProtocolDecl *Super) const;

/// Determine whether this protocol has ~<target>` stated on
/// itself, one of its inherited types or `Self` requirements.
InverseMarking::Mark hasInverseMarking(InvertibleProtocolKind target) const;

/// Determine whether this protocol requires conformance to `IP`, without
/// querying a generic signature.
bool requiresInvertible(InvertibleProtocolKind ip) const;
Expand Down
4 changes: 0 additions & 4 deletions include/swift/AST/InverseMarking.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

namespace swift {

class InvertibleAnnotationRequest;

/// Describes the way an inverse and its corresponding positive contraint
/// appears on a TypeDecl, i.e., the way it was marked.
struct InverseMarking {
Expand Down Expand Up @@ -91,8 +89,6 @@ struct InverseMarking {
Mark inverse;
Mark positive;

// This friend initializes the marks.
friend InvertibleAnnotationRequest;
public:

// Creates an empty marking.
Expand Down
22 changes: 0 additions & 22 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -429,28 +429,6 @@ class IsFinalRequest :
void cacheResult(bool value) const;
};

/// Determine the kind of invertible protocol markings for this declaration,
/// for example, if a conformance to IP or ~IP was written on it in the
/// inheritance clause and/or where clause.
class InvertibleAnnotationRequest
: public SimpleRequest<InvertibleAnnotationRequest,
InverseMarking(TypeDecl *, InvertibleProtocolKind),
RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

// Evaluation.
InverseMarking evaluate(Evaluator &evaluator,
TypeDecl *decl, InvertibleProtocolKind ip) const;

public:
// Caching.
bool isCached() const { return true; }
};

/// Determine whether the given declaration is 'dynamic''.
class IsDynamicRequest :
public SimpleRequest<IsDynamicRequest,
Expand Down
3 changes: 0 additions & 3 deletions include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,6 @@ SWIFT_REQUEST(TypeChecker, IsDynamicRequest, bool(ValueDecl *),
SeparatelyCached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, IsFinalRequest, bool(ValueDecl *), SeparatelyCached,
NoLocationInfo)
SWIFT_REQUEST(TypeChecker, InvertibleAnnotationRequest,
InverseMarking(TypeDecl *, InvertibleProtocolKind),
Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, IsGetterMutatingRequest, bool(AbstractStorageDecl *),
SeparatelyCached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, IsImplicitlyUnwrappedOptionalRequest,
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/TypeRepr.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ class alignas(1 << TypeReprAlignInBits) TypeRepr
/// Is this type representation a protocol?
bool isProtocolOrProtocolComposition(DeclContext *dc);

/// Is this `~<target>` representation.
bool isInverseOf(InvertibleProtocolKind target,
DeclContext *dc);

/// Is this type representation known to be invalid?
bool isInvalid() const { return Bits.TypeRepr.Invalid; }

Expand Down
72 changes: 37 additions & 35 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3368,9 +3368,8 @@ static bool usesFeatureFlowSensitiveConcurrencyCaptures(Decl *decl) {
/// \param isRelevantInverse the function used to inspect a mark corresponding
/// to an inverse to determine whether it "has" an inverse that we care about.
static bool hasInverse(
Decl *decl,
InvertibleProtocolKind ip,
std::function<bool(InverseMarking const&)> isRelevantInverse) {
Decl *decl, InvertibleProtocolKind ip,
std::function<bool(InverseMarking::Mark const &)> isRelevantInverse) {

auto getTypeDecl = [](Type type) -> TypeDecl* {
if (auto genericTy = type->getAnyGeneric())
Expand All @@ -3382,40 +3381,41 @@ static bool hasInverse(

if (auto *extension = dyn_cast<ExtensionDecl>(decl)) {
if (auto *nominal = extension->getSelfNominalTypeDecl())
if (isRelevantInverse(nominal->getMarking(ip)))
return true;
return hasInverse(nominal, ip, isRelevantInverse);
return false;
}

if (auto typeDecl = dyn_cast<TypeDecl>(decl)) {
if (isRelevantInverse(typeDecl->getMarking(ip)))
return true;
auto hasInverseInType = [&](Type type) {
return type.findIf([&](Type type) -> bool {
if (auto *typeDecl = getTypeDecl(type))
return hasInverse(typeDecl, ip, isRelevantInverse);
return false;
});
};

// Check the protocol's associated types too.
if (auto proto = dyn_cast<ProtocolDecl>(decl)) {
auto hasInverse =
llvm::any_of(proto->getAssociatedTypeMembers(),
[&](AssociatedTypeDecl *assocTyDecl) {
return isRelevantInverse(assocTyDecl->getMarking(ip));
});
if (hasInverse)
if (auto *TD = dyn_cast<TypeDecl>(decl)) {
if (auto *alias = dyn_cast<TypeAliasDecl>(TD))
return hasInverseInType(alias->getUnderlyingType());

if (auto *NTD = dyn_cast<NominalTypeDecl>(TD)) {
if (isRelevantInverse(NTD->hasInverseMarking(ip)))
return true;
}
}

if (auto value = dyn_cast<ValueDecl>(decl)) {
// Check for noncopyable types in the types of this declaration.
if (Type type = value->getInterfaceType()) {
bool hasInverse = type.findIf([&](Type type) {
if (auto *typeDecl = getTypeDecl(type))
if (isRelevantInverse(typeDecl->getMarking(ip)))
return true;
if (auto *P = dyn_cast<ProtocolDecl>(TD)) {
// Check the protocol's associated types too.
return llvm::any_of(
P->getAssociatedTypeMembers(), [&](AssociatedTypeDecl *ATD) {
return isRelevantInverse(ATD->hasInverseMarking(ip));
});
}

return false;
});
return false;
}

if (hasInverse)
return true;
}
if (auto *VD = dyn_cast<ValueDecl>(decl)) {
if (VD->hasInterfaceType())
return hasInverseInType(VD->getInterfaceType());
}

return false;
Expand All @@ -3424,8 +3424,8 @@ static bool hasInverse(
static bool usesFeatureMoveOnly(Decl *decl) {
return hasInverse(decl, InvertibleProtocolKind::Copyable,
[](auto &marking) -> bool {
return marking.getInverse().is(InverseMarking::Kind::LegacyExplicit);
});
return marking.is(InverseMarking::Kind::LegacyExplicit);
});
}

static bool usesFeatureLazyImmediate(Decl *D) { return false; }
Expand Down Expand Up @@ -3469,8 +3469,8 @@ static bool usesFeatureMoveOnlyPartialReinitialization(Decl *decl) {
}

static bool usesFeatureNoncopyableGenerics(Decl *decl) {
auto checkMarking = [](auto &marking) -> bool {
switch (marking.getInverse().getKind()) {
auto checkInverseMarking = [](auto &marking) -> bool {
switch (marking.getKind()) {
case InverseMarking::Kind::None:
case InverseMarking::Kind::LegacyExplicit: // covered by other checks.
return false;
Expand All @@ -3481,8 +3481,10 @@ static bool usesFeatureNoncopyableGenerics(Decl *decl) {
}
};

return hasInverse(decl, InvertibleProtocolKind::Copyable, checkMarking)
|| hasInverse(decl, InvertibleProtocolKind::Escapable, checkMarking);
return hasInverse(decl, InvertibleProtocolKind::Copyable,
checkInverseMarking) ||
hasInverse(decl, InvertibleProtocolKind::Escapable,
checkInverseMarking);
}

static bool usesFeatureOneWayClosureParameters(Decl *decl) {
Expand Down
3 changes: 1 addition & 2 deletions lib/AST/ConformanceLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,7 @@ getBuiltinInvertibleProtocolConformance(NominalTypeDecl *nominal,
case InvertibleProtocolKind::Copyable:
// If move-only classes is enabled, we'll check the markings.
if (ctx.LangOpts.hasFeature(Feature::MoveOnlyClasses)) {
auto marking = nominal->getMarking(*ip);
switch (marking.getInverse().getKind()) {
switch (nominal->hasInverseMarking(*ip).getKind()) {
case InverseMarking::Kind::LegacyExplicit:
case InverseMarking::Kind::Explicit:
// An inverse ~Copyable prevents conformance.
Expand Down
Loading