Skip to content

Fix generic method overrides #5424

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 5 commits into from
Oct 24, 2016
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
8 changes: 8 additions & 0 deletions include/swift/AST/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,14 @@ class alignas(1 << TypeAlignInBits) TypeBase {
Type getTypeOfMember(ModuleDecl *module, Type memberType,
const DeclContext *memberDC);

/// Get the type of a superclass member as seen from the subclass,
/// substituting generic parameters, dynamic Self return, and the
/// 'self' argument type as appropriate.
Type adjustSuperclassMemberDeclType(const ValueDecl *decl,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different from getTypeOfMember?

const ValueDecl *parentDecl,
Type memberType,
LazyResolver *resolver);

/// Return T if this type is Optional<T>; otherwise, return the null type.
Type getOptionalObjectType();

Expand Down
44 changes: 43 additions & 1 deletion lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3053,14 +3053,56 @@ Type TypeBase::getTypeOfMember(Module *module, Type memberType,
return memberType;

// Compute the set of member substitutions to apply.
TypeSubstitutionMap substitutions = getMemberSubstitutions(memberDC);
auto substitutions = getMemberSubstitutions(memberDC);
if (substitutions.empty())
return memberType;

// Perform the substitutions.
return memberType.subst(module, substitutions, None);
}

Type TypeBase::adjustSuperclassMemberDeclType(const ValueDecl *decl,
const ValueDecl *parentDecl,
Type memberType,
LazyResolver *resolver) {
auto *DC = parentDecl->getDeclContext();
auto superclass = getSuperclassForDecl(
DC->getAsClassOrClassExtensionContext(), resolver);
auto subs = superclass->getMemberSubstitutions(DC);

if (auto *parentFunc = dyn_cast<AbstractFunctionDecl>(parentDecl)) {
if (auto *func = dyn_cast<AbstractFunctionDecl>(decl)) {
auto *genericParams = func->getGenericParams();
auto *parentParams = parentFunc->getGenericParams();
if (genericParams && parentParams &&
genericParams->size() == parentParams->size()) {
for (unsigned i = 0, e = genericParams->size(); i < e; i++) {
auto paramTy = parentParams->getParams()[i]->getDeclaredInterfaceType()
->getCanonicalType().getPointer();
subs[paramTy] = genericParams->getParams()[i]
->getDeclaredInterfaceType();
}
}
}
}

auto type = memberType.subst(parentDecl->getModuleContext(), subs);

if (isa<AbstractFunctionDecl>(parentDecl)) {
type = type->replaceSelfParameterType(this);
if (auto func = dyn_cast<FuncDecl>(parentDecl)) {
if (func->hasDynamicSelf()) {
type = type->replaceCovariantResultType(this,
func->getNumParameterLists());
}
} else if (isa<ConstructorDecl>(parentDecl)) {
type = type->replaceCovariantResultType(this, /*uncurryLevel=*/2);
}
}

return type;
}

Identifier DependentMemberType::getName() const {
if (NameOrAssocType.is<Identifier>())
return NameOrAssocType.get<Identifier>();
Expand Down
29 changes: 8 additions & 21 deletions lib/SIL/SILFunctionType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2041,10 +2041,14 @@ SILConstantInfo TypeConverter::getConstantOverrideInfo(SILDeclRef derived,
auto selfInterfaceTy = derivedInterfaceTy.getInput()->getRValueInstanceType();

auto overrideInterfaceTy =
selfInterfaceTy->getTypeOfMember(M.getSwiftModule(), baseInterfaceTy,
base.getDecl()->getDeclContext());

// Copy generic signature from derived to the override type
selfInterfaceTy->adjustSuperclassMemberDeclType(
derived.getDecl(), base.getDecl(), baseInterfaceTy,
/*resolver=*/nullptr);

// Copy generic signature from derived to the override type, to handle
// the case where the base member is not generic (because the base class
// is concrete) but the derived member is generic (because the derived
// class is generic).
if (auto derivedInterfaceFnTy = derivedInterfaceTy->getAs<GenericFunctionType>()) {
auto overrideInterfaceFnTy = overrideInterfaceTy->castTo<AnyFunctionType>();
overrideInterfaceTy =
Expand All @@ -2054,23 +2058,6 @@ SILConstantInfo TypeConverter::getConstantOverrideInfo(SILDeclRef derived,
overrideInterfaceFnTy->getExtInfo());
}

// Replace occurrences of 'Self' in the signature with the derived type.
// FIXME: these should all be modeled with a DynamicSelfType.
overrideInterfaceTy =
overrideInterfaceTy->replaceSelfParameterType(selfInterfaceTy);

bool hasDynamicSelf = false;
if (auto funcDecl = dyn_cast<FuncDecl>(derived.getDecl()))
hasDynamicSelf = funcDecl->hasDynamicSelf();
else if (isa<ConstructorDecl>(derived.getDecl()))
hasDynamicSelf = true;

if (hasDynamicSelf) {
overrideInterfaceTy =
overrideInterfaceTy->replaceCovariantResultType(selfInterfaceTy,
base.uncurryLevel + 1);
}

// Lower the formal AST type.
auto overrideLoweredInterfaceTy = getLoweredASTFunctionType(
cast<AnyFunctionType>(overrideInterfaceTy->getCanonicalType()),
Expand Down
40 changes: 7 additions & 33 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5014,32 +5014,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {

void visitModuleDecl(ModuleDecl *) { }

/// Adjust the type of the given declaration to appear as if it were
/// in the given subclass of its actual declared class.
static Type adjustSuperclassMemberDeclType(TypeChecker &TC,
const ValueDecl *decl,
Type subclass) {
ClassDecl *superclassDecl =
decl->getDeclContext()->getDeclaredTypeInContext()
->getClassOrBoundGenericClass();
auto superclass = subclass;
while (superclass->getClassOrBoundGenericClass() != superclassDecl)
superclass = TC.getSuperClassOf(superclass);
auto type = TC.substMemberTypeWithBase(decl->getModuleContext(), decl,
superclass,
/*isTypeReference=*/false);
if (auto func = dyn_cast<FuncDecl>(decl)) {
if (func->hasDynamicSelf()) {
type = type->replaceCovariantResultType(subclass,
func->getNumParameterLists());
}
} else if (isa<ConstructorDecl>(decl)) {
type = type->replaceCovariantResultType(subclass, /*uncurryLevel=*/2);
}

return type;
}

/// Perform basic checking to determine whether a declaration can override a
/// declaration in a superclass.
static bool areOverrideCompatibleSimple(ValueDecl *decl,
Expand Down Expand Up @@ -5093,15 +5067,15 @@ class DeclChecker : public DeclVisitor<DeclChecker> {

static bool
diagnoseMismatchedOptionals(TypeChecker &TC,
const Decl *member,
const ValueDecl *member,
const ParameterList *params,
TypeLoc resultTL,
const ValueDecl *parentMember,
Type owningTy,
bool treatIUOResultAsError) {
bool emittedError = false;
Type plainParentTy = adjustSuperclassMemberDeclType(TC, parentMember,
owningTy);
Type plainParentTy = owningTy->adjustSuperclassMemberDeclType(
member, parentMember, parentMember->getInterfaceType(), &TC);
const auto *parentTy = plainParentTy->castTo<AnyFunctionType>();
if (isa<AbstractFunctionDecl>(parentMember))
parentTy = parentTy->getResult()->castTo<AnyFunctionType>();
Expand Down Expand Up @@ -5538,8 +5512,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {

// Check whether the types are identical.
// FIXME: It's wrong to use the uncurried types here for methods.
auto parentDeclTy = adjustSuperclassMemberDeclType(TC, parentDecl,
owningTy);
auto parentDeclTy = owningTy->adjustSuperclassMemberDeclType(
decl, parentDecl, parentDecl->getInterfaceType(), &TC);
parentDeclTy = parentDeclTy->getUnlabeledType(TC.Context);
if (method) {
parentDeclTy = parentDeclTy->castTo<AnyFunctionType>()->getResult();
Expand Down Expand Up @@ -5807,8 +5781,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
}
} else if (auto property = dyn_cast_or_null<VarDecl>(abstractStorage)) {
auto propertyTy = property->getInterfaceType();
auto parentPropertyTy = adjustSuperclassMemberDeclType(TC, matchDecl,
superclass);
auto parentPropertyTy = superclass->adjustSuperclassMemberDeclType(
decl, matchDecl, matchDecl->getInterfaceType(), &TC);

if (!propertyTy->canOverride(parentPropertyTy,
OverrideMatchMode::Strict,
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckGeneric.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ Type CompleteGenericTypeResolver::resolveDependentMemberType(
// concrete type, or resolve its components down to another dependent member.
if (auto alias = nestedPA->getTypeAliasDecl()) {
return TC.substMemberTypeWithBase(DC->getParentModule(), alias,
baseTy, true);
baseTy);
}

Identifier name = ref->getIdentifier();
Expand Down
3 changes: 1 addition & 2 deletions lib/Sema/TypeCheckNameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,7 @@ LookupTypeResult TypeChecker::lookupMemberType(DeclContext *dc,

// Substitute the base into the member's type.
auto memberType = substMemberTypeWithBase(dc->getParentModule(),
typeDecl, type,
/*isTypeReference=*/true);
typeDecl, type);

// FIXME: It is not clear why this substitution can fail, but the
// standard library won't build without this check.
Expand Down
34 changes: 15 additions & 19 deletions lib/Sema/TypeCheckType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ Type TypeChecker::resolveTypeInContext(
}

return substMemberTypeWithBase(parentDC->getParentModule(), typeDecl,
fromType, /*isTypeReference=*/true);
fromType);
}
}

Expand Down Expand Up @@ -378,7 +378,7 @@ Type TypeChecker::resolveTypeInContext(
}

return substMemberTypeWithBase(parentDC->getParentModule(), typeDecl,
fromType, /*isTypeReference=*/true);
fromType);
}

if (auto superclassTy = getSuperClassOf(fromType))
Expand Down Expand Up @@ -1142,8 +1142,7 @@ static Type resolveNestedIdentTypeComponent(

// Otherwise, simply substitute the parent type into the member.
memberType = TC.substMemberTypeWithBase(DC->getParentModule(), typeDecl,
parentTy,
/*isTypeReference=*/true);
parentTy);

// Propagate failure.
if (!memberType || memberType->hasError()) return memberType;
Expand Down Expand Up @@ -2797,21 +2796,18 @@ Type TypeResolver::buildProtocolType(
}

Type TypeChecker::substMemberTypeWithBase(Module *module,
const ValueDecl *member,
Type baseTy, bool isTypeReference) {
Type memberType = isTypeReference
? cast<TypeDecl>(member)->getDeclaredInterfaceType()
: member->getInterfaceType();
if (isTypeReference) {
// The declared interface type for a generic type will have the type
// arguments; strip them off.
if (auto nominalTypeDecl = dyn_cast<NominalTypeDecl>(member)) {
if (auto boundGenericTy = memberType->getAs<BoundGenericType>()) {
memberType = UnboundGenericType::get(
const_cast<NominalTypeDecl *>(nominalTypeDecl),
boundGenericTy->getParent(),
Context);
}
const TypeDecl *member,
Type baseTy) {
Type memberType = member->getDeclaredInterfaceType();

// The declared interface type for a generic type will have the type
// arguments; strip them off.
if (auto nominalTypeDecl = dyn_cast<NominalTypeDecl>(member)) {
if (auto boundGenericTy = memberType->getAs<BoundGenericType>()) {
memberType = UnboundGenericType::get(
const_cast<NominalTypeDecl *>(nominalTypeDecl),
boundGenericTy->getParent(),
Context);
}
}

Expand Down
9 changes: 3 additions & 6 deletions lib/Sema/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -855,17 +855,14 @@ class TypeChecker final : public LazyResolver {
bool isGenericSignature,
GenericTypeResolver *resolver);

/// \brief Substitute the given base type into the type of the given member,
/// producing the effective type that the member will have.
/// \brief Substitute the given base type into the type of the given nested type,
/// producing the effective type that the nested type will have.
///
/// \param module The module in which the substitution will be performed.
/// \param member The member whose type projection is being computed.
/// \param baseTy The base type that will be substituted for the 'Self' of the
/// member.
/// \param isTypeReference Whether this is a reference to a type declaration
/// within a type context.
Type substMemberTypeWithBase(Module *module, const ValueDecl *member,
Type baseTy, bool isTypeReference);
Type substMemberTypeWithBase(Module *module, const TypeDecl *member, Type baseTy);

/// \brief Retrieve the superclass type of the given type, or a null type if
/// the type has no supertype.
Expand Down
39 changes: 10 additions & 29 deletions stdlib/public/core/ContiguousArrayBuffer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,15 @@ internal var _emptyArrayStorage : _EmptyArrayStorage {
Builtin.addressof(&_swiftEmptyArrayStorage))
}

// FIXME(ABI)#141 : This whole class is a workaround for
// <rdar://problem/18560464> Can't override generic method in generic
// subclass. If it weren't for that bug, we'd override
// _withVerbatimBridgedUnsafeBuffer directly in
// _ContiguousArrayStorage<Element>.
// rdar://problem/19341002
class _ContiguousArrayStorage1 : _ContiguousArrayStorageBase {
// The class that implements the storage for a ContiguousArray<Element>
@_versioned
final class _ContiguousArrayStorage<Element> : _ContiguousArrayStorageBase {

deinit {
_elementPointer.deinitialize(count: countAndCapacity.count)
_fixLifetime(self)
}

#if _runtime(_ObjC)
/// If the `Element` is bridged verbatim, invoke `body` on an
/// `UnsafeBufferPointer` to the elements and return the result.
Expand All @@ -82,28 +84,7 @@ class _ContiguousArrayStorage1 : _ContiguousArrayStorageBase {

/// If `Element` is bridged verbatim, invoke `body` on an
/// `UnsafeBufferPointer` to the elements.
internal func _withVerbatimBridgedUnsafeBufferImpl(
_ body: (UnsafeBufferPointer<AnyObject>) throws -> Void
) rethrows {
_sanityCheckFailure(
"Must override _withVerbatimBridgedUnsafeBufferImpl in derived classes")
}
#endif
}

// The class that implements the storage for a ContiguousArray<Element>
@_versioned
final class _ContiguousArrayStorage<Element> : _ContiguousArrayStorage1 {

deinit {
_elementPointer.deinitialize(count: countAndCapacity.count)
_fixLifetime(self)
}

#if _runtime(_ObjC)
/// If `Element` is bridged verbatim, invoke `body` on an
/// `UnsafeBufferPointer` to the elements.
internal final override func _withVerbatimBridgedUnsafeBufferImpl(
internal final func _withVerbatimBridgedUnsafeBufferImpl(
_ body: (UnsafeBufferPointer<AnyObject>) throws -> Void
) rethrows {
if _isBridgedVerbatimToObjectiveC(Element.self) {
Expand Down
1 change: 1 addition & 0 deletions stdlib/public/core/Unicode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import SwiftShims
/// of a decoding error.
///
/// - SeeAlso: `UnicodeCodec.decode(next:)`
@_fixed_layout
public enum UnicodeDecodingResult : Equatable {
/// A decoded Unicode scalar value.
case scalarValue(UnicodeScalar)
Expand Down
30 changes: 30 additions & 0 deletions test/SILGen/vtable_thunks_reabstraction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,33 @@ class ConcreteOptional<X>: Opaque<S?> {
override func variantOptionality(x: S??) -> S? { return x! }
}
*/

// Make sure we remap the method's innermost generic parameters
// to the correct depth
class GenericBase<T> {
func doStuff<U>(t: T, u: U) {}
init<U>(t: T, u: U) {}
}

class ConcreteSub : GenericBase<Int> {
override func doStuff<U>(t: Int, u: U) {
super.doStuff(t: t, u: u)
}
override init<U>(t: Int, u: U) {
super.init(t: t, u: u)
}
}

class ConcreteBase {
init<U>(t: Int, u: U) {}
func doStuff<U>(t: Int, u: U) {}
}

class GenericSub<T> : ConcreteBase {
override init<U>(t: Int, u: U) {
super.init(t: t, u: u)
}
override func doStuff<U>(t: Int, u: U) {
super.doStuff(t: t, u: u)
}
}
Loading