Skip to content
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

[SR-5777] Existential Metatype in generics #48347

Closed
swift-ci opened this issue Aug 27, 2017 · 10 comments
Closed

[SR-5777] Existential Metatype in generics #48347

swift-ci opened this issue Aug 27, 2017 · 10 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself

Comments

@swift-ci
Copy link
Collaborator

Previous ID SR-5777
Radar None
Original Reporter morpheby (JIRA User)
Type Bug
Status Resolved
Resolution Duplicate
Environment

Swift 4.0 (57bb55f)

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug
Assignee None
Priority Medium

md5: fb37d7b27c98b21b2d54c8bc6055fcc8

duplicates:

  • SR-55 non-@objc protocol existentials do not conform to their own protocol type

Issue Description:

The bug appears with the following (supposedly correct) code:

class C {
    let s: String
    required init(s: String) {
        self.s = s
    }
}
 
protocol D {
    var x: String { get set }
}
 
class V: C, D {
    var x: String = "123"
}
 
typealias X = C & D
 
class B {
    func f<T>(_ type: T.Type, s: String) -> T where T: C, T: D {
        return type.init(s: s)
    }
 }
let t: X.Type = V.self
let x: C = B().f(t, s: "Test")

Result is

error: generic parameter 'T' could not be inferred
@swift-ci
Copy link
Collaborator Author

Comment by Ilya Mikhaltsou (JIRA)

Through analysis of the code I've managed to come up with three potential solutions:

1. Implement UnknownMetatypeType : AnyMetatypeType, to be used specifically with .Type expressions that have a generic parameter

2. Implement fixes for constraint system and expression rewriter to accommodate the possibility of MetatypeType being in fact ExistentialMetatypeType. Patch to fix constraint system:

diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp
index ef44b95d50..02d9931e22 100644
--- a/lib/AST/Module.cpp
+++ b/lib/AST/Module.cpp
@@ -603,19 +603,20 @@ ModuleDecl::lookupConformance(Type type, ProtocolDecl *protocol,
if (!protocol->hasValidSignature())
return None;

+
+ auto layout = type->getExistentialLayout();
+
// If the existential type cannot be represented or the protocol does not
// conform to itself, there's no point in looking further.
- if (!protocol->existentialConformsToSelf() ||
+ if ((!layout.superclass && !protocol->existentialConformsToSelf()) ||
!protocol->existentialTypeSupported(resolver))
return None;

- auto layout = type->getExistentialLayout();
-
// Due to an IRGen limitation, witness tables cannot be passed from an
// existential to an archetype parameter, so for now we restrict this to
// @objc protocols.
- if (!layout.isObjC())
- return None;
+// if (!layout.isObjC())
+// return None;

// If the existential is class-constrained, the class might conform
// concretely.
diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
index 73ebc03aca..b23e386936 100644
--- a/lib/AST/Type.cpp
+++ b/lib/AST/Type.cpp
@@ -1573,6 +1573,9 @@ bool TypeBase::mayHaveSuperclass() {
if (auto archetype = getAs<ArchetypeType>())
return (bool)archetype->requiresClass();

+ if (auto compositionTy = getAs<ProtocolCompositionType>())
+ return (bool)compositionTy->requiresClass();
+
return is<DynamicSelfType>();
}

diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
index 60ac895bb6..ee887f572a 100644
--- a/lib/Sema/CSApply.cpp
+++ b/lib/Sema/CSApply.cpp
@@ -5031,9 +5031,9 @@ Expr *ExprRewriter::coerceExistential(Expr *expr, Type toType,

// Look through metatypes
while (fromInstanceType->is<AnyMetatypeType>() &&
- toInstanceType->is<ExistentialMetatypeType>()) {
+ toInstanceType->is<AnyMetatypeType>()) {
fromInstanceType = fromInstanceType->castTo<AnyMetatypeType>()->getInstanceType();
- toInstanceType = toInstanceType->castTo<ExistentialMetatypeType>()->getInstanceType();
+ toInstanceType = toInstanceType->castTo<AnyMetatypeType>()->getInstanceType();
}

ASTContext &ctx = tc.Context;
@@ -5929,9 +5929,10 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,

case ConversionRestrictionKind::Existential:
case ConversionRestrictionKind::MetatypeToExistentialMetatype:
+ case ConversionRestrictionKind::ExistentialMetatypeToMetatype:
return coerceExistential(expr, toType, locator);

- case ConversionRestrictionKind::ExistentialMetatypeToMetatype:
+ case ConversionRestrictionKind::ExistentialMetatypeToSuperclassMetatype:
return coerceSuperclass(expr, toType, locator);

case ConversionRestrictionKind::ClassMetatypeToAnyObject: {
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
index 31ed35f41b..8b821f8186 100644
--- a/lib/Sema/CSSimplify.cpp
+++ b/lib/Sema/CSSimplify.cpp
@@ -2044,12 +2044,15 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
ConversionRestrictionKind::MetatypeToExistentialMetatype);
}

- // Existential-metatype-to-superclass-metatype conversion.
+ // Existential-metatype-to-superclass-metatype conversion
+ // or Existential-metatype-to-metatype conversion (try both)
if (type2->is<MetatypeType>()) {
if (auto *meta1 = type1->getAs<ExistentialMetatypeType>()) {
if (meta1->getInstanceType()->isClassExistentialType()) {
conversionsOrFixes.push_back(
- ConversionRestrictionKind::ExistentialMetatypeToMetatype);
+ ConversionRestrictionKind::ExistentialMetatypeToSuperclassMetatype);
+ conversionsOrFixes.push_back(
+ ConversionRestrictionKind::ExistentialMetatypeToMetatype);
}
}
}
@@ -2609,14 +2612,15 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
TypeMatchOptions flags) {
// Dig out the fixed type to which this type refers.
type = getFixedTypeRecursive(type, flags, /*wantRValue=*/true);
+ auto desugar = type->getDesugaredType();

// If we hit a type variable without a fixed type, we can't
// solve this yet.
- if (type->isTypeVariableOrMember()) {
+ if (desugar->isTypeVariableOrMember()) {
// If we're supposed to generate constraints, do so.
if (flags.contains(TMF_GenerateConstraints)) {
addUnsolvedConstraint(
- Constraint::create(*this, kind, type, protocol->getDeclaredType(),
+ Constraint::create(*this, kind, desugar, protocol->getDeclaredType(),
getConstraintLocator(locator)));
return SolutionKind::Solved;
}
@@ -2630,7 +2634,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
switch (kind) {
case ConstraintKind::SelfObjectOfProtocol:
if (auto conformance =
- TC.containsProtocol(type, protocol, DC,
+ TC.containsProtocol(desugar, protocol, DC,
ConformanceCheckFlags::InExpression)) {
CheckedConformances.push_back({getConstraintLocator(locator),
*conformance});
@@ -2641,7 +2645,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
case ConstraintKind::LiteralConformsTo: {
// Check whether this type conforms to the protocol.
if (auto conformance =
- TC.conformsToProtocol(type, protocol, DC,
+ TC.conformsToProtocol(desugar, protocol, DC,
ConformanceCheckFlags::InExpression)) {
CheckedConformances.push_back({getConstraintLocator(locator),
*conformance});
@@ -2659,7 +2663,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(

// See if there's anything we can do to fix the conformance:
OptionalTypeKind optionalKind;
- if (auto optionalObjectType = type->getAnyOptionalObjectType(optionalKind)) {
+ if (auto optionalObjectType = desugar->getAnyOptionalObjectType(optionalKind)) {
if (optionalKind == OTK_Optional) {
TypeMatchOptions subflags = getDefaultDecompositionOptions(flags);
// The underlying type of an optional may conform to the protocol if the
@@ -4363,11 +4367,26 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
locator.withPathElement(ConstraintLocator::InstanceType));

// for $< in { <, <c, <oc }:
- // for P protocol, C class, D class,
- // (P & C) : D ===> (P & C).Type $< D.Type
+ // for P protocol, C class, Dx class,
+ // (P & C) $< D ===> (P & C).Type $< D.Type
case ConversionRestrictionKind::ExistentialMetatypeToMetatype: {
addContextualScore();

+ return matchTypes(
+ type1->castTo<ExistentialMetatypeType>()->getInstanceType(),
+ type2->castTo<MetatypeType>()->getInstanceType(),
+ ConstraintKind::Subtype,
+ subflags,
+ locator.withPathElement(ConstraintLocator::InstanceType));
+
+ }
+
+ // for $< in { <, <c, <oc }:
+ // for P protocol, C class, D class,
+ // C : D ===> (P & C).Type $< D.Type
+ case ConversionRestrictionKind::ExistentialMetatypeToSuperclassMetatype: {
+ addContextualScore();
+
auto instance1 = type1->castTo<ExistentialMetatypeType>()->getInstanceType();
auto instance2 = type2->castTo<MetatypeType>()->getInstanceType();
auto superclass1 = TC.getSuperClassOf(instance1);
@@ -4383,6 +4402,7 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
locator.withPathElement(ConstraintLocator::InstanceType));

}
+
// for $< in { <, <c, <oc }:
// T $< U ===> T $< U?
case ConversionRestrictionKind::ValueToOptional: {
diff --git a/lib/Sema/Constraint.cpp b/lib/Sema/Constraint.cpp
index b23a49c83e..2b02745fac 100644
--- a/lib/Sema/Constraint.cpp
+++ b/lib/Sema/Constraint.cpp
@@ -444,6 +444,8 @@ StringRef swift::constraints::getName(ConversionRestrictionKind kind) {
return "[metatype-to-existential-metatype]";
case ConversionRestrictionKind::ExistentialMetatypeToMetatype:
return "[existential-metatype-to-metatype]";
+ case ConversionRestrictionKind::ExistentialMetatypeToSuperclassMetatype:
+ return "[existential-metatype-to-superclass-metatype]";
case ConversionRestrictionKind::ValueToOptional:
return "[value-to-optional]";
case ConversionRestrictionKind::OptionalToOptional:
diff --git a/lib/Sema/Constraint.h b/lib/Sema/Constraint.h
index ddbf2a0d41..28be113933 100644
--- a/lib/Sema/Constraint.h
+++ b/lib/Sema/Constraint.h
@@ -194,7 +194,9 @@ enum class ConversionRestrictionKind {
Existential,
/// Metatype to existential metatype conversion.
MetatypeToExistentialMetatype,
- /// Existential metatype to metatype conversion.
+ /// Existential metatype to superclass metatype conversion.
+ ExistentialMetatypeToSuperclassMetatype,
+ // Existential metatype to metatype conversion.
ExistentialMetatypeToMetatype,
/// T -> U? value to optional conversion (or to implicitly unwrapped optional).
ValueToOptional,

This doesn't include expression rewriting and fails with an error at attempting to do so due to ExistentialMetatypeType != MetatypeType, but fixes the constraint solver.

3. Make .Type expressions with generic parameters always produce ExistentialMetatypeType and introduce SIL Lowering for MetatypeType to ExistentialMetatypeType. Partial patch:

diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
index fb97da8adc..81e0296f6c 100644
--- a/lib/Sema/TypeCheckType.cpp
+++ b/lib/Sema/TypeCheckType.cpp
@@ -2991,7 +2991,7 @@ Type TypeResolver::buildMetatypeType(
MetatypeTypeRepr *repr,
Type instanceType,
Optional<MetatypeRepresentation> storedRepr) {
- if (instanceType->isAnyExistentialType()) {
+ if (instanceType->isAnyExistentialType() || instanceType->isTypeParameter()) {
// TODO: diagnose invalid representations?
return ExistentialMetatypeType::get(instanceType, storedRepr);
} else {

With this, constraint solver works, expression rewriter works, but SIL doesn't.

I'm unsure of what is the best way to fix this, and maybe there are even other ways I haven't though of.

@belkadan
Copy link
Contributor

I believe this behavior is actually correct, since we don't consider a protocol value (existential value) with type P to conform to the protocol P. @slavapestov can elaborate.

@swift-ci
Copy link
Collaborator Author

Comment by Ilya Mikhaltsou (JIRA)

Well, if this remains so, isn’t this a major limitation that will need to be fixed later? And seemingly from the changes I tried to do, won’t it be problematic to do those changes later, with a locked down ABI?

@swift-ci
Copy link
Collaborator Author

Comment by Ilya Mikhaltsou (JIRA)

@slavapestov, I wouldn’t be so sure it is a duplicate. Resolving SR-55 would not completely fix this, and resolvong this does not require resolving SR-55 (at least not third option and first option), I think.

@slavapestov
Copy link
Member

morpheby (JIRA User) It's a duplicate. 😉

@swift-ci
Copy link
Collaborator Author

Comment by Ilya Mikhaltsou (JIRA)

Ugh 🙂 ok. @slavapestov, any plans to resolve it soon? If not soon, I may take a try at fixing protocols first, but then I don’t know what is the best course to fix metatypes

@slavapestov
Copy link
Member

morpheby (JIRA User) implementing self-conforming protocols is a very difficult problem because of some fundamental design descisions in the compiler and runtime representation. I don't think we've figured out an efficient solution yet.

@swift-ci
Copy link
Collaborator Author

Comment by Ilya Mikhaltsou (JIRA)

@slavapestov, yes, I definitely noticed that x) Is there anywhere it is discussed, to see what you've already considered?

Also, I think I will try solving it anyway, at least just for fun of it.

@slavapestov
Copy link
Member

Here is an old e-mail I wrote on the topic: https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160815/026349.html

> Also, I think I will try solving it anyway, at least just for fun of it.

I won't try to stop you, but keep in mind the tricky part here is the actual runtime representation and calling conventions, not the type checker and SIL changes. 🙂

@swift-ci
Copy link
Collaborator Author

Comment by Ilya Mikhaltsou (JIRA)

@slavapestov Thank you 🙂

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself
Projects
None yet
Development

No branches or pull requests

3 participants