From 90f6e5b3682f70affcd4ab8be61c2b6c662a24fd Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Fri, 15 May 2020 21:15:27 -0700 Subject: [PATCH] xxx --- include/swift/AST/ASTContext.h | 8 +- include/swift/AST/Decl.h | 14 +- include/swift/AST/Identifier.h | 7 + include/swift/AST/NameLookupRequests.h | 18 + include/swift/AST/NameLookupTypeIDZone.def | 4 + include/swift/AST/SourceFile.h | 16 - lib/AST/ASTContext.cpp | 12 +- lib/AST/Decl.cpp | 3 - lib/AST/Identifier.cpp | 4 +- lib/AST/NameLookup.cpp | 173 ++++++--- lib/ClangImporter/ClangImporter.cpp | 10 +- lib/ClangImporter/ImportDecl.cpp | 102 ++--- lib/ClangImporter/ImporterImpl.h | 6 - lib/Sema/TypeCheckAttr.cpp | 14 +- lib/Sema/TypeCheckDeclObjC.cpp | 432 +++++++-------------- lib/Sema/TypeCheckDeclPrimary.cpp | 28 +- lib/Sema/TypeCheckProtocol.cpp | 76 +++- lib/Sema/TypeChecker.cpp | 3 - lib/Sema/TypeChecker.h | 12 +- test/attr/attr_objc.swift | 2 +- 20 files changed, 442 insertions(+), 502 deletions(-) diff --git a/include/swift/AST/ASTContext.h b/include/swift/AST/ASTContext.h index 160b905be6626..bd1e950a589e2 100644 --- a/include/swift/AST/ASTContext.h +++ b/include/swift/AST/ASTContext.h @@ -745,6 +745,9 @@ class ASTContext final { /// \param isInstanceMethod Whether we are looking for an instance method /// (vs. a class method). /// + /// \param swiftOnly If true, only loads methods from imported Swift modules, + /// skipping the Clang importer. + /// /// \param previousGeneration The previous generation with which this /// callback was invoked. The list of methods will already contain all of /// the results from generations up and including \c previousGeneration. @@ -752,9 +755,8 @@ class ASTContext final { /// \param methods The list of @objc methods in this class that have this /// selector and are instance/class methods as requested. This list will be /// extended with any methods found in subsequent generations. - void loadObjCMethods(ClassDecl *classDecl, - ObjCSelector selector, - bool isInstanceMethod, + void loadObjCMethods(ClassDecl *classDecl, ObjCSelector selector, + bool isInstanceMethod, bool swiftOnly, unsigned previousGeneration, llvm::TinyPtrVector &methods); diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index eb6af822e7ab7..6089b304d18c9 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -3874,13 +3874,16 @@ using AncestryOptions = OptionSet; /// The type of the decl itself is a MetatypeType; use getDeclaredType() /// to get the declared type ("Complex" in the above example). class ClassDecl final : public NominalTypeDecl { + friend class NominalTypeDecl; + friend class ObjCMethodDirectLookupRequest; + class ObjCMethodLookupTable; SourceLoc ClassLoc; ObjCMethodLookupTable *ObjCMethodLookup = nullptr; - /// Create the Objective-C member lookup table. - void createObjCMethodLookup(); + /// Prepare the Objective-C member lookup table. + void prepareObjCMethodLookup(); struct { /// The superclass decl and a bit to indicate whether the @@ -4117,11 +4120,8 @@ class ClassDecl final : public NominalTypeDecl { /// /// \param isInstance Whether we are looking for an instance method /// (vs. a class method). - MutableArrayRef lookupDirect(ObjCSelector selector, - bool isInstance); - - /// Record the presence of an @objc method with the given selector. - void recordObjCMethod(AbstractFunctionDecl *method, ObjCSelector selector); + TinyPtrVector lookupDirect(ObjCSelector selector, + bool isInstance) const; /// Get all the members of this class, synthesizing any implicit members /// that appear in the vtable if needed. diff --git a/include/swift/AST/Identifier.h b/include/swift/AST/Identifier.h index 85e4101ebe590..490e622febdd2 100644 --- a/include/swift/AST/Identifier.h +++ b/include/swift/AST/Identifier.h @@ -891,8 +891,15 @@ class ObjCSelector { friend bool operator>=(ObjCSelector lhs, ObjCSelector rhs) { return lhs.compare(rhs) >= 0; } + + friend llvm::hash_code hash_value(ObjCSelector selector) { + using llvm::hash_value; + return hash_value(selector.getOpaqueValue()); + } }; +void simple_display(llvm::raw_ostream &out, ObjCSelector selector); + } // end namespace swift namespace llvm { diff --git a/include/swift/AST/NameLookupRequests.h b/include/swift/AST/NameLookupRequests.h index df8c0dea04179..7b47b0d252612 100644 --- a/include/swift/AST/NameLookupRequests.h +++ b/include/swift/AST/NameLookupRequests.h @@ -770,6 +770,24 @@ class LookupConformanceInModuleRequest ProtocolConformanceRef result) const; }; +class ObjCMethodDirectLookupRequest + : public SimpleRequest( + ClassDecl *, ObjCSelector, bool), + RequestFlags::Uncached> { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + // Evaluation. + TinyPtrVector evaluate(Evaluator &evaluator, + ClassDecl *CD, + ObjCSelector selector, + bool isInstance) const; +}; + #define SWIFT_TYPEID_ZONE NameLookup #define SWIFT_TYPEID_HEADER "swift/AST/NameLookupTypeIDZone.def" #include "swift/Basic/DefineTypeIDZone.h" diff --git a/include/swift/AST/NameLookupTypeIDZone.def b/include/swift/AST/NameLookupTypeIDZone.def index de346c54f8806..6b49111296a51 100644 --- a/include/swift/AST/NameLookupTypeIDZone.def +++ b/include/swift/AST/NameLookupTypeIDZone.def @@ -97,3 +97,7 @@ SWIFT_REQUEST(NameLookup, LookupPostfixOperatorRequest, SWIFT_REQUEST(NameLookup, LookupPrecedenceGroupRequest, PrecedenceGroupDecl *(OperatorLookupDescriptor), Cached, NoLocationInfo) +SWIFT_REQUEST(NameLookup, ObjCMethodDirectLookupRequest, + TinyPtrVector(ClassDecl *, ObjCSelector, + bool), + Uncached, NoLocationInfo) diff --git a/include/swift/AST/SourceFile.h b/include/swift/AST/SourceFile.h index 820af1095f4e9..c251286d65e6b 100644 --- a/include/swift/AST/SourceFile.h +++ b/include/swift/AST/SourceFile.h @@ -263,22 +263,6 @@ class SourceFile final : public FileUnit { llvm::DenseMap> ObjCMethods; - /// List of Objective-C methods, which is used for checking unintended - /// Objective-C overrides. - std::vector ObjCMethodList; - - /// An unsatisfied, optional @objc requirement in a protocol conformance. - using ObjCUnsatisfiedOptReq = std::pair; - - /// List of optional @objc protocol requirements that have gone - /// unsatisfied, which might conflict with other Objective-C methods. - std::vector ObjCUnsatisfiedOptReqs; - - using ObjCMethodConflict = std::tuple; - - /// List of Objective-C member conflicts we have found during type checking. - std::vector ObjCMethodConflicts; - /// Describes what kind of file this is, which can affect some type checking /// and other behavior. const SourceFileKind Kind; diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 9bfeef7665326..20f3c6287cc75 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -1486,14 +1486,16 @@ void ASTContext::loadExtensions(NominalTypeDecl *nominal, } void ASTContext::loadObjCMethods( - ClassDecl *classDecl, - ObjCSelector selector, - bool isInstanceMethod, - unsigned previousGeneration, - llvm::TinyPtrVector &methods) { + ClassDecl *classDecl, ObjCSelector selector, bool isInstanceMethod, + bool swiftOnly, unsigned previousGeneration, + llvm::TinyPtrVector &methods) { PrettyStackTraceSelector stackTraceSelector("looking for", selector); PrettyStackTraceDecl stackTraceDecl("...in", classDecl); for (auto &loader : getImpl().ModuleLoaders) { + // Ignore the Clang importer if we've been asked for Swift-only results. + if (swiftOnly && loader.get() == getClangModuleLoader()) + continue; + loader->loadObjCMethods(classDecl, selector, isInstanceMethod, previousGeneration, methods); } diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 047b23bbe6234..e74aab388f17f 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -4274,9 +4274,6 @@ GetDestructorRequest::evaluate(Evaluator &evaluator, ClassDecl *CD) const { // Mark DD as ObjC, as all dtors are. DD->setIsObjC(ctx.LangOpts.EnableObjCInterop); - if (ctx.LangOpts.EnableObjCInterop) - CD->recordObjCMethod(DD, DD->getObjCSelector()); - return DD; } diff --git a/lib/AST/Identifier.cpp b/lib/AST/Identifier.cpp index 7c2d461e955a1..396e5e5f52191 100644 --- a/lib/AST/Identifier.cpp +++ b/lib/AST/Identifier.cpp @@ -271,4 +271,6 @@ void ObjCSelector::dump() const { llvm::errs() << *this << "\n"; } - +void swift::simple_display(llvm::raw_ostream &out, ObjCSelector selector) { + out << "'" << selector << "'"; +} diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index b6b2e577d00d6..2dbb7e01e12de 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -954,25 +954,56 @@ class swift::MemberLookupTable { } }; -namespace { +/// Class member lookup table, which is a member lookup table with a second +/// table for lookup based on Objective-C selector. +class ClassDecl::ObjCMethodLookupTable { + using Key = std::pair; + /// Stores the set of Objective-C methods with a given selector within the /// Objective-C method lookup table. - struct StoredObjCMethods { + struct StoredMethods { /// The generation count at which this list was last updated. unsigned Generation = 0; /// The set of methods with the given selector. llvm::TinyPtrVector Methods; }; -} // end anonymous namespace -/// Class member lookup table, which is a member lookup table with a second -/// table for lookup based on Objective-C selector. -class ClassDecl::ObjCMethodLookupTable - : public llvm::DenseMap, - StoredObjCMethods> -{ + llvm::DenseMap Table; + llvm::DenseSet LazilyCompleteNames; + public: + /// Create a new Obj-C method lookup table. + explicit ObjCMethodLookupTable(ASTContext &ctx) { + // Make sure the destructor is called when the AST context is torn down. + ctx.addCleanup([this]() { + this->~ObjCMethodLookupTable(); + }); + } + + /// Returns \c true if the lookup table has a complete accounting of the + /// given name. + bool isLazilyComplete(ObjCSelector selector, bool isInstanceMethod) const { + return LazilyCompleteNames.count({selector, isInstanceMethod}); + } + + /// Mark a given lazily-loaded name as being complete. + void markLazilyComplete(ObjCSelector selector, bool isInstanceMethod) { + LazilyCompleteNames.insert({selector, isInstanceMethod}); + } + + /// Clears the cache of lazily-complete names. This _must_ be called when + /// new extensions with lazy members are added to the type, or lookups will + /// return inconsistent or stale results. + void clearLazilyCompleteCache() { + LazilyCompleteNames.clear(); + } + + void addMember(Decl *member); + void addMembers(DeclRange members); + + StoredMethods &operator[](Key key) { return Table[key]; } + // Only allow allocation of member lookup tables using the allocator in // ASTContext or by doing a placement new. void *operator new(size_t Bytes, ASTContext &C, @@ -1045,13 +1076,22 @@ void MemberLookupTable::updateLookupTable(NominalTypeDecl *nominal) { } void NominalTypeDecl::addedExtension(ExtensionDecl *ext) { - if (!LookupTable) return; + ClassDecl::ObjCMethodLookupTable *ObjCLookupTable = nullptr; + if (auto *CD = dyn_cast(this)) + ObjCLookupTable = CD->ObjCMethodLookup; if (ext->hasLazyMembers()) { - LookupTable->addMembers(ext->getCurrentMembersWithoutLoading()); - LookupTable->clearLazilyCompleteCache(); + if (LookupTable) { + LookupTable->addMembers(ext->getCurrentMembersWithoutLoading()); + LookupTable->clearLazilyCompleteCache(); + } + if (ObjCLookupTable) + ObjCLookupTable->clearLazilyCompleteCache(); } else { - LookupTable->addMembers(ext->getMembers()); + if (LookupTable) + LookupTable->addMembers(ext->getMembers()); + if (ObjCLookupTable) + ObjCLookupTable->addMembers(ext->getMembers()); } } @@ -1297,63 +1337,80 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, includeAttrImplements); } -void ClassDecl::createObjCMethodLookup() { - assert(!ObjCMethodLookup && "Already have an Objective-C member table"); - auto &ctx = getASTContext(); - ObjCMethodLookup = new (ctx) ObjCMethodLookupTable(); +void ClassDecl::ObjCMethodLookupTable::addMember(Decl *member) { + if (auto *storage = dyn_cast(member)) { + // If this is an @objc var, make sure to add the necessary accessors. + if (storage->isObjC()) { + storage->visitEmittedAccessors( + [&](AccessorDecl *accessor) { addMember(accessor); }); + } + return; + } - // Register a cleanup with the ASTContext to call the lookup table - // destructor. - ctx.addCleanup([this]() { - this->ObjCMethodLookup->~ObjCMethodLookupTable(); - }); + // We're only interested in tracking @objc functions. + auto *afd = dyn_cast(member); + if (!afd || !afd->isObjC()) + return; + + auto &stored = Table[{afd->getObjCSelector(), afd->isObjCInstanceMethod()}]; + stored.Methods.push_back(afd); } -MutableArrayRef -ClassDecl::lookupDirect(ObjCSelector selector, bool isInstance) { - if (!ObjCMethodLookup) { - createObjCMethodLookup(); - } +void ClassDecl::ObjCMethodLookupTable::addMembers(DeclRange members) { + for (auto *member : members) + addMember(member); +} - // If any modules have been loaded since we did the search last (or if we - // hadn't searched before), look in those modules, too. - auto &stored = (*ObjCMethodLookup)[{selector, isInstance}]; - ASTContext &ctx = getASTContext(); - if (ctx.getCurrentGeneration() > stored.Generation) { - ctx.loadObjCMethods(this, selector, isInstance, stored.Generation, - stored.Methods); - stored.Generation = ctx.getCurrentGeneration(); +void ClassDecl::prepareObjCMethodLookup() { + if (ObjCMethodLookup) { + // Make sure the list of extensions is up-to-date, which may trigger the + // clearing of the lazily-complete cache. + (void)getExtensions(); + return; } - return { stored.Methods.begin(), stored.Methods.end() }; -} + auto &ctx = getASTContext(); + ObjCMethodLookup = new (ctx) ObjCMethodLookupTable(ctx); -void ClassDecl::recordObjCMethod(AbstractFunctionDecl *method, - ObjCSelector selector) { - if (!ObjCMethodLookup) { - createObjCMethodLookup(); - } + // Pre-populate the table with any entries from the main module. + if (!hasLazyMembers()) + ObjCMethodLookup->addMembers(getMembers()); - // Record the method. - bool isInstanceMethod = method->isObjCInstanceMethod(); - auto &vec = (*ObjCMethodLookup)[{selector, isInstanceMethod}].Methods; + for (auto *ext : getExtensions()) { + if (ext->wasDeserialized() || ext->hasClangNode()) + continue; - // Check whether we have a duplicate. This only checks more than one - // element in ill-formed code, so the linear search is acceptable. - if (std::find(vec.begin(), vec.end(), method) != vec.end()) - return; + ObjCMethodLookup->addMembers(ext->getMembers()); + } +} - if (auto *sf = method->getParentSourceFile()) { - if (vec.size() == 1) { - // We have a conflict. - sf->ObjCMethodConflicts.push_back(std::make_tuple(this, selector, - isInstanceMethod)); - } if (vec.empty()) { - sf->ObjCMethodList.push_back(method); - } +TinyPtrVector +ObjCMethodDirectLookupRequest::evaluate(Evaluator &evaluator, ClassDecl *CD, + ObjCSelector selector, + bool isInstance) const { + CD->prepareObjCMethodLookup(); + auto &stored = (*CD->ObjCMethodLookup)[{selector, isInstance}]; + + // If we haven't seen this name before, or additional imported extensions have + // been bound since we last did a lookup, ask the module loaders to update the + // lookup table. + if (!CD->ObjCMethodLookup->isLazilyComplete(selector, isInstance)) { + auto &ctx = CD->getASTContext(); + ctx.loadObjCMethods(CD, selector, isInstance, /*ignoreClang*/ false, + stored.Generation, stored.Methods); + stored.Generation = ctx.getCurrentGeneration(); + CD->ObjCMethodLookup->markLazilyComplete(selector, isInstance); } + return stored.Methods; +} - vec.push_back(method); +TinyPtrVector +ClassDecl::lookupDirect(ObjCSelector selector, bool isInstance) const { + auto &ctx = getASTContext(); + auto *mutableThis = const_cast(this); + return evaluateOrDefault( + ctx.evaluator, + ObjCMethodDirectLookupRequest{mutableThis, selector, isInstance}, {}); } /// Determine whether the given declaration is an acceptable lookup diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 5d5fdb30326e4..34537217fe447 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -3045,11 +3045,6 @@ void ClangImporter::loadObjCMethods( bool isInstanceMethod, unsigned previousGeneration, llvm::TinyPtrVector &methods) { - // If we're currently looking for this selector, don't load any Objective-C - // methods. - if (Impl.ActiveSelectors.count({selector, isInstanceMethod})) - return; - const auto *objcClass = dyn_cast_or_null(classDecl->getClangDecl()); if (!objcClass) @@ -3082,10 +3077,7 @@ void ClangImporter::loadObjCMethods( // earlier, because we aren't tracking generation counts for Clang modules. // Filter out the duplicates. // FIXME: We shouldn't need to do this. - llvm::SmallPtrSet known; - known.insert(methods.begin(), methods.end()); - - if (known.insert(method).second) + if (!llvm::is_contained(methods, method)) methods.push_back(method); } diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 98dccac4e67fb..84c39954a90ca 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -2191,7 +2191,9 @@ namespace { if (fd->isInstanceMember() != decl->isInstanceProperty()) continue; - assert(fd->getName().getArgumentNames().empty()); + if (!fd->getName().getArgumentNames().empty()) + continue; + foundMethod = true; } else { auto *var = cast(result); @@ -2280,7 +2282,7 @@ namespace { return getVersion() == getActiveSwiftVersion(); } - void recordMemberInContext(DeclContext *dc, ValueDecl *member) { + void recordMemberInContext(const DeclContext *dc, ValueDecl *member) { assert(member && "Attempted to record null member!"); auto *nominal = dc->getSelfNominalTypeDecl(); auto name = member->getBaseName(); @@ -4130,17 +4132,6 @@ namespace { } decl->setIsObjC(true); decl->setIsDynamic(true); - - // If the declaration we attached the 'objc' attribute to is within a - // class, record it in the class. - if (auto contextTy = decl->getDeclContext()->getDeclaredInterfaceType()) { - if (auto classDecl = contextTy->getClassOrBoundGenericClass()) { - if (auto method = dyn_cast(decl)) { - if (name) - classDecl->recordObjCMethod(method, *name); - } - } - } } /// Add an @objc(name) attribute with the given, optional name expressed as @@ -4170,33 +4161,50 @@ namespace { bool isInstance, const DeclContext *dc, llvm::function_ref filter) { // We only need to perform this check for classes. - auto classDecl - = dc->getDeclaredInterfaceType()->getClassOrBoundGenericClass(); + auto *classDecl = dc->getSelfClassDecl(); if (!classDecl) return false; - // Make sure we don't search in Clang modules for this method. - ++Impl.ActiveSelectors[{selector, isInstance}]; - - // Look for a matching imported or deserialized member. - bool result = false; - for (auto decl : classDecl->lookupDirect(selector, isInstance)) { - if ((decl->getClangDecl() - || !decl->getDeclContext()->getParentSourceFile()) - && importedName.getDeclName() == decl->getName() - && filter(decl)) { - result = true; - break; + auto predicate = [&](Decl *member) -> bool { + auto *afd = dyn_cast(member); + if (!afd || (afd->isObjCInstanceMethod() != isInstance)) + return false; + + // Both the selector and imported name must match. + if (afd->getObjCSelector() != selector || + importedName.getDeclName() != afd->getName()) + return false; + + return filter(afd); + }; + + // First check to see if we've already imported a method with the same + // selector. + auto importedMembers = Impl.MembersForNominal.find(classDecl); + if (importedMembers != Impl.MembersForNominal.end()) { + auto baseName = importedName.getDeclName().getBaseName(); + auto membersForName = importedMembers->second.find(baseName); + if (membersForName != importedMembers->second.end()) { + for (auto *member : membersForName->second) { + if (predicate(member)) + return true; + } } } - // Restore the previous active count in the active-selector mapping. - auto activeCount = Impl.ActiveSelectors.find({selector, isInstance}); - --activeCount->second; - if (activeCount->second == 0) - Impl.ActiveSelectors.erase(activeCount); - - return result; + // Then check to see if there's any matching @objc methods from imported + // Swift modules if this is for a deserialized class. + if (classDecl->wasDeserialized()) { + auto &ctx = Impl.SwiftContext; + TinyPtrVector deserializedMethods; + ctx.loadObjCMethods(classDecl, selector, isInstance, /*swiftOnly*/ true, + /*prevGeneration*/ 0, deserializedMethods); + for (auto *member : deserializedMethods) { + if (predicate(member)) + return true; + } + } + return false; } Decl *importObjCMethodDecl(const clang::ObjCMethodDecl *decl, @@ -4485,12 +4493,8 @@ namespace { } } - // We only care about recording methods with no arguments here, because - // they can shadow imported properties. - if (!isa(result) && - result->getName().getArgumentNames().empty()) { + if (!isa(result)) recordMemberInContext(dc, result); - } return result; } @@ -6462,6 +6466,7 @@ ConstructorDecl *SwiftDeclConverter::importConstructor( addObjCAttribute(result, selector); + recordMemberInContext(dc, result); Impl.recordImplicitUnwrapForDecl(result, importedType.isImplicitlyUnwrapped()); @@ -6668,14 +6673,21 @@ SwiftDeclConverter::importSubscript(Decl *decl, }; auto findCounterpart = [&](clang::Selector sel) -> FuncDecl * { - // If the declaration we're starting from is in a class, first - // look for a class member with the appropriate selector. + // If the declaration we're starting from is in a class, first check to see + // if we've already imported a class method with the appropriate selector. if (auto classDecl = decl->getDeclContext()->getSelfClassDecl()) { auto swiftSel = Impl.importSelector(sel); - for (auto found : classDecl->lookupDirect(swiftSel, true)) { - if (auto foundFunc = dyn_cast(found)) - if (foundFunc->hasClangNode()) - return foundFunc; + auto importedMembers = Impl.MembersForNominal.find(classDecl); + if (importedMembers != Impl.MembersForNominal.end()) { + for (auto membersForName : importedMembers->second) { + for (auto *member : membersForName.second) { + auto *afd = dyn_cast(member); + if (afd && afd->isInstanceMember() && + afd->getObjCSelector() == swiftSel) { + return afd; + } + } + } } } diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index 72de3a47bcde7..a17c72e4baea8 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -439,12 +439,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation llvm::DenseMap> ImportedMacroConstants; - /// Keeps track of active selector-based lookups, so that we don't infinitely - /// recurse when checking whether a method with a given selector has already - /// been imported. - llvm::DenseMap, unsigned> - ActiveSelectors; - // Mapping from imported types to their raw value types. llvm::DenseMap RawTypes; diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index ccac659fb96f5..fdc3d8ee66ea5 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -934,10 +934,6 @@ static void diagnoseObjCAttrWithoutFoundation(ObjCAttr *attr, Decl *decl) { auto *SF = decl->getDeclContext()->getParentSourceFile(); assert(SF); - // We only care about explicitly written @objc attributes. - if (attr->isImplicit()) - return; - auto &ctx = SF->getASTContext(); if (ctx.LangOpts.EnableObjCInterop) { // Don't diagnose in a SIL file. @@ -966,6 +962,10 @@ static void diagnoseObjCAttrWithoutFoundation(ObjCAttr *attr, Decl *decl) { } void AttributeChecker::visitObjCAttr(ObjCAttr *attr) { + // We only care about explicitly written @objc attributes. + if (attr->isImplicit()) + return; + // Only certain decls can be ObjC. Optional> error; if (isa(D) || @@ -1034,12 +1034,6 @@ void AttributeChecker::visitObjCAttr(ObjCAttr *attr) { } else { auto func = cast(D); - // Trigger lazy loading of any imported members with the same selector. - // This ensures we correctly diagnose selector conflicts. - if (auto *CD = D->getDeclContext()->getSelfClassDecl()) { - (void) CD->lookupDirect(*objcName, !func->isStatic()); - } - // We have a function. Make sure that the number of parameters // matches the "number of colons" in the name. auto params = func->getParameters(); diff --git a/lib/Sema/TypeCheckDeclObjC.cpp b/lib/Sema/TypeCheckDeclObjC.cpp index 21002610485e9..e67afd4cbcce2 100644 --- a/lib/Sema/TypeCheckDeclObjC.cpp +++ b/lib/Sema/TypeCheckDeclObjC.cpp @@ -35,7 +35,6 @@ bool swift::shouldDiagnoseObjCReason(ObjCReason reason, ASTContext &ctx) { case ObjCReason::ExplicitlyCDecl: case ObjCReason::ExplicitlyDynamic: case ObjCReason::ExplicitlyObjC: - case ObjCReason::ExplicitlyIBOutlet: case ObjCReason::ExplicitlyIBAction: case ObjCReason::ExplicitlyIBSegueAction: case ObjCReason::ExplicitlyNSManaged: @@ -50,6 +49,11 @@ bool swift::shouldDiagnoseObjCReason(ObjCReason reason, ASTContext &ctx) { case ObjCReason::ExplicitlyGKInspectable: return !ctx.LangOpts.EnableSwift3ObjCInference; + case ObjCReason::ExplicitlyIBOutlet: + // @IBOutlet has stricter checking in TypeCheckAttr for what the type of the + // property can be, so defer to that. + return false; + case ObjCReason::MemberOfObjCSubclass: case ObjCReason::MemberOfObjCMembersClass: case ObjCReason::ElementOfObjCEnum: @@ -1657,11 +1661,6 @@ void markAsObjC(ValueDecl *D, ObjCReason reason, } } - // Record the method in the class, if it's a member of one. - if (auto classDecl = D->getDeclContext()->getSelfClassDecl()) { - classDecl->recordObjCMethod(method, selector); - } - // Record the method in the source file. if (auto sourceFile = method->getParentSourceFile()) { sourceFile->ObjCMethods[selector].push_back(method); @@ -1977,330 +1976,161 @@ static AbstractFunctionDecl *lookupObjCMethodInClass( inheritingInits); } -bool swift::diagnoseUnintendedObjCMethodOverrides(SourceFile &sf) { - auto &Ctx = sf.getASTContext(); - auto &methods = sf.ObjCMethodList; - - // If no Objective-C methods were defined in this file, we're done. - if (methods.empty()) - return false; - - // Sort the methods by declaration order. - std::sort(methods.begin(), methods.end(), OrderDeclarations(Ctx.SourceMgr)); +void swift::diagnoseUnintendedObjCMethodOverrides( + AbstractFunctionDecl *method) { + auto *classDecl = method->getDeclContext()->getSelfClassDecl(); + assert(classDecl); // For each Objective-C method declared in this file, check whether // it overrides something in one of its superclasses. We // intentionally don't respect access control here, since everything // is visible to the Objective-C runtime. - bool diagnosedAny = false; - for (auto method : methods) { - // If the method has an @objc override, we don't need to do any - // more checking. - if (auto overridden = method->getOverriddenDecl()) { - if (overridden->isObjC()) - continue; - } - - // Skip deinitializers. - if (isa(method)) - continue; - - // Skip invalid declarations. - if (method->isInvalid()) - continue; - // Skip declarations with an invalid 'override' attribute on them. - if (auto attr = method->getAttrs().getAttribute(true)) { - if (attr->isInvalid()) - continue; - } + // Skip deinitializers. + if (isa(method)) + return; - auto classDecl = method->getDeclContext()->getSelfClassDecl(); - if (!classDecl) - continue; // error-recovery path, only + // Skip invalid declarations. + if (method->isInvalid()) + return; - if (!classDecl->hasSuperclass()) - continue; + // Skip declarations with an invalid 'override' attribute on them. + if (auto attr = method->getAttrs().getAttribute(true)) { + if (attr->isInvalid()) + return; + } - // Look for a method that we have overridden in one of our - // superclasses. - // Note: This should be treated as a lookup for intra-module dependency - // purposes, but a subclass already depends on its superclasses and any - // extensions for many other reasons. - auto selector = method->getObjCSelector(); - AbstractFunctionDecl *overriddenMethod - = lookupObjCMethodInClass(classDecl->getSuperclassDecl(), - selector, - method->isObjCInstanceMethod(), - isa(method), - Ctx.SourceMgr); - if (!overriddenMethod) - continue; + // If the method has an @objc override, we don't need to do any + // more checking. + if (auto overridden = method->getOverriddenDecl()) { + if (overridden->isObjC()) + return; + } - // Ignore stub implementations. - if (auto overriddenCtor = dyn_cast(overriddenMethod)) { - if (overriddenCtor->hasStubImplementation()) - continue; - } + if (!classDecl->hasSuperclass()) + return; - // Require the declaration to actually come from a class. Selectors that - // come from protocol requirements are not actually overrides. - if (!overriddenMethod->getDeclContext()->getSelfClassDecl()) - continue; + // Look for a method that we have overridden in one of our + // superclasses. + // Note: This should be treated as a lookup for intra-module dependency + // purposes, but a subclass already depends on its superclasses and any + // extensions for many other reasons. + auto &ctx = method->getASTContext(); + auto selector = method->getObjCSelector(); + AbstractFunctionDecl *overriddenMethod = lookupObjCMethodInClass( + classDecl->getSuperclassDecl(), selector, method->isObjCInstanceMethod(), + isa(method), ctx.SourceMgr); + if (!overriddenMethod) + return; - // Diagnose the override. - auto methodDiagInfo = getObjCMethodDiagInfo(method); - auto overriddenDiagInfo = getObjCMethodDiagInfo(overriddenMethod); - Ctx.Diags.diagnose(method, diag::objc_override_other, - methodDiagInfo.first, - methodDiagInfo.second, - overriddenDiagInfo.first, - overriddenDiagInfo.second, - selector, - overriddenMethod->getDeclContext() - ->getDeclaredInterfaceType()); - const ValueDecl *overriddenDecl = overriddenMethod; - if (overriddenMethod->isImplicit()) - if (auto accessor = dyn_cast(overriddenMethod)) - overriddenDecl = accessor->getStorage(); - Ctx.Diags.diagnose(overriddenDecl, diag::objc_declared_here, - overriddenDiagInfo.first, overriddenDiagInfo.second); - - diagnosedAny = true; + // Ignore stub implementations. + if (auto overriddenCtor = dyn_cast(overriddenMethod)) { + if (overriddenCtor->hasStubImplementation()) + return; } - return diagnosedAny; -} - -/// Retrieve the source file for the given Objective-C member conflict. -static MutableArrayRef -getObjCMethodConflictDecls(const SourceFile::ObjCMethodConflict &conflict) { - ClassDecl *classDecl = std::get<0>(conflict); - ObjCSelector selector = std::get<1>(conflict); - bool isInstanceMethod = std::get<2>(conflict); - - return classDecl->lookupDirect(selector, isInstanceMethod); -} + // Require the declaration to actually come from a class. Selectors that + // come from protocol requirements are not actually overrides. + if (!overriddenMethod->getDeclContext()->getSelfClassDecl()) + return; -/// Given a set of conflicting Objective-C methods, remove any methods -/// that are legitimately overridden in Objective-C, i.e., because -/// they occur in different modules, one is defined in the class, and -/// the other is defined in an extension (category) thereof. -static void removeValidObjCConflictingMethods( - MutableArrayRef &methods) { - // Erase any invalid or stub declarations. We don't want to complain about - // them, because we might already have complained about - // redeclarations based on Swift matching. - auto newEnd = std::remove_if(methods.begin(), methods.end(), - [&](AbstractFunctionDecl *method) { - if (method->isInvalid()) - return true; - - if (auto ad = dyn_cast(method)) { - return ad->getStorage()->isInvalid(); - } - - if (auto ctor - = dyn_cast(method)) { - if (ctor->hasStubImplementation()) - return true; - - return false; - } - - return false; - }); - methods = methods.slice(0, newEnd - methods.begin()); + // Diagnose the override. + auto methodDiagInfo = getObjCMethodDiagInfo(method); + auto overriddenDiagInfo = getObjCMethodDiagInfo(overriddenMethod); + ctx.Diags.diagnose( + method, diag::objc_override_other, methodDiagInfo.first, + methodDiagInfo.second, overriddenDiagInfo.first, + overriddenDiagInfo.second, selector, + overriddenMethod->getDeclContext()->getDeclaredInterfaceType()); + method->setInvalid(); + + const ValueDecl *overriddenDecl = overriddenMethod; + if (overriddenMethod->isImplicit()) + if (auto accessor = dyn_cast(overriddenMethod)) + overriddenDecl = accessor->getStorage(); + + ctx.Diags.diagnose(overriddenDecl, diag::objc_declared_here, + overriddenDiagInfo.first, overriddenDiagInfo.second); } -bool swift::diagnoseObjCMethodConflicts(SourceFile &sf) { - // If there were no conflicts, we're done. - if (sf.ObjCMethodConflicts.empty()) - return false; +void swift::diagnoseObjCMethodConflicts(AbstractFunctionDecl *afd) { + auto *CD = afd->getDeclContext()->getSelfClassDecl(); + assert(CD && afd->isObjC()); - auto &Ctx = sf.getASTContext(); + // Skip deinitializers. + if (isa(afd)) + return; - OrderDeclarations ordering(Ctx.SourceMgr); + if (afd->isInvalid()) + return; - // Sort the set of conflicts so we get a deterministic order for - // diagnostics. We use the first conflicting declaration in each set to - // perform the sort. - auto localConflicts = sf.ObjCMethodConflicts; - std::sort(localConflicts.begin(), localConflicts.end(), - [&](const SourceFile::ObjCMethodConflict &lhs, - const SourceFile::ObjCMethodConflict &rhs) { - return ordering(getObjCMethodConflictDecls(lhs)[1], - getObjCMethodConflictDecls(rhs)[1]); - }); + auto selector = afd->getObjCSelector(); + auto otherMethods = CD->lookupDirect(selector, afd->isObjCInstanceMethod()); // Diagnose each conflict. - bool anyConflicts = false; - for (const auto &conflict : localConflicts) { - ObjCSelector selector = std::get<1>(conflict); - - auto methods = getObjCMethodConflictDecls(conflict); - - // Prune out cases where it is acceptable to have a conflict. - removeValidObjCConflictingMethods(methods); - if (methods.size() < 2) + for (auto *other : otherMethods) { + if (other == afd) continue; - // Diagnose the conflict. - anyConflicts = true; + if (other->isInvalid()) + continue; - // If the first method is in an extension and the second is not, swap them - // so the primary diagnostic is on the extension method. - if (isa(methods[0]->getDeclContext()) && - !isa(methods[1]->getDeclContext())) { - std::swap(methods[0], methods[1]); - - // Within a source file, use our canonical ordering. - } else if (methods[0]->getParentSourceFile() == - methods[1]->getParentSourceFile() && - !ordering(methods[0], methods[1])) { - std::swap(methods[0], methods[1]); - } - - // Otherwise, fall back to the order in which the declarations were type - // checked. - - auto originalMethod = methods.front(); - auto conflictingMethods = methods.slice(1); - - auto origDiagInfo = getObjCMethodDiagInfo(originalMethod); - for (auto conflictingDecl : conflictingMethods) { - auto diagInfo = getObjCMethodDiagInfo(conflictingDecl); - - const ValueDecl *originalDecl = originalMethod; - if (originalMethod->isImplicit()) - if (auto accessor = dyn_cast(originalMethod)) - originalDecl = accessor->getStorage(); - - if (diagInfo == origDiagInfo) { - Ctx.Diags.diagnose(conflictingDecl, diag::objc_redecl_same, - diagInfo.first, diagInfo.second, selector); - Ctx.Diags.diagnose(originalDecl, diag::invalid_redecl_prev, - originalDecl->getBaseName()); - } else { - Ctx.Diags.diagnose(conflictingDecl, diag::objc_redecl, - diagInfo.first, - diagInfo.second, - origDiagInfo.first, - origDiagInfo.second, - selector); - Ctx.Diags.diagnose(originalDecl, diag::objc_declared_here, - origDiagInfo.first, origDiagInfo.second); - } + if (auto ad = dyn_cast(other)) { + if (ad->getStorage()->isInvalid()) + continue; } - } - - return anyConflicts; -} - -/// Retrieve the source location associated with this declaration -/// context. -static SourceLoc getDeclContextLoc(DeclContext *dc) { - if (auto ext = dyn_cast(dc)) - return ext->getLoc(); - return cast(dc)->getLoc(); -} - -bool swift::diagnoseObjCUnsatisfiedOptReqConflicts(SourceFile &sf) { - // If there are no unsatisfied, optional @objc requirements, we're done. - if (sf.ObjCUnsatisfiedOptReqs.empty()) - return false; + if (auto *ctor = dyn_cast(other)) { + if (ctor->hasStubImplementation()) + continue; + } - auto &Ctx = sf.getASTContext(); - - // Sort the set of local unsatisfied requirements, so we get a - // deterministic order for diagnostics. - auto &localReqs = sf.ObjCUnsatisfiedOptReqs; - std::sort(localReqs.begin(), localReqs.end(), - [&](const SourceFile::ObjCUnsatisfiedOptReq &lhs, - const SourceFile::ObjCUnsatisfiedOptReq &rhs) -> bool { - return Ctx.SourceMgr.isBeforeInBuffer(getDeclContextLoc(lhs.first), - getDeclContextLoc(rhs.first)); - }); - - // Check each of the unsatisfied optional requirements. - bool anyDiagnosed = false; - for (const auto &unsatisfied : localReqs) { - // Check whether there is a conflict here. - ClassDecl *classDecl = unsatisfied.first->getSelfClassDecl(); - auto req = unsatisfied.second; - auto selector = req->getObjCSelector(); - bool isInstanceMethod = req->isInstanceMember(); - // FIXME: Also look in superclasses? - auto conflicts = classDecl->lookupDirect(selector, isInstanceMethod); - if (conflicts.empty()) - continue; + auto &ctx = afd->getASTContext(); + OrderDeclarations ordering(ctx.SourceMgr); - // Diagnose the conflict. - auto reqDiagInfo = getObjCMethodDiagInfo(unsatisfied.second); - auto conflictDiagInfo = getObjCMethodDiagInfo(conflicts[0]); - auto protocolName - = cast(req->getDeclContext())->getName(); - Ctx.Diags.diagnose(conflicts[0], - diag::objc_optional_requirement_conflict, - conflictDiagInfo.first, - conflictDiagInfo.second, - reqDiagInfo.first, - reqDiagInfo.second, - selector, - protocolName); - - // Fix the name of the witness, if we can. - if (req->getName() != conflicts[0]->getName() && - req->getKind() == conflicts[0]->getKind() && - isa(req) == isa(conflicts[0])) { - // They're of the same kind: fix the name. - unsigned kind; - if (isa(req)) - kind = 1; - else if (auto accessor = dyn_cast(req)) - kind = isa(accessor->getStorage()) ? 3 : 2; - else if (isa(req)) - kind = 0; - else { - llvm_unreachable("unhandled @objc declaration kind"); - } + auto original = afd; + auto conflicting = other; - auto diag = Ctx.Diags.diagnose(conflicts[0], - diag::objc_optional_requirement_swift_rename, - kind, req->getName()); - - // Fix the Swift name. - fixDeclarationName(diag, conflicts[0], req->getName()); - - // Fix the '@objc' attribute, if needed. - if (!conflicts[0]->canInferObjCFromRequirement(req)) - fixDeclarationObjCName(diag, conflicts[0], - conflicts[0]->getObjCRuntimeName(), - req->getObjCRuntimeName(), - /*ignoreImpliedName=*/true); - } - - // @nonobjc will silence this warning. - bool hasExplicitObjCAttribute = false; - if (auto objcAttr = conflicts[0]->getAttrs().getAttribute()) - hasExplicitObjCAttribute = !objcAttr->isImplicit(); - if (!hasExplicitObjCAttribute) - Ctx.Diags.diagnose(conflicts[0], diag::req_near_match_nonobjc, true) - .fixItInsert( - conflicts[0]->getAttributeInsertionLoc(/*forModifier=*/false), - "@nonobjc "); - - Ctx.Diags.diagnose(getDeclContextLoc(unsatisfied.first), - diag::protocol_conformance_here, - true, - classDecl->getName(), - protocolName); - Ctx.Diags.diagnose(req, diag::kind_declname_declared_here, - DescriptiveDeclKind::Requirement, reqDiagInfo.second); - - anyDiagnosed = true; + // If the first method is in an extension and the second is not, swap them + // so the conflicting decl is the one in the extension. + if (isa(original->getDeclContext()) && + !isa(conflicting->getDeclContext())) { + std::swap(original, conflicting); + + // Within a source file, use our canonical ordering. + } else if (original->getParentSourceFile() == + conflicting->getParentSourceFile() && + !ordering(original, conflicting)) { + std::swap(original, conflicting); + + // If one decl is in a source file, and the other isn't, make sure we + // diagnose on the one in the source file. + } else if (!conflicting->getParentSourceFile() && + original->getParentSourceFile()) { + std::swap(original, conflicting); + } + + auto origDiagInfo = getObjCMethodDiagInfo(original); + auto diagInfo = getObjCMethodDiagInfo(conflicting); + + // If we have an implicit accessor, diagnose on the storage decl instead. + const ValueDecl *originalDecl = original; + if (original->isImplicit()) + if (auto accessor = dyn_cast(original)) + originalDecl = accessor->getStorage(); + + if (diagInfo == origDiagInfo) { + ctx.Diags.diagnose(conflicting, diag::objc_redecl_same, diagInfo.first, + diagInfo.second, selector); + ctx.Diags.diagnose(originalDecl, diag::invalid_redecl_prev, + originalDecl->getBaseName()); + } else { + ctx.Diags.diagnose(conflicting, diag::objc_redecl, diagInfo.first, + diagInfo.second, origDiagInfo.first, + origDiagInfo.second, selector); + ctx.Diags.diagnose(originalDecl, diag::objc_declared_here, + origDiagInfo.first, origDiagInfo.second); + } + conflicting->setInvalid(); } - - return anyDiagnosed; } diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index d9e2333066ece..96e4ce65fa1c8 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -1240,6 +1240,13 @@ class DeclChecker : public DeclVisitor { (void) evaluateOrDefault(decl->getASTContext().evaluator, CheckRedeclarationRequest{VD}, {}); + if (auto *afd = dyn_cast(VD)) { + if (afd->isObjC() && afd->getDeclContext()->getSelfClassDecl()) { + swift::diagnoseObjCMethodConflicts(afd); + swift::diagnoseUnintendedObjCMethodOverrides(afd); + } + } + // Compute access level. (void) VD->getFormalAccess(); @@ -1913,27 +1920,6 @@ class DeclChecker : public DeclVisitor { if (CD->requiresStoredPropertyInits()) checkRequiredInClassInits(CD); - // Compute @objc for each superclass member, to catch selector - // conflicts resulting from unintended overrides. - // - // FIXME: This should be a request so we can measure how much work - // we're doing here. - CD->walkSuperclasses( - [&](ClassDecl *superclass) { - if (!superclass->getParentSourceFile()) - return TypeWalker::Action::Stop; - - for (auto *member : superclass->getMembers()) { - if (auto *vd = dyn_cast(member)) { - if (vd->isPotentiallyOverridable()) { - (void) vd->isObjC(); - } - } - } - - return TypeWalker::Action::Continue; - }); - if (auto superclassTy = CD->getSuperclass()) { ClassDecl *Super = superclassTy->getClassOrBoundGenericClass(); bool isInvalidSuperclass = false; diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 0de8b1ceee583..44a9895097eb9 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -5034,6 +5034,76 @@ LookupAllConformancesInContextRequest::evaluate( return DC->getLocalConformances(ConformanceLookupKind::All); } +static void diagnoseObjCUnsatisfiedOptReqConflict(AbstractFunctionDecl *req, + DeclContext *dc) { + auto &Ctx = dc->getASTContext(); + + // Check whether there is a conflict here. + auto *classDecl = dc->getSelfClassDecl(); + auto selector = req->getObjCSelector(); + bool isInstanceMethod = req->isObjCInstanceMethod(); + + // FIXME: Also look in superclasses? + auto conflicts = classDecl->lookupDirect(selector, isInstanceMethod); + if (conflicts.empty()) + return; + + // Diagnose the conflict. + auto reqDiagInfo = getObjCMethodDiagInfo(req); + auto conflictDiagInfo = getObjCMethodDiagInfo(conflicts[0]); + auto protocolName = cast(req->getDeclContext())->getName(); + Ctx.Diags.diagnose(conflicts[0], diag::objc_optional_requirement_conflict, + conflictDiagInfo.first, conflictDiagInfo.second, + reqDiagInfo.first, reqDiagInfo.second, selector, + protocolName); + + // Fix the name of the witness, if we can. + if (req->getName() != conflicts[0]->getName() && + req->getKind() == conflicts[0]->getKind() && + isa(req) == isa(conflicts[0])) { + // They're of the same kind: fix the name. + unsigned kind; + if (isa(req)) + kind = 1; + else if (auto accessor = dyn_cast(req)) + kind = isa(accessor->getStorage()) ? 3 : 2; + else if (isa(req)) + kind = 0; + else { + llvm_unreachable("unhandled @objc declaration kind"); + } + + auto diag = Ctx.Diags.diagnose(conflicts[0], + diag::objc_optional_requirement_swift_rename, + kind, req->getName()); + + // Fix the Swift name. + fixDeclarationName(diag, conflicts[0], req->getName()); + + // Fix the '@objc' attribute, if needed. + if (!conflicts[0]->canInferObjCFromRequirement(req)) + fixDeclarationObjCName(diag, conflicts[0], + conflicts[0]->getObjCRuntimeName(), + req->getObjCRuntimeName(), + /*ignoreImpliedName=*/true); + } + + // @nonobjc will silence this warning. + bool hasExplicitObjCAttribute = false; + if (auto objcAttr = conflicts[0]->getAttrs().getAttribute()) + hasExplicitObjCAttribute = !objcAttr->isImplicit(); + if (!hasExplicitObjCAttribute) + Ctx.Diags.diagnose(conflicts[0], diag::req_near_match_nonobjc, true) + .fixItInsert( + conflicts[0]->getAttributeInsertionLoc(/*forModifier=*/false), + "@nonobjc "); + + Ctx.Diags.diagnose(dc->getAsDecl(), diag::protocol_conformance_here, true, + classDecl->getName(), protocolName); + Ctx.Diags.diagnose(req, diag::kind_declname_declared_here, + DescriptiveDeclKind::Requirement, reqDiagInfo.second); +} + void TypeChecker::checkConformancesInContext(DeclContext *dc, IterableDeclContext *idc) { // For anything imported from Clang, lazily check conformances. @@ -5310,13 +5380,13 @@ void TypeChecker::checkConformancesInContext(DeclContext *dc, // Record this requirement. if (auto funcReq = dyn_cast(req)) { - sf->ObjCUnsatisfiedOptReqs.emplace_back(dc, funcReq); + diagnoseObjCUnsatisfiedOptReqConflict(funcReq, dc); } else { auto storageReq = cast(req); if (auto getter = storageReq->getParsedAccessor(AccessorKind::Get)) - sf->ObjCUnsatisfiedOptReqs.emplace_back(dc, getter); + diagnoseObjCUnsatisfiedOptReqConflict(getter, dc); if (auto setter = storageReq->getParsedAccessor(AccessorKind::Set)) - sf->ObjCUnsatisfiedOptReqs.emplace_back(dc, setter); + diagnoseObjCUnsatisfiedOptReqConflict(setter, dc); } } } diff --git a/lib/Sema/TypeChecker.cpp b/lib/Sema/TypeChecker.cpp index 1be236f1bb27f..e3e802a04c882 100644 --- a/lib/Sema/TypeChecker.cpp +++ b/lib/Sema/TypeChecker.cpp @@ -375,9 +375,6 @@ void swift::performWholeModuleTypeChecking(SourceFile &SF) { auto &Ctx = SF.getASTContext(); FrontendStatsTracer tracer(Ctx.Stats, "perform-whole-module-type-checking"); - diagnoseObjCMethodConflicts(SF); - diagnoseObjCUnsatisfiedOptReqConflicts(SF); - diagnoseUnintendedObjCMethodOverrides(SF); // In whole-module mode, import verification is deferred until all files have // been type checked. This avoids caching imported declarations when a valid diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index dabd2c602e014..57cc76c6e11d7 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -1397,19 +1397,11 @@ bool isAdditiveArithmeticConformanceDerivationEnabled(SourceFile &SF); /// Diagnose any Objective-C method overrides that aren't reflected /// as overrides in Swift. -bool diagnoseUnintendedObjCMethodOverrides(SourceFile &sf); +void diagnoseUnintendedObjCMethodOverrides(AbstractFunctionDecl *afd); /// Diagnose all conflicts between members that have the same /// Objective-C selector in the same class. -/// -/// \param sf The source file for which we are diagnosing conflicts. -/// -/// \returns true if there were any conflicts diagnosed. -bool diagnoseObjCMethodConflicts(SourceFile &sf); - -/// Diagnose any unsatisfied @objc optional requirements of -/// protocols that conflict with methods. -bool diagnoseObjCUnsatisfiedOptReqConflicts(SourceFile &sf); +void diagnoseObjCMethodConflicts(AbstractFunctionDecl *afd); /// Retrieve information about the given Objective-C method for /// diagnostic purposes, to be used with OBJC_DIAG_SELECT in diff --git a/test/attr/attr_objc.swift b/test/attr/attr_objc.swift index 77848a0ab196a..63eeb332164e7 100644 --- a/test/attr/attr_objc.swift +++ b/test/attr/attr_objc.swift @@ -847,7 +847,7 @@ class infer_instanceVar1 { } var observingAccessorsVar1: Int { - // CHECK: @_hasStorage @objc var observingAccessorsVar1: Int { + // CHECK: @objc @_hasStorage var observingAccessorsVar1: Int { willSet {} // CHECK-NEXT: {{^}} @objc get { // CHECK-NEXT: return