Skip to content

Non-copyable generics fixes, part 8 #72112

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 4 commits into from
Mar 7, 2024
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
4 changes: 0 additions & 4 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -7630,10 +7630,6 @@ NOTE(add_inverse,none,
NOTE(note_inverse_preventing_conformance,none,
"%0 has '~%1' constraint preventing implicit '%1' conformance",
(Type, StringRef))
NOTE(note_inverse_preventing_conformance_implicit,none,
"%kind0 has '~%1' constraint on a generic parameter, "
"making its '%1' conformance conditional",
(const ValueDecl *, StringRef))
NOTE(note_inverse_preventing_conformance_explicit,none,
"%kind0 has '~%1' constraint preventing '%1' conformance",
(const ValueDecl *, StringRef))
Expand Down
20 changes: 0 additions & 20 deletions include/swift/AST/InverseMarking.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ namespace swift {
struct InverseMarking {
enum class Kind : uint8_t {
None, // No inverse marking is present
Inferred, // Inverse is inferred based on generic parameters.
Explicit, // Inverse is explicitly present.
LegacyExplicit, // An equivalent, explicit legacy annotation is present.
};
Expand Down Expand Up @@ -84,25 +83,6 @@ struct InverseMarking {
return Mark(k, loc);
}
};

private:
Mark inverse;
Mark positive;

public:

// Creates an empty marking.
InverseMarking() {}

Mark const& getInverse() const { return inverse; }
Mark const& getPositive() const { return positive; }

static InverseMarking forInverse(Kind kind, SourceLoc loc = SourceLoc()) {
InverseMarking marking;
marking.inverse.set(kind, loc);
marking.positive.set(Kind::None);
return marking;
}
};

}
Expand Down
1 change: 0 additions & 1 deletion lib/AST/ConformanceLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,6 @@ getBuiltinInvertibleProtocolConformance(NominalTypeDecl *nominal,
// An inverse ~Copyable prevents conformance.
return ProtocolConformanceRef::forInvalid();

case InverseMarking::Kind::Inferred: // ignore "inferred" inverse marking
case InverseMarking::Kind::None:
break;
}
Expand Down
55 changes: 2 additions & 53 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6694,9 +6694,9 @@ NominalTypeDecl::hasInverseMarking(InvertibleProtocolKind target) const {
if (!ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics))
return InverseMarking::Mark(InverseMarking::Kind::None);

// Claim that the tuple decl has an inferred ~TARGET marking.
// Claim that the tuple decl has an explicit ~TARGET marking.
if (isa<BuiltinTupleDecl>(this))
return InverseMarking::Mark(InverseMarking::Kind::Inferred);
return InverseMarking::Mark(InverseMarking::Kind::Explicit);

if (auto P = dyn_cast<ProtocolDecl>(this))
return P->hasInverseMarking(target);
Expand All @@ -6705,57 +6705,6 @@ NominalTypeDecl::hasInverseMarking(InvertibleProtocolKind target) const {
if (auto inverse = findInverseInInheritance(getInherited(), target))
return inverse;

// Check the generic parameters for an explicit ~TARGET marking
// which would result in an Inferred ~TARGET marking for this context.
auto *gpList = getParsedGenericParams();
if (!gpList)
return InverseMarking::Mark();

llvm::SmallSet<GenericTypeParamDecl *, 4> params;

// Scan the inheritance clauses of generic parameters only for an inverse.
for (GenericTypeParamDecl *param : gpList->getParams()) {
auto inverse = findInverseInInheritance(param->getInherited(), target);

// Inverse is inferred from one of the generic parameters.
if (inverse)
return inverse.with(InverseMarking::Kind::Inferred);

params.insert(param);
}

// Next, scan the where clause and return the result.
auto whereClause = getTrailingWhereClause();
if (!whereClause)
return InverseMarking::Mark();

auto requirements = whereClause->getRequirements();
for (unsigned i : indices(requirements)) {
auto requirementRepr = requirements[i];
if (requirementRepr.getKind() != RequirementReprKind::TypeConstraint)
continue;

auto *subjectRepr =
dyn_cast<UnqualifiedIdentTypeRepr>(requirementRepr.getSubjectRepr());

if (!(subjectRepr && subjectRepr->isBound()))
continue;

auto *subjectGP =
dyn_cast<GenericTypeParamDecl>(subjectRepr->getBoundDecl());
if (!subjectGP || !params.contains(subjectGP))
continue;

auto *constraintRepr =
dyn_cast<InverseTypeRepr>(requirementRepr.getConstraintRepr());
if (!constraintRepr || constraintRepr->isInvalid())
continue;

if (constraintRepr->isInverseOf(target, getDeclContext()))
return InverseMarking::Mark(InverseMarking::Kind::Inferred,
constraintRepr->getLoc());
}

return InverseMarking::Mark();
}

Expand Down
148 changes: 74 additions & 74 deletions lib/AST/FeatureSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#include "swift/AST/Decl.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/InverseMarking.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/ParameterList.h"
#include "swift/AST/Pattern.h"
#include "clang/AST/DeclObjC.h"
Expand All @@ -33,62 +33,6 @@ static bool usesTypeMatching(Decl *decl, llvm::function_ref<bool(Type)> fn) {
return false;
}

/// \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::Mark const &)> isRelevantInverse) {

auto getTypeDecl = [](Type type) -> TypeDecl * {
if (auto genericTy = type->getAnyGeneric())
return genericTy;
if (auto gtpt = dyn_cast<GenericTypeParamType>(type))
return gtpt->getDecl();
return nullptr;
};

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

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

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 *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;
}

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

return false;
}

// ----------------------------------------------------------------------------
// MARK: - Standard Features
// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -255,10 +199,39 @@ static bool usesFeatureExtensionMacros(Decl *decl) {
}

static bool usesFeatureMoveOnly(Decl *decl) {
return hasInverse(decl, InvertibleProtocolKind::Copyable,
[](auto &marking) -> bool {
return marking.is(InverseMarking::Kind::LegacyExplicit);
});
if (auto *extension = dyn_cast<ExtensionDecl>(decl)) {
if (auto *nominal = extension->getExtendedNominal())
return usesFeatureMoveOnly(nominal);
return false;
}

auto hasInverseInType = [&](Type type) {
return type.findIf([&](Type type) -> bool {
if (auto *NTD = type->getAnyNominal()) {
if (NTD->getAttrs().hasAttribute<MoveOnlyAttr>())
return true;
}
return false;
});
};

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 (NTD->getAttrs().hasAttribute<MoveOnlyAttr>())
return true;
}

return false;
}

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

return false;
}

static bool usesFeatureMoveOnlyResilientTypes(Decl *decl) {
Expand Down Expand Up @@ -525,22 +498,49 @@ static bool usesFeatureRawLayout(Decl *decl) {
UNINTERESTING_FEATURE(Embedded)

static bool usesFeatureNoncopyableGenerics(Decl *decl) {
auto checkInverseMarking = [](auto &marking) -> bool {
switch (marking.getKind()) {
case InverseMarking::Kind::None:
case InverseMarking::Kind::LegacyExplicit: // covered by other checks.
return false;
if (auto *valueDecl = dyn_cast<ValueDecl>(decl)) {
if (isa<StructDecl, EnumDecl, ClassDecl>(decl)) {
auto *nominalDecl = cast<NominalTypeDecl>(valueDecl);

case InverseMarking::Kind::Explicit:
case InverseMarking::Kind::Inferred:
return true;
InvertibleProtocolSet inverses;
bool anyObject = false;
getDirectlyInheritedNominalTypeDecls(nominalDecl, inverses, anyObject);
if (!inverses.empty())
return true;
}
};

return hasInverse(decl, InvertibleProtocolKind::Copyable,
checkInverseMarking) ||
hasInverse(decl, InvertibleProtocolKind::Escapable,
checkInverseMarking);
if (isa<AbstractFunctionDecl>(valueDecl) ||
isa<AbstractStorageDecl>(valueDecl)) {
if (valueDecl->getInterfaceType().findIf([&](Type type) -> bool {
if (auto *nominalDecl = type->getAnyNominal()) {
if (isa<StructDecl, EnumDecl, ClassDecl>(nominalDecl))
return usesFeatureNoncopyableGenerics(nominalDecl);
}
return false;
})) {
return true;
}
}
}

if (auto *ext = dyn_cast<ExtensionDecl>(decl)) {
if (auto *nominal = ext->getExtendedNominal())
if (usesFeatureNoncopyableGenerics(nominal))
return true;
}

SmallVector<Requirement, 2> reqs;
SmallVector<InverseRequirement, 2> inverseReqs;

if (auto *proto = dyn_cast<ProtocolDecl>(decl)) {
proto->getRequirementSignature().getRequirementsWithInverses(
proto, reqs, inverseReqs);
} else if (auto *genCtx = decl->getAsGenericContext()) {
if (auto genericSig = genCtx->getGenericSignature())
genericSig->getRequirementsWithInverses(reqs, inverseReqs);
}

return !inverseReqs.empty();
}

static bool usesFeatureStructLetDestructuring(Decl *decl) {
Expand Down
10 changes: 4 additions & 6 deletions lib/AST/RequirementMachine/KnuthBendix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,8 @@ RewriteSystem::computeCriticalPair(ArrayRef<Symbol>::const_iterator from,
return true;
}

/// Computes the confluent completion using the Knuth-Bendix algorithm and
/// returns a status code.
///
/// The first element of the pair is a status.
/// Runs the Knuth-Bendix algorithm and returns a pair consisting of a
/// status code and code-specific result.
///
/// The status is CompletionResult::MaxRuleCount if we add more than
/// \p maxRuleCount rules.
Expand All @@ -341,8 +339,8 @@ RewriteSystem::computeCriticalPair(ArrayRef<Symbol>::const_iterator from,
/// Otherwise, the status is CompletionResult::Success and the second element
/// is zero.
std::pair<CompletionResult, unsigned>
RewriteSystem::computeConfluentCompletion(unsigned maxRuleCount,
unsigned maxRuleLength) {
RewriteSystem::performKnuthBendix(unsigned maxRuleCount,
unsigned maxRuleLength) {
assert(Initialized);
assert(!Minimized);
assert(!Frozen);
Expand Down
4 changes: 2 additions & 2 deletions lib/AST/RequirementMachine/RequirementMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy) {
unsigned ruleCount = System.getRules().size();

// First, run the Knuth-Bendix algorithm to resolve overlapping rules.
auto result = System.computeConfluentCompletion(MaxRuleCount, MaxRuleLength);
auto result = System.performKnuthBendix(MaxRuleCount, MaxRuleLength);

unsigned rulesAdded = (System.getRules().size() - ruleCount);

Expand Down Expand Up @@ -492,7 +492,7 @@ RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy) {
return std::make_pair(CompletionResult::MaxRuleLength,
ruleCount + i);
}
if (newRule.getNesting() > MaxConcreteNesting) {
if (newRule.getNesting() > MaxConcreteNesting + System.getDeepestInitialRule()) {
return std::make_pair(CompletionResult::MaxConcreteNesting,
ruleCount + i);
}
Expand Down
2 changes: 2 additions & 0 deletions lib/AST/RequirementMachine/RewriteSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ RewriteSystem::RewriteSystem(RewriteContext &ctx)
Frozen = 0;
RecordLoops = 0;
LongestInitialRule = 0;
DeepestInitialRule = 0;
}

RewriteSystem::~RewriteSystem() {
Expand Down Expand Up @@ -88,6 +89,7 @@ void RewriteSystem::initialize(

for (const auto &rule : getLocalRules()) {
LongestInitialRule = std::max(LongestInitialRule, rule.getDepth());
DeepestInitialRule = std::max(DeepestInitialRule, rule.getNesting());
}
}

Expand Down
Loading