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 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/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; 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()) { diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index c8c5ca907aa14..4f5f8480c8fea 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()) @@ -323,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. /// @@ -523,263 +522,226 @@ 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; - - auto classDecl = owningTy->getClassOrBoundGenericClass(); - if (!classDecl) - 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; - Type superclass = classDecl->getSuperclass(); - if (!superclass) - return false; + /// The superclass in which we'll look. + Type superclass; - // Ignore accessor methods (e.g. getters and setters), they will be handled - // when their storage decl is processed. - if (isa(decl)) - return false; + // Cached member lookup results. + LookupResult members; - auto method = dyn_cast(decl); - ConstructorDecl *ctor = nullptr; - if (method) - ctor = dyn_cast(method); + /// The lookup name used to find \c members. + DeclName membersName; - auto abstractStorage = dyn_cast(decl); - assert((method || abstractStorage) && "Not a method or abstractStorage?"); - SubscriptDecl *subscript = nullptr; - if (abstractStorage) - subscript = dyn_cast(abstractStorage); - - // Figure out the type of the declaration that we're using for comparisons. - auto declTy = getMemberTypeForComparison(TC.Context, decl); + /// The type of the declaration, cached here once it has been computed. + Type cachedDeclType; - // 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; + public: + OverrideMatcher(TypeChecker &tc, ValueDecl *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; + /// Returns true when it's possible to perform any override matching. + explicit operator bool() const { + return static_cast(superclass); } - if (members.empty()) { - auto lookupOptions = defaultMemberLookupOptions; - - // Class methods cannot override declarations only - // visible via dynamic dispatch. - lookupOptions -= NameLookupFlags::DynamicLookup; - - // Class methods cannot override declarations only - // visible as protocol requirements or protocol - // extension members. - lookupOptions -= NameLookupFlags::ProtocolMembers; - lookupOptions -= NameLookupFlags::PerformConformanceCheck; + /// 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); + } - members = TC.lookupMember(dc, superclass, - name, lookupOptions); + return cachedDeclType; } + }; +} - for (auto memberResult : members) { - auto member = memberResult.getValueDecl(); +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; - if (member->isInvalid()) - continue; + auto *dc = decl->getDeclContext(); - if (member->getKind() != decl->getKind()) - continue; + auto owningTy = dc->getDeclaredInterfaceType(); + if (!owningTy) + return; - if (!dc->getAsClassOrClassExtensionContext()) - continue; + auto classDecl = owningTy->getClassOrBoundGenericClass(); + if (!classDecl) + return; - auto parentDecl = cast(member); + // FIXME: Get the superclass from owningTy directly? + superclass = classDecl->getSuperclass(); +} - // Check whether there are any obvious reasons why the two given - // declarations do not have an overriding relationship. - if (!areOverrideCompatibleSimple(decl, parentDecl)) - continue; +SmallVector OverrideMatcher::match( + OverrideCheckingAttempt attempt) { + // If there's no matching we can do, fail. + if (!*this) return { }; - 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; - } + auto dc = decl->getDeclContext(); - // Properties don't need anything here since they are always - // checked by name. - } + // 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 { }; + } - if (ctor) { - // Factory methods cannot be overridden. - auto parentCtor = cast(parentDecl); - if (parentCtor->isFactoryInit()) - continue; - } + // 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 the types are identical. - auto parentDeclTy = - getMemberTypeForComparison(TC.Context, parentDecl, decl); - if (parentDeclTy->hasError()) - continue; + // Class methods cannot override declarations only + // visible via dynamic dispatch. + lookupOptions -= NameLookupFlags::DynamicLookup; - if (isOverrideBasedOnType(decl, declTy, parentDecl, parentDeclTy)) { - matches.push_back({parentDecl, true, parentDeclTy}); - hadExactMatch = true; - continue; - } + // Class methods cannot override declarations only + // visible as protocol requirements or protocol + // extension members. + lookupOptions -= NameLookupFlags::ProtocolMembers; + lookupOptions -= NameLookupFlags::PerformConformanceCheck; - // 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; - } + membersName = name; + members = tc.lookupMember(dc, superclass, membersName, lookupOptions); + } - // 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; - } + // Check each member we found. + SmallVector matches; + for (auto memberResult : members) { + auto parentDecl = memberResult.getValueDecl(); - 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 there are any obvious reasons why the two given + // declarations do not have an overriding relationship. + if (!areOverrideCompatibleSimple(decl, parentDecl)) + 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); + auto parentMethod = dyn_cast(parentDecl); + auto parentStorage = dyn_cast(parentDecl); + assert(parentMethod || parentStorage); - // Put an invalid 'override' attribute here. - makeInvalidOverrideAttr(decl); + // Check whether the types are identical. + auto parentDeclTy = getMemberTypeForComparison(ctx, parentDecl, decl); + if (parentDeclTy->hasError()) + continue; - return true; - } + Type declTy = getDeclComparisonType(); + if (isOverrideBasedOnType(decl, declTy, parentDecl, parentDeclTy)) { + matches.push_back({parentDecl, true, parentDeclTy}); + continue; } - if (!matches.empty()) - break; - ++attempt; - } while (true); + // 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; + } - assert(!matches.empty()); + // 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; + } - // 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()); + 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, false, parentDeclTy}); + continue; + } + } else if (getDeclComparisonType()->matches(parentDeclTy, matchMode)) { + matches.push_back({parentDecl, false, parentDeclTy}); + continue; + } + } - // 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 +750,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 +761,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 +780,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 +793,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 +813,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 +836,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 +853,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 +934,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 +946,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 { @@ -1534,6 +1571,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 +1631,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; } @@ -1599,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); 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); 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}}