From 61f806e01c0c5b5015ca0c1cb548026264e836bf Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 13 Nov 2017 15:37:40 -0800 Subject: [PATCH 1/2] SILOptimizer: Fix devirtualization of static method returning 'Self' in a generic class Fixes . --- lib/AST/Type.cpp | 19 +++++++++++------- .../devirt_static_covariant_return.swift | 20 +++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) create mode 100644 test/SILOptimizer/devirt_static_covariant_return.swift diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index df753e4a7220b..46807d32461fe 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -3336,17 +3336,22 @@ const DependentMemberType *TypeBase::findUnresolvedDependentMemberType() { Type TypeBase::getSuperclassForDecl(const ClassDecl *baseClass) { Type t(this); + + if (!t->getAnyNominal()) { + if (auto archetype = t->getAs()) { + t = archetype->getSuperclass(); + } else if (auto dynamicSelfTy = t->getAs()) { + t = dynamicSelfTy->getSelfType(); + } else if (auto compositionTy = t->getAs()) { + t = compositionTy->getExistentialLayout().superclass; + } + } + while (t) { // If we have a class-constrained archetype or class-constrained // existential, get the underlying superclass constraint. auto *nominalDecl = t->getAnyNominal(); - if (!nominalDecl) { - assert(t->is() || t->isExistentialType() && - "expected a class, archetype or existential"); - t = t->getSuperclass(); - assert(t && "archetype or existential is not class constrained"); - continue; - } + assert(nominalDecl && "expected nominal type here"); assert(isa(nominalDecl) && "expected a class here"); if (nominalDecl == baseClass) diff --git a/test/SILOptimizer/devirt_static_covariant_return.swift b/test/SILOptimizer/devirt_static_covariant_return.swift new file mode 100644 index 0000000000000..fa525981c0f30 --- /dev/null +++ b/test/SILOptimizer/devirt_static_covariant_return.swift @@ -0,0 +1,20 @@ +// RUN: %target-swift-frontend -O -whole-module-optimization %s -emit-sil -sil-verify-all | %FileCheck %s + +// CHECK-NOT: class_method + +// Static method returning Self in generic class +public class Factory { + public required init() {} + + @inline(never) class func bar() -> Self { + return self.init() + } + + @inline(never) class func foo() -> Self { + return bar() + } +} + +public func foo(m: Factory.Type) { + m.foo() +} From b6200728a64db5e0d73ae4fae76a7c4fe8bbc786 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 14 Nov 2017 00:07:05 -0800 Subject: [PATCH 2/2] Sema: Fix crash on invalid openUnboundGenericType() We can't construct a nominal type with an ErrorType as a parent, so in the bad circular case, return a Type() instead to bail out of transform() altogether. --- lib/AST/Type.cpp | 4 +++- lib/Sema/ConstraintSystem.cpp | 11 +++++++---- .../28858-val-isa-used-on-a-null-pointer.swift | 2 +- ...pe-expected-a-class-archetype-or-existential.swift | 2 +- 4 files changed, 12 insertions(+), 7 deletions(-) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/28858-val-isa-used-on-a-null-pointer.swift (88%) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/28863-t-is-archetypetype-t-isexistentialtype-expected-a-class-archetype-or-existential.swift (88%) diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 46807d32461fe..d27e11859ad84 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -3368,7 +3368,9 @@ TypeBase::getContextSubstitutions(const DeclContext *dc, assert(dc->isTypeContext()); Type baseTy(this); - assert(!baseTy->hasLValueType() && !baseTy->is()); + assert(!baseTy->hasLValueType() && + !baseTy->is() && + !baseTy->is()); // The resulting set of substitutions. Always use this to ensure we // don't miss out on NRVO anywhere. diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 39c5ae6ad002f..8fbfe3dc4a210 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -390,14 +390,12 @@ Type ConstraintSystem::openUnboundGenericType(UnboundGenericType *unbound, ConstraintLocatorBuilder locator, OpenedTypeMap &replacements) { auto unboundDecl = unbound->getDecl(); - if (unboundDecl->isInvalid()) - return ErrorType::get(getASTContext()); // If the unbound decl hasn't been validated yet, we have a circular // dependency that isn't being diagnosed properly. if (!unboundDecl->getGenericSignature()) { TC.diagnose(unboundDecl, diag::circular_reference); - return ErrorType::get(unbound); + return Type(); } auto parentTy = unbound->getParent(); @@ -458,7 +456,7 @@ Type ConstraintSystem::openUnboundGenericType( if (!type->hasUnboundGenericType()) return type; - return type.transform([&](Type type) -> Type { + type = type.transform([&](Type type) -> Type { if (auto unbound = type->getAs()) { OpenedTypeMap replacements; return openUnboundGenericType(unbound, locator, replacements); @@ -466,6 +464,11 @@ Type ConstraintSystem::openUnboundGenericType( return type; }); + + if (!type) + return ErrorType::get(getASTContext()); + + return type; } Type ConstraintSystem::openType(Type type, OpenedTypeMap &replacements) { diff --git a/validation-test/compiler_crashers/28858-val-isa-used-on-a-null-pointer.swift b/validation-test/compiler_crashers_fixed/28858-val-isa-used-on-a-null-pointer.swift similarity index 88% rename from validation-test/compiler_crashers/28858-val-isa-used-on-a-null-pointer.swift rename to validation-test/compiler_crashers_fixed/28858-val-isa-used-on-a-null-pointer.swift index 026a0b172fe7a..8d55ec18bc216 100644 --- a/validation-test/compiler_crashers/28858-val-isa-used-on-a-null-pointer.swift +++ b/validation-test/compiler_crashers_fixed/28858-val-isa-used-on-a-null-pointer.swift @@ -6,7 +6,7 @@ // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors // REQUIRES: asserts -// RUN: not --crash %target-swift-frontend %s -emit-ir +// RUN: not %target-swift-frontend %s -emit-ir func a class a