From f989c0afcfd564b547c20625ebc2310bc3ba6ae9 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 29 Jun 2018 17:06:34 -0700 Subject: [PATCH 1/8] [Evaluator] Strengthen check that inherited types don't get evaluated late. --- lib/Sema/TypeCheckRequestFunctions.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckRequestFunctions.cpp b/lib/Sema/TypeCheckRequestFunctions.cpp index 699a560e87538..f973d65293371 100644 --- a/lib/Sema/TypeCheckRequestFunctions.cpp +++ b/lib/Sema/TypeCheckRequestFunctions.cpp @@ -71,9 +71,8 @@ Type InheritedTypeRequest::evaluate( resolver = &archetypeResolver; } - // FIXME: Hack for calls through here when we have no type checker. auto lazyResolver = dc->getASTContext().getLazyResolver(); - if (!lazyResolver) return ErrorType::get(dc->getASTContext()); + assert(lazyResolver && "Cannot resolve inherited type at this point"); TypeChecker &tc = *static_cast(lazyResolver); TypeLoc &typeLoc = getTypeLoc(decl, index); From 950ea58159f15c71b33844ada4bb61585f5b1a52 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 3 Jul 2018 10:02:35 -0700 Subject: [PATCH 2/8] [Type checker] Factor out computation of overridden asssociated types. This is a separable path; give it its own path and isolate the state changes to TypeChecker::resolveOverriddenDecl(). NFC. --- lib/Sema/TypeCheckDeclOverride.cpp | 78 ++++++++++++++++-------------- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index c8c5ca907aa14..828b9cd076803 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -1534,6 +1534,47 @@ static int compareSimilarAssociatedTypes(ValueDecl *const *lhs, return TypeDecl::compare(lhsProto, rhsProto); } +/// Compute the set of associated types that are overridden by the given +/// associated type. +static SmallVector +computeOverriddenAssociatedTypes(AssociatedTypeDecl *assocType) { + // Find associated types with the given name in all of the inherited + // protocols. + SmallVector overriddenAssocTypes; + auto proto = assocType->getProtocol(); + proto->walkInheritedProtocols([&](ProtocolDecl *inheritedProto) { + if (proto == inheritedProto) return TypeWalker::Action::Continue; + + // Objective-C protocols + if (inheritedProto->isObjC()) return TypeWalker::Action::Continue; + + // Look for associated types with the same name. + bool foundAny = false; + for (auto member : inheritedProto->lookupDirect( + assocType->getFullName(), + /*ignoreNewExtensions=*/true)) { + if (auto assocType = dyn_cast(member)) { + overriddenAssocTypes.push_back(assocType); + foundAny = true; + } + } + + return foundAny ? TypeWalker::Action::SkipChildren + : TypeWalker::Action::Continue; + }); + + // Minimize the set of inherited associated types, eliminating any that + // themselves are overridden. + minimizeOverriddenAssociatedTypes(overriddenAssocTypes); + + // Sort the set of inherited associated types. + llvm::array_pod_sort(overriddenAssocTypes.begin(), + overriddenAssocTypes.end(), + compareSimilarAssociatedTypes); + + return overriddenAssocTypes; +} + void TypeChecker::resolveOverriddenDecl(ValueDecl *VD) { // If this function or something it calls didn't set any overridden // declarations, it means that there are no overridden declarations. Set @@ -1553,41 +1594,8 @@ void TypeChecker::resolveOverriddenDecl(ValueDecl *VD) { // FIXME: The request-evaluator will eventually handle this for us. (void)assocType->setOverriddenDecls({ }); - // Find associated types with the given name in all of the inherited - // protocols. - SmallVector inheritedAssociatedTypes; - auto proto = assocType->getProtocol(); - proto->walkInheritedProtocols([&](ProtocolDecl *inheritedProto) { - if (proto == inheritedProto) return TypeWalker::Action::Continue; - - // Objective-C protocols - if (inheritedProto->isObjC()) return TypeWalker::Action::Continue; - - // Look for associated types with the same name. - bool foundAny = false; - for (auto member : inheritedProto->lookupDirect( - assocType->getFullName(), - /*ignoreNewExtensions=*/true)) { - if (auto assocType = dyn_cast(member)) { - inheritedAssociatedTypes.push_back(assocType); - foundAny = true; - } - } - - return foundAny ? TypeWalker::Action::SkipChildren - : TypeWalker::Action::Continue; - }); - - // Minimize the set of inherited associated types, eliminating any that - // themselves are overridden. - minimizeOverriddenAssociatedTypes(inheritedAssociatedTypes); - - // Sort the set of inherited associated types. - llvm::array_pod_sort(inheritedAssociatedTypes.begin(), - inheritedAssociatedTypes.end(), - compareSimilarAssociatedTypes); - - (void)assocType->setOverriddenDecls(inheritedAssociatedTypes); + auto overriddenAssocTypes = computeOverriddenAssociatedTypes(assocType); + (void)assocType->setOverriddenDecls(overriddenAssocTypes); return; } From d0a12c171e15f0c69092762f4b02e937dc23fb02 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 3 Jul 2018 15:16:24 -0700 Subject: [PATCH 3/8] [Type checker] Refactor override checking. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Break the gigantic swift::checkOverrides() into a few separable pieces encapsulated in a new class, OverrideMatcher: * Setup: establish whether we can do matching at all * Matching: check whether we have a match based on a set of checking rules * Checking: check that the selected override is semantically correct * Search: apply various matching rules to try to find the overridden declaration While here, limit the dependencies of override checking somewhat. Specifically, don’t ask for the type of the declaration until we’ve found a potential overridden declaration to check against, and even then only do it when the “simple” checks fail. --- lib/Sema/TypeCheckDeclOverride.cpp | 634 +++++++++++++++++------------ 1 file changed, 368 insertions(+), 266 deletions(-) diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index 828b9cd076803..d9164491dc483 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -150,6 +150,20 @@ static bool areOverrideCompatibleSimple(ValueDecl *decl, parentDecl->getFullName().getArgumentNames().size()) return false; + // If the parent declaration is not in a class (or extension thereof), we + // cannot override it. + if (!parentDecl->getDeclContext()->getAsClassOrClassExtensionContext()) + return false; + + // The declarations must be of the same kind. + if (decl->getKind() != parentDecl->getKind()) + return false; + + // Ignore invalid parent declarations. + // FIXME: Do we really need this? + if (parentDecl->isInvalid()) + return false; + if (auto func = dyn_cast(decl)) { // Specific checking for methods. auto parentFunc = cast(parentDecl); @@ -161,6 +175,11 @@ static bool areOverrideCompatibleSimple(ValueDecl *decl, auto parentCtor = cast(parentDecl); if (ctor->isGeneric() != parentCtor->isGeneric()) return false; + + // Factory initializers cannot be overridden. + if (parentCtor->isFactoryInit()) + return false; + } else if (auto var = dyn_cast(decl)) { auto parentVar = cast(parentDecl); if (var->isStatic() != parentVar->isStatic()) @@ -523,263 +542,271 @@ static bool parameterTypesMatch(const ValueDecl *derivedDecl, return true; } -/// Determine which method or subscript this method or subscript overrides -/// (if any). -/// -/// \returns true if an error occurred. -bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { - if (decl->isInvalid() || decl->getOverriddenDecl()) - return false; - - auto *dc = decl->getDeclContext(); - - auto owningTy = dc->getDeclaredInterfaceType(); - if (!owningTy) - return false; +namespace { + /// Class that handles the checking of a particular declaration against + /// superclass entities that it could override. + class OverrideMatcher { + TypeChecker &tc; + ASTContext &ctx; + ValueDecl *decl; - auto classDecl = owningTy->getClassOrBoundGenericClass(); - if (!classDecl) - return false; + /// The superclass in which we'll look. + Type superclass; - Type superclass = classDecl->getSuperclass(); - if (!superclass) - return false; + // Cached member lookup results. + LookupResult members; - // Ignore accessor methods (e.g. getters and setters), they will be handled - // when their storage decl is processed. - if (isa(decl)) - return false; + /// The lookup name used to find \c members. + DeclName membersName; - auto method = dyn_cast(decl); - ConstructorDecl *ctor = nullptr; - if (method) - ctor = dyn_cast(method); + /// The type of the declaration, cached here once it has been computed. + Type cachedDeclType; - auto abstractStorage = dyn_cast(decl); - assert((method || abstractStorage) && "Not a method or abstractStorage?"); - SubscriptDecl *subscript = nullptr; - if (abstractStorage) - subscript = dyn_cast(abstractStorage); + public: + OverrideMatcher(TypeChecker &tc, ValueDecl *decl); - // Figure out the type of the declaration that we're using for comparisons. - auto declTy = getMemberTypeForComparison(TC.Context, decl); + /// Returns true when it's possible to perform any override matching. + explicit operator bool() const { + return static_cast(superclass); + } - // Look for members with the same name and matching types as this - // one. - auto attempt = OverrideCheckingAttempt::PerfectMatch; - SmallVector matches; - DeclName name = decl->getFullName(); - bool hadExactMatch = false; - LookupResult members; + /// Match this declaration against potential members in the superclass, + /// using the heuristics appropriate for the given \c attempt. + SmallVector match(OverrideCheckingAttempt attempt); + + /// We have determined that we have an override of the given \c baseDecl. + /// + /// Check that the override itself is valid. + bool checkOverride(ValueDecl *baseDecl, + OverrideCheckingAttempt attempt); + + private: + /// Retrieve the type of the declaration, to be used in comparisons. + Type getDeclComparisonType() { + if (!cachedDeclType) { + cachedDeclType = getMemberTypeForComparison(ctx, decl); + } - do { - switch (attempt) { - case OverrideCheckingAttempt::PerfectMatch: - break; - case OverrideCheckingAttempt::MismatchedOptional: - // Don't keep looking if the user didn't indicate it's an override. - if (!decl->getAttrs().hasAttribute()) - return false; - break; - case OverrideCheckingAttempt::MismatchedTypes: - break; - case OverrideCheckingAttempt::BaseName: - // Don't keep looking if this is already a simple name, or if there - // are no arguments. - if (name.isSimpleName() || name.getArgumentNames().empty()) - return false; - name = name.getBaseName(); - members.clear(); - break; - case OverrideCheckingAttempt::BaseNameWithMismatchedOptional: - break; - case OverrideCheckingAttempt::Final: - // Give up. - return false; + return cachedDeclType; } + }; +} - if (members.empty()) { - auto lookupOptions = defaultMemberLookupOptions; +OverrideMatcher::OverrideMatcher(TypeChecker &tc, ValueDecl *decl) + : tc(tc), ctx(decl->getASTContext()), decl(decl) { + // The final step for this constructor is to set up the superclass type, + // without which we will not perform an matching. Early exits therefore imply + // that there is no way we can match this declaration. + if (decl->isInvalid()) + return; - // Class methods cannot override declarations only - // visible via dynamic dispatch. - lookupOptions -= NameLookupFlags::DynamicLookup; + auto *dc = decl->getDeclContext(); - // Class methods cannot override declarations only - // visible as protocol requirements or protocol - // extension members. - lookupOptions -= NameLookupFlags::ProtocolMembers; - lookupOptions -= NameLookupFlags::PerformConformanceCheck; + auto owningTy = dc->getDeclaredInterfaceType(); + if (!owningTy) + return; - members = TC.lookupMember(dc, superclass, - name, lookupOptions); - } + auto classDecl = owningTy->getClassOrBoundGenericClass(); + if (!classDecl) + return; - for (auto memberResult : members) { - auto member = memberResult.getValueDecl(); + // FIXME: Get the superclass from owningTy directly? + superclass = classDecl->getSuperclass(); +} - if (member->isInvalid()) - continue; +SmallVector OverrideMatcher::match( + OverrideCheckingAttempt attempt) { + // If there's no matching we can do, fail. + if (!*this) return { }; - if (member->getKind() != decl->getKind()) - continue; + auto dc = decl->getDeclContext(); - if (!dc->getAsClassOrClassExtensionContext()) - continue; + // Determine what name we should look for. + DeclName name; + switch (attempt) { + case OverrideCheckingAttempt::PerfectMatch: + case OverrideCheckingAttempt::MismatchedOptional: + case OverrideCheckingAttempt::MismatchedTypes: + name = decl->getFullName(); + break; + case OverrideCheckingAttempt::BaseName: + case OverrideCheckingAttempt::BaseNameWithMismatchedOptional: + name = decl->getBaseName(); + break; + case OverrideCheckingAttempt::Final: + // Give up. + return { }; + } - auto parentDecl = cast(member); + // If we don't have members available yet, or we looked them up based on a + // different name, look them up now. + if (members.empty() || name != membersName) { + auto lookupOptions = defaultMemberLookupOptions; - // Check whether there are any obvious reasons why the two given - // declarations do not have an overriding relationship. - if (!areOverrideCompatibleSimple(decl, parentDecl)) - continue; + // Class methods cannot override declarations only + // visible via dynamic dispatch. + lookupOptions -= NameLookupFlags::DynamicLookup; - auto parentMethod = dyn_cast(parentDecl); - auto parentStorage = dyn_cast(parentDecl); - assert(parentMethod || parentStorage); - - // If both are Objective-C, then match based on selectors or - // subscript kind and check the types separately. - bool objCMatch = false; - if (parentDecl->isObjC() && decl->isObjC()) { - if (method) { - if (method->getObjCSelector() == parentMethod->getObjCSelector()) - objCMatch = true; - } else if (auto *parentSubscript = - dyn_cast(parentStorage)) { - // If the subscript kinds don't match, it's not an override. - if (subscript->getObjCSubscriptKind() - == parentSubscript->getObjCSubscriptKind()) - objCMatch = true; - } + // Class methods cannot override declarations only + // visible as protocol requirements or protocol + // extension members. + lookupOptions -= NameLookupFlags::ProtocolMembers; + lookupOptions -= NameLookupFlags::PerformConformanceCheck; - // Properties don't need anything here since they are always - // checked by name. - } + membersName = name; + members = tc.lookupMember(dc, superclass, membersName, lookupOptions); + } - if (ctor) { - // Factory methods cannot be overridden. - auto parentCtor = cast(parentDecl); - if (parentCtor->isFactoryInit()) - continue; - } + auto method = dyn_cast(decl); + SubscriptDecl *subscript = dyn_cast(decl); - // Check whether the types are identical. - auto parentDeclTy = - getMemberTypeForComparison(TC.Context, parentDecl, decl); - if (parentDeclTy->hasError()) - continue; + // Check each member we found. + SmallVector matches; + for (auto memberResult : members) { + auto parentDecl = memberResult.getValueDecl(); - if (isOverrideBasedOnType(decl, declTy, parentDecl, parentDeclTy)) { - matches.push_back({parentDecl, true, parentDeclTy}); - hadExactMatch = true; - continue; - } + // Check whether there are any obvious reasons why the two given + // declarations do not have an overriding relationship. + if (!areOverrideCompatibleSimple(decl, parentDecl)) + continue; - // If this is a property, we accept the match and then reject it below - // if the types don't line up, since you can't overload properties based - // on types. - if (isa(parentDecl) || - attempt == OverrideCheckingAttempt::MismatchedTypes) { - matches.push_back({parentDecl, false, parentDeclTy}); - continue; + auto parentMethod = dyn_cast(parentDecl); + auto parentStorage = dyn_cast(parentDecl); + assert(parentMethod || parentStorage); + + // If both are Objective-C, then match based on selectors or + // subscript kind and check the types separately. + // FIXME: This is wrong. We should be matching based on type information, + // then checking Objective-C selectors much later. + bool objCMatch = false; + if (parentDecl->isObjC() && decl->isObjC()) { + if (method) { + if (method->getObjCSelector() == parentMethod->getObjCSelector()) + objCMatch = true; + } else if (auto *parentSubscript = + dyn_cast(parentStorage)) { + // If the subscript kinds don't match, it's not an override. + if (subscript->getObjCSubscriptKind() + == parentSubscript->getObjCSubscriptKind()) + objCMatch = true; } - // Failing that, check for subtyping. - TypeMatchOptions matchMode = TypeMatchFlags::AllowOverride; - if (attempt == OverrideCheckingAttempt::MismatchedOptional || - attempt == OverrideCheckingAttempt::BaseNameWithMismatchedOptional){ - matchMode |= TypeMatchFlags::AllowTopLevelOptionalMismatch; - } else if (parentDecl->isObjC()) { - matchMode |= TypeMatchFlags::AllowNonOptionalForIUOParam; - matchMode |= - TypeMatchFlags::IgnoreNonEscapingForOptionalFunctionParam; - } + // Properties don't need anything here since they are always + // checked by name. + } - auto declFnTy = declTy->getAs(); - auto parentDeclFnTy = parentDeclTy->getAs(); - if (declFnTy && parentDeclFnTy) { - auto paramsAndResultMatch = [=]() -> bool { - return parameterTypesMatch(decl, parentDecl, matchMode) && - declFnTy->getResult()->matches(parentDeclFnTy->getResult(), - matchMode); - }; - - if (declFnTy->matchesFunctionType(parentDeclFnTy, matchMode, - paramsAndResultMatch)) { - matches.push_back({parentDecl, objCMatch, parentDeclTy}); - hadExactMatch |= objCMatch; - continue; - } - } else if (declTy->matches(parentDeclTy, matchMode)) { - matches.push_back({parentDecl, objCMatch, parentDeclTy}); - hadExactMatch |= objCMatch; - continue; - } + // Check whether the types are identical. + auto parentDeclTy = getMemberTypeForComparison(ctx, parentDecl, decl); + if (parentDeclTy->hasError()) + continue; - // Not a match. If we had an Objective-C match, this is a serious - // problem. - if (objCMatch) { - if (method) { - TC.diagnose(decl, diag::override_objc_type_mismatch_method, - method->getObjCSelector(), declTy); - } else { - TC.diagnose(decl, diag::override_objc_type_mismatch_subscript, - static_cast( - subscript->getObjCSubscriptKind()), - declTy); - } - TC.diagnose(parentDecl, diag::overridden_here_with_type, - parentDeclTy); + Type declTy = getDeclComparisonType(); + if (isOverrideBasedOnType(decl, declTy, parentDecl, parentDeclTy)) { + matches.push_back({parentDecl, true, parentDeclTy}); + continue; + } + + // If this is a property, we accept the match and then reject it below + // if the types don't line up, since you can't overload properties based + // on types. + if (isa(parentDecl) || + attempt == OverrideCheckingAttempt::MismatchedTypes) { + matches.push_back({parentDecl, false, parentDeclTy}); + continue; + } - // Put an invalid 'override' attribute here. - makeInvalidOverrideAttr(decl); + // Failing that, check for subtyping. + TypeMatchOptions matchMode = TypeMatchFlags::AllowOverride; + if (attempt == OverrideCheckingAttempt::MismatchedOptional || + attempt == OverrideCheckingAttempt::BaseNameWithMismatchedOptional){ + matchMode |= TypeMatchFlags::AllowTopLevelOptionalMismatch; + } else if (parentDecl->isObjC()) { + matchMode |= TypeMatchFlags::AllowNonOptionalForIUOParam; + matchMode |= TypeMatchFlags::IgnoreNonEscapingForOptionalFunctionParam; + } - return true; + auto declFnTy = getDeclComparisonType()->getAs(); + auto parentDeclFnTy = parentDeclTy->getAs(); + if (declFnTy && parentDeclFnTy) { + auto paramsAndResultMatch = [=]() -> bool { + return parameterTypesMatch(decl, parentDecl, matchMode) && + declFnTy->getResult()->matches(parentDeclFnTy->getResult(), + matchMode); + }; + + if (declFnTy->matchesFunctionType(parentDeclFnTy, matchMode, + paramsAndResultMatch)) { + matches.push_back({parentDecl, objCMatch, parentDeclTy}); + continue; } + } else if (getDeclComparisonType()->matches(parentDeclTy, matchMode)) { + matches.push_back({parentDecl, objCMatch, parentDeclTy}); + continue; } - if (!matches.empty()) - break; - ++attempt; - } while (true); + // Not a match. If we had an Objective-C match, this is a serious + // problem. + if (objCMatch) { + if (method) { + ctx.Diags.diagnose(decl, diag::override_objc_type_mismatch_method, + method->getObjCSelector(), declTy); + } else { + ctx.Diags.diagnose(decl, diag::override_objc_type_mismatch_subscript, + static_cast( + subscript->getObjCSubscriptKind()), + getDeclComparisonType()); + } + ctx.Diags.diagnose(parentDecl, diag::overridden_here_with_type, + parentDeclTy); - assert(!matches.empty()); + // Put an invalid 'override' attribute here. + makeInvalidOverrideAttr(decl); - // If we had an exact match, throw away any non-exact matches. - if (hadExactMatch) - matches.erase(std::remove_if(matches.begin(), matches.end(), - [&](OverrideMatch &match) { - return !match.IsExact; - }), matches.end()); + return { }; + } + } - // If we override more than one declaration, complain. + // If we have more than one match, and any of them was exact, remove all + // non-exact matches. if (matches.size() > 1) { - diagnoseGeneralOverrideFailure(decl, matches, attempt); - return true; + bool hadExactMatch = std::find_if(matches.begin(), matches.end(), + [](const OverrideMatch &match) { + return match.IsExact; + }) != matches.end(); + if (hadExactMatch) { + matches.erase(std::remove_if(matches.begin(), matches.end(), + [&](const OverrideMatch &match) { + return !match.IsExact; + }), + matches.end()); + } } - // If we have a single match (exact or not), take it. - auto matchDecl = matches.front().Decl; - auto matchType = matches.front().SubstType; + return matches; +} + +bool OverrideMatcher::checkOverride(ValueDecl *baseDecl, + OverrideCheckingAttempt attempt) { + auto &diags = ctx.Diags; + auto baseTy = getMemberTypeForComparison(ctx, baseDecl, decl); bool emittedMatchError = false; // If the name of our match differs from the name we were looking for, // complain. - if (decl->getFullName() != matchDecl->getFullName()) { - auto diag = TC.diagnose(decl, diag::override_argument_name_mismatch, - isa(decl), - decl->getFullName(), - matchDecl->getFullName()); + if (decl->getFullName() != baseDecl->getFullName()) { + auto diag = diags.diagnose(decl, diag::override_argument_name_mismatch, + isa(decl), + decl->getFullName(), + baseDecl->getFullName()); fixDeclarationName(diag, cast(decl), - matchDecl->getFullName()); + baseDecl->getFullName()); emittedMatchError = true; } // If we have an explicit ownership modifier and our parent doesn't, // complain. auto parentAttr = - matchDecl->getAttrs().getAttribute(); + baseDecl->getAttrs().getAttribute(); if (auto ownershipAttr = decl->getAttrs().getAttribute()) { ReferenceOwnership parentOwnership; @@ -788,9 +815,9 @@ bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { else parentOwnership = ReferenceOwnership::Strong; if (parentOwnership != ownershipAttr->get()) { - TC.diagnose(decl, diag::override_ownership_mismatch, - parentOwnership, ownershipAttr->get()); - TC.diagnose(matchDecl, diag::overridden_here); + diags.diagnose(decl, diag::override_ownership_mismatch, + parentOwnership, ownershipAttr->get()); + diags.diagnose(baseDecl, diag::overridden_here); } } @@ -799,12 +826,14 @@ bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { // This case gets this far because the type matching above specifically // strips out dynamic self via replaceCovariantResultType(), and that // is helpful in several cases - just not this one. + auto dc = decl->getDeclContext(); + auto classDecl = dc->getAsClassOrClassExtensionContext(); if (decl->getASTContext().isSwiftVersionAtLeast(5) && - matchDecl->getInterfaceType()->hasDynamicSelfType() && + baseDecl->getInterfaceType()->hasDynamicSelfType() && !decl->getInterfaceType()->hasDynamicSelfType() && !classDecl->isFinal()) { - TC.diagnose(decl, diag::override_dynamic_self_mismatch); - TC.diagnose(matchDecl, diag::overridden_here); + diags.diagnose(decl, diag::override_dynamic_self_mismatch); + diags.diagnose(baseDecl, diag::overridden_here); } // Check that the override has the required access level. @@ -816,12 +845,12 @@ bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { // defining module. This is not required for constructors, which are // never really "overridden" in the intended sense here, because of // course derived classes will change how the class is initialized. - AccessLevel matchAccess = matchDecl->getFormalAccess(dc); + AccessLevel matchAccess = baseDecl->getFormalAccess(dc); if (matchAccess < AccessLevel::Open && - matchDecl->getModuleContext() != decl->getModuleContext() && + baseDecl->getModuleContext() != decl->getModuleContext() && !isa(decl)) { - TC.diagnose(decl, diag::override_of_non_open, - decl->getDescriptiveKind()); + diags.diagnose(decl, diag::override_of_non_open, + decl->getDescriptiveKind()); } else if (matchAccess == AccessLevel::Open && classDecl->getFormalAccess(dc) == @@ -829,17 +858,17 @@ bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { decl->getFormalAccess() != AccessLevel::Open && !decl->isFinal()) { { - auto diag = TC.diagnose(decl, diag::override_not_accessible, - /*setter*/false, - decl->getDescriptiveKind(), - /*fromOverridden*/true); + auto diag = diags.diagnose(decl, diag::override_not_accessible, + /*setter*/false, + decl->getDescriptiveKind(), + /*fromOverridden*/true); fixItAccess(diag, decl, AccessLevel::Open); } - TC.diagnose(matchDecl, diag::overridden_here); + diags.diagnose(baseDecl, diag::overridden_here); } else if (!isa(decl)) { auto matchAccessScope = - matchDecl->getFormalAccessScope(dc); + baseDecl->getFormalAccessScope(dc); auto classAccessScope = classDecl->getFormalAccessScope(dc); auto requiredAccessScope = @@ -849,8 +878,8 @@ bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { bool shouldDiagnose = !decl->isAccessibleFrom(scopeDC); bool shouldDiagnoseSetter = false; - if (!shouldDiagnose && matchDecl->isSettable(dc)){ - auto matchASD = cast(matchDecl); + if (!shouldDiagnose && baseDecl->isSettable(dc)){ + auto matchASD = cast(baseDecl); if (matchASD->isSetterAccessibleFrom(dc)) { auto matchSetterAccessScope = matchASD->getSetter() ->getFormalAccessScope(dc); @@ -872,14 +901,13 @@ bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { AccessLevel requiredAccess = requiredAccessScope->requiredAccessForDiagnostics(); { - auto diag = TC.diagnose(decl, diag::override_not_accessible, - shouldDiagnoseSetter, - decl->getDescriptiveKind(), - overriddenForcesAccess); - fixItAccess(diag, decl, requiredAccess, - shouldDiagnoseSetter); + auto diag = diags.diagnose(decl, diag::override_not_accessible, + shouldDiagnoseSetter, + decl->getDescriptiveKind(), + overriddenForcesAccess); + fixItAccess(diag, decl, requiredAccess, shouldDiagnoseSetter); } - TC.diagnose(matchDecl, diag::overridden_here); + diags.diagnose(baseDecl, diag::overridden_here); } } @@ -890,73 +918,75 @@ bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { auto declIUOAttr = decl->getAttrs().hasAttribute(); auto matchDeclIUOAttr = - matchDecl->getAttrs().hasAttribute(); + baseDecl->getAttrs().hasAttribute(); // If this is an exact type match, we're successful! - if (declIUOAttr == matchDeclIUOAttr && declTy->isEqual(matchType)) { + Type declTy = getDeclComparisonType(); + Type owningTy = dc->getDeclaredInterfaceType(); + if (declIUOAttr == matchDeclIUOAttr && declTy->isEqual(baseTy)) { // Nothing to do. - } else if (method) { + } else if (auto method = dyn_cast(decl)) { if (attempt == OverrideCheckingAttempt::MismatchedTypes) { auto diagKind = diag::method_does_not_override; - if (ctor) + if (isa(method)) diagKind = diag::initializer_does_not_override; - TC.diagnose(decl, diagKind); - noteFixableMismatchedTypes(decl, matchDecl); - TC.diagnose(matchDecl, diag::overridden_near_match_here, - matchDecl->getDescriptiveKind(), - matchDecl->getFullName()); + diags.diagnose(decl, diagKind); + noteFixableMismatchedTypes(decl, baseDecl); + diags.diagnose(baseDecl, diag::overridden_near_match_here, + baseDecl->getDescriptiveKind(), + baseDecl->getFullName()); emittedMatchError = true; } else if (!isa(method) && - (matchDecl->isObjC() || mayHaveMismatchedOptionals)) { + (baseDecl->isObjC() || mayHaveMismatchedOptionals)) { // Private migration help for overrides of Objective-C methods. TypeLoc resultTL; if (auto *methodAsFunc = dyn_cast(method)) resultTL = methodAsFunc->getBodyResultTypeLoc(); + emittedMatchError |= diagnoseMismatchedOptionals( - method, method->getParameterList(1), resultTL, matchDecl, - cast(matchDecl)->getParameterList(1), + method, method->getParameterList(1), resultTL, baseDecl, + cast(baseDecl)->getParameterList(1), owningTy, mayHaveMismatchedOptionals); } - } else if (auto subscript = - dyn_cast_or_null(abstractStorage)) { + } else if (auto subscript = dyn_cast(decl)) { // Otherwise, if this is a subscript, validate that covariance is ok. // If the parent is non-mutable, it's okay to be covariant. - auto parentSubscript = cast(matchDecl); + auto parentSubscript = cast(baseDecl); if (parentSubscript->getSetter()) { - TC.diagnose(subscript, diag::override_mutable_covariant_subscript, - declTy, matchType); - TC.diagnose(matchDecl, diag::subscript_override_here); + diags.diagnose(subscript, diag::override_mutable_covariant_subscript, + declTy, baseTy); + diags.diagnose(baseDecl, diag::subscript_override_here); return true; } if (attempt == OverrideCheckingAttempt::MismatchedTypes) { - TC.diagnose(decl, diag::subscript_does_not_override); - noteFixableMismatchedTypes(decl, matchDecl); - TC.diagnose(matchDecl, diag::overridden_near_match_here, - matchDecl->getDescriptiveKind(), - matchDecl->getFullName()); + diags.diagnose(decl, diag::subscript_does_not_override); + noteFixableMismatchedTypes(decl, baseDecl); + diags.diagnose(baseDecl, diag::overridden_near_match_here, + baseDecl->getDescriptiveKind(), + baseDecl->getFullName()); emittedMatchError = true; } else if (mayHaveMismatchedOptionals) { emittedMatchError |= diagnoseMismatchedOptionals( subscript, subscript->getIndices(), - subscript->getElementTypeLoc(), matchDecl, - cast(matchDecl)->getIndices(), owningTy, + subscript->getElementTypeLoc(), baseDecl, + cast(baseDecl)->getIndices(), owningTy, mayHaveMismatchedOptionals); } - } else if (auto property = dyn_cast_or_null(abstractStorage)) { + } else if (auto property = dyn_cast(decl)) { auto propertyTy = property->getInterfaceType(); auto parentPropertyTy = superclass->adjustSuperclassMemberDeclType( - matchDecl, decl, matchDecl->getInterfaceType()); + baseDecl, decl, baseDecl->getInterfaceType()); if (!propertyTy->matches(parentPropertyTy, TypeMatchFlags::AllowOverride)) { - TC.diagnose(property, diag::override_property_type_mismatch, - property->getName(), propertyTy, parentPropertyTy); - noteFixableMismatchedTypes(decl, matchDecl); - TC.diagnose(matchDecl, diag::property_override_here); + diags.diagnose(property, diag::override_property_type_mismatch, + property->getName(), propertyTy, parentPropertyTy); + noteFixableMismatchedTypes(decl, baseDecl); + diags.diagnose(baseDecl, diag::property_override_here); return true; } @@ -969,11 +999,11 @@ bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { IsSilentDifference = true; // The overridden property must not be mutable. - if (cast(matchDecl)->getSetter() && + if (cast(baseDecl)->getSetter() && !IsSilentDifference) { - TC.diagnose(property, diag::override_mutable_covariant_property, + diags.diagnose(property, diag::override_mutable_covariant_property, property->getName(), parentPropertyTy, propertyTy); - TC.diagnose(matchDecl, diag::property_override_here); + diags.diagnose(baseDecl, diag::property_override_here); return true; } } @@ -981,10 +1011,82 @@ bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { // Catch-all to make sure we don't silently accept something we shouldn't. if (attempt != OverrideCheckingAttempt::PerfectMatch && !emittedMatchError) { + OverrideMatch match{decl, /*isExact=*/false, declTy}; + diagnoseGeneralOverrideFailure(decl, match, attempt); + } + + return recordOverride(tc, decl, baseDecl); +} + +/// Determine which method or subscript this method or subscript overrides +/// (if any). +/// +/// \returns true if an error occurred. +bool swift::checkOverrides(TypeChecker &TC, ValueDecl *decl) { + if (decl->getOverriddenDecl()) + return false; + + // Set up matching, but bail out if there's nothing to match. + OverrideMatcher matcher(TC, decl); + if (!matcher) return false; + + // Ignore accessor methods (e.g. getters and setters), they will be handled + // when their storage decl is processed. + // FIXME: We should pull information from the storage declaration, but + // that will be handled at a different point. + if (isa(decl)) + return false; + + // Look for members with the same name and matching types as this + // one. + SmallVector matches; + auto attempt = OverrideCheckingAttempt::PerfectMatch; + do { + // Determine whether we should attempt to perform matching now, or exit + // early with a failure. + switch (attempt) { + case OverrideCheckingAttempt::PerfectMatch: + break; + case OverrideCheckingAttempt::MismatchedOptional: + // Don't keep looking if the user didn't indicate it's an override. + if (!decl->getAttrs().hasAttribute()) + return false; + break; + case OverrideCheckingAttempt::MismatchedTypes: + break; + case OverrideCheckingAttempt::BaseName: + // Don't keep looking if this is already a simple name, or if there + // are no arguments. + if (decl->getFullName() == decl->getBaseName() || + decl->getFullName().getArgumentNames().empty()) + return false; + break; + case OverrideCheckingAttempt::BaseNameWithMismatchedOptional: + break; + case OverrideCheckingAttempt::Final: + // Give up. + return false; + } + + // Try to match with the + matches = matcher.match(attempt); + if (!matches.empty()) + break; + + // Try the next version. + ++attempt; + } while (true); + + assert(!matches.empty()); + + // If we override more than one declaration, complain. + if (matches.size() > 1) { diagnoseGeneralOverrideFailure(decl, matches, attempt); + return true; } - return recordOverride(TC, decl, matchDecl); + // If we have a single match (exact or not), take it. + return matcher.checkOverride(matches.front().Decl, attempt); } namespace { From bc78b198531968ede2530e890479fa31fba23e2e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 3 Jul 2018 15:42:20 -0700 Subject: [PATCH 4/8] =?UTF-8?q?[Type=20checker]=20Make=20=E2=80=9Coverride?= =?UTF-8?q?=E2=80=9D=20computation=20not=20dependent=20on=20Objective-C=20?= =?UTF-8?q?interop.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For reasons lost to history and not interesting enough to uncover, the “override” computation performed a check of the overriding declaration’s Objective-C selector against the potentially-overridden declaration’s Objective-C selector. This introduced a cycle dependency between the “is exposed to Objective-C” computation and the “overridden declarations” computation, because @objc is inherited from an overridden declaration. We’ll detect the same errors via other, existing diagnostics, so prune out the Objective-C selector checks from the override path. --- include/swift/AST/DiagnosticsSema.def | 9 ---- lib/Sema/TypeCheckDeclOverride.cpp | 69 +-------------------------- test/attr/attr_objc_override.swift | 8 ++-- 3 files changed, 6 insertions(+), 80 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index a461fae86dbd5..8e2a5b9dede9f 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1976,15 +1976,6 @@ NOTE(overridden_here,none, NOTE(overridden_here_can_be_objc,none, "add '@objc' to make this declaration overridable", ()) -ERROR(override_objc_type_mismatch_method,none, - "overriding method with selector %0 has incompatible type %1", - (ObjCSelector, Type)) -ERROR(override_objc_type_mismatch_subscript,none, - "overriding %select{|indexed |keyed }0subscript with incompatible type " - "%1", - (unsigned, Type)) -NOTE(overridden_here_with_type,none, - "overridden declaration here has type %0", (Type)) ERROR(missing_override,none, "overriding declaration requires an 'override' keyword", ()) diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index d9164491dc483..5f1b6c17713de 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -342,26 +342,6 @@ diagnoseMismatchedOptionals(const ValueDecl *member, return emittedError; } -/// Make sure that there is an invalid 'override' attribute on the -/// given declaration. -static void makeInvalidOverrideAttr(ValueDecl *decl) { - if (auto overrideAttr = decl->getAttrs().getAttribute()) { - overrideAttr->setInvalid(); - } else { - auto attr = new (decl->getASTContext()) OverrideAttr(true); - decl->getAttrs().add(attr); - attr->setInvalid(); - } - - // FIXME: What about the other accessors? - if (auto storage = dyn_cast(decl)) { - if (auto getter = storage->getGetter()) - makeInvalidOverrideAttr(getter); - if (auto setter = storage->getSetter()) - makeInvalidOverrideAttr(setter); - } -} - /// Record that the \c overriding declarations overrides the /// \c overridden declaration. /// @@ -657,9 +637,6 @@ SmallVector OverrideMatcher::match( members = tc.lookupMember(dc, superclass, membersName, lookupOptions); } - auto method = dyn_cast(decl); - SubscriptDecl *subscript = dyn_cast(decl); - // Check each member we found. SmallVector matches; for (auto memberResult : members) { @@ -674,27 +651,6 @@ SmallVector OverrideMatcher::match( auto parentStorage = dyn_cast(parentDecl); assert(parentMethod || parentStorage); - // If both are Objective-C, then match based on selectors or - // subscript kind and check the types separately. - // FIXME: This is wrong. We should be matching based on type information, - // then checking Objective-C selectors much later. - bool objCMatch = false; - if (parentDecl->isObjC() && decl->isObjC()) { - if (method) { - if (method->getObjCSelector() == parentMethod->getObjCSelector()) - objCMatch = true; - } else if (auto *parentSubscript = - dyn_cast(parentStorage)) { - // If the subscript kinds don't match, it's not an override. - if (subscript->getObjCSubscriptKind() - == parentSubscript->getObjCSubscriptKind()) - objCMatch = true; - } - - // Properties don't need anything here since they are always - // checked by name. - } - // Check whether the types are identical. auto parentDeclTy = getMemberTypeForComparison(ctx, parentDecl, decl); if (parentDeclTy->hasError()) @@ -736,34 +692,13 @@ SmallVector OverrideMatcher::match( if (declFnTy->matchesFunctionType(parentDeclFnTy, matchMode, paramsAndResultMatch)) { - matches.push_back({parentDecl, objCMatch, parentDeclTy}); + matches.push_back({parentDecl, false, parentDeclTy}); continue; } } else if (getDeclComparisonType()->matches(parentDeclTy, matchMode)) { - matches.push_back({parentDecl, objCMatch, parentDeclTy}); + matches.push_back({parentDecl, false, parentDeclTy}); continue; } - - // Not a match. If we had an Objective-C match, this is a serious - // problem. - if (objCMatch) { - if (method) { - ctx.Diags.diagnose(decl, diag::override_objc_type_mismatch_method, - method->getObjCSelector(), declTy); - } else { - ctx.Diags.diagnose(decl, diag::override_objc_type_mismatch_subscript, - static_cast( - subscript->getObjCSubscriptKind()), - getDeclComparisonType()); - } - ctx.Diags.diagnose(parentDecl, diag::overridden_here_with_type, - parentDeclTy); - - // Put an invalid 'override' attribute here. - makeInvalidOverrideAttr(decl); - - return { }; - } } // If we have more than one match, and any of them was exact, remove all diff --git a/test/attr/attr_objc_override.swift b/test/attr/attr_objc_override.swift index 65ac9065f5b68..b3cd53b1b0a73 100644 --- a/test/attr/attr_objc_override.swift +++ b/test/attr/attr_objc_override.swift @@ -11,8 +11,8 @@ class MixedKeywordsAndAttributes { // expected-note{{in declaration of 'MixedKey struct SwiftStruct { } class A { - @objc subscript (a: ObjCClass) -> ObjCClass { // expected-note{{overridden declaration here has type '(ObjCClass) -> ObjCClass'}} - get { return ObjCClass() } + @objc subscript (a: ObjCClass) -> ObjCClass { + get { return ObjCClass() } // expected-note{{subscript getter declared here}} } func bar() { } // expected-note{{add '@objc' to make this declaration overridable}}{{3-3=@objc }} @@ -25,8 +25,8 @@ extension A { class B : A { // Objective-C - @objc subscript (a: ObjCClass) -> AnyObject { // expected-error{{overriding keyed subscript with incompatible type '(ObjCClass) -> AnyObject'}} - get { return self } + @objc subscript (a: ObjCClass) -> AnyObject { + get { return self } // expected-error{{subscript getter with Objective-C selector 'objectForKeyedSubscript:' conflicts with subscript getter from superclass 'A'}} } override func foo() { } // expected-error{{overriding non-@objc declarations from extensions is not supported}} From d5bd991e527d80174661eedee4f2e736b3c253a8 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 3 Jul 2018 15:45:03 -0700 Subject: [PATCH 5/8] [AST] Prune unnecessary calls to LazyResolver::resolveOverriddenDecl(). ValueDecl::getOverriddenDecl(s)() does this for us automagically. --- lib/AST/NameLookup.cpp | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index d82c7c26706a7..888fb6a997a76 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -84,13 +84,8 @@ bool swift::removeOverriddenDecls(SmallVectorImpl &decls) { if (decls.size() < 2) return false; - auto lazyResolver = decls.front()->getASTContext().getLazyResolver(); llvm::SmallPtrSet overridden; for (auto decl : decls) { - // Compute enough information to make the overridden-declaration available. - if (lazyResolver) - lazyResolver->resolveOverriddenDecl(decl); - while (auto overrides = decl->getOverriddenDecl()) { overridden.insert(overrides); @@ -105,10 +100,6 @@ bool swift::removeOverriddenDecls(SmallVectorImpl &decls) { /// cause instead (incomplete circularity detection). assert(decl != overrides && "Circular class inheritance?"); decl = overrides; - - if (lazyResolver) - lazyResolver->resolveOverriddenDecl(decl); - continue; } @@ -1908,9 +1899,6 @@ bool DeclContext::lookupQualified(Type type, // If the declaration has an override, name lookup will also have // found the overridden method. Skip this declaration, because we // prefer the overridden method. - if (typeResolver) - typeResolver->resolveOverriddenDecl(decl); - if (decl->getOverriddenDecl()) continue; From bc0a18f12acecd7a8bf711c40eefed3232c66d68 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 3 Jul 2018 15:56:38 -0700 Subject: [PATCH 6/8] =?UTF-8?q?[Type=20Checker]=20Only=20resolveOverridden?= =?UTF-8?q?Decl()=20for=20decls=20with=20=E2=80=98override=E2=80=99.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveOverriddenDecl() is meant to be used to provide access to the overridden declarations, e.g., from another source file. Only resolve when the declaration has an explicit ‘override’ modifier or is an initializer (for which there are cases where ‘override’ is not required). This does not yet reduce override checking across source files, because checkOverrides() is still called during validateDeclForNameLookup(), and we’re still doing too much work to compute overrides. --- lib/Sema/TypeCheckDeclOverride.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index 5f1b6c17713de..4f5f8480c8fea 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -1644,8 +1644,10 @@ void TypeChecker::resolveOverriddenDecl(ValueDecl *VD) { if (isa(VD)) return; - // FIXME: We should check for the 'override' or 'required' keywords - // here, to short-circuit checking in the common case. + // Only initializers and declarations marked with the 'override' declaration + // modifier can override declarations. + if (!isa(VD) && !VD->getAttrs().hasAttribute()) + return; // FIXME: We should perform more minimal validation. validateDeclForNameLookup(VD); From dcf53dcea0db638a107b24f874fd28b728d99572 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 3 Jul 2018 17:18:09 -0700 Subject: [PATCH 7/8] [AST] Make DeclAttributes::getAttribute() propagate its AllowInvalid bit. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because, you know, I’d like it to do what it says it does. --- include/swift/AST/Attr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/swift/AST/Attr.h b/include/swift/AST/Attr.h index d7059161ce130..4b0aa1bfa397d 100644 --- a/include/swift/AST/Attr.h +++ b/include/swift/AST/Attr.h @@ -1385,7 +1385,7 @@ class DeclAttributes { /// Retrieve the first attribute of the given attribute class. template const ATTR *getAttribute(bool AllowInvalid = false) const { - return const_cast(this)->getAttribute(); + return const_cast(this)->getAttribute(AllowInvalid); } template From 19aa5521a70473bc73e281d387fb4a0e389561bf Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 3 Jul 2018 17:19:21 -0700 Subject: [PATCH 8/8] =?UTF-8?q?[Clang=20Importer]=20Always=20set=20overrid?= =?UTF-8?q?den=20decls=20and=20add=20=E2=80=98override=E2=80=99=20attribut?= =?UTF-8?q?e.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/ClangImporter/ImportDecl.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 8efc9f77d86b4..981d2d309e61b 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -6389,6 +6389,12 @@ ConstructorDecl *SwiftDeclConverter::importConstructor( } void SwiftDeclConverter::recordObjCOverride(AbstractFunctionDecl *decl) { + // Make sure that we always set the overriden declarations. + SWIFT_DEFER { + if (!decl->overriddenDeclsComputed()) + (void)decl->setOverriddenDecls({ }); + }; + // Figure out the class in which this method occurs. if (!decl->getDeclContext()->isTypeContext()) return; @@ -6419,6 +6425,7 @@ void SwiftDeclConverter::recordObjCOverride(AbstractFunctionDecl *decl) { func->getObjCSelector() != foundFunc->getObjCSelector()) continue; func->setOverriddenDecl(foundFunc); + func->getAttrs().add(new (func->getASTContext()) OverrideAttr(true)); return; } // Set constructor override. @@ -6429,6 +6436,8 @@ void SwiftDeclConverter::recordObjCOverride(AbstractFunctionDecl *decl) { ctor->getObjCSelector() != memberCtor->getObjCSelector()) continue; ctor->setOverriddenDecl(memberCtor); + ctor->getAttrs().add(new (ctor->getASTContext()) OverrideAttr(true)); + // Propagate 'required' to subclass initializers. if (memberCtor->isRequired() && !ctor->getAttrs().hasAttribute()) {