[Sema] Move @objc-without-Foundation checking to TypeCheckAttr

There doesn't seem to be any reason to delay this
diagnostic until after type-checking has finished
any more, so run it along with the rest of
attribute checking.

Author: hamishknight

include/swift/AST/SourceFile.h

@@ -251,12 +251,6 @@ class SourceFile final : public FileUnit {
   /// The list of local type declarations in the source file.
   llvm::SetVector<TypeDecl *> LocalTypeDecls;
 
-  /// A set of special declaration attributes which require the
-  /// Foundation module to be imported to work. If the foundation
-  /// module is still not imported by the time type checking is
-  /// complete, we diagnose.
-  llvm::SetVector<const DeclAttribute *> AttrsRequiringFoundation;
-
   /// A set of synthesized declarations that need to be type checked.
   llvm::SmallVector<Decl *, 8> SynthesizedDecls;
 

lib/Parse/ParseDecl.cpp

@@ -3935,15 +3935,6 @@ Parser::parseDecl(ParseDeclOptions Flags,
     DeclResult.setHasCodeCompletion();
   }
 
-  if (auto SF = CurDeclContext->getParentSourceFile()) {
-    if (!getScopeInfo().isInactiveConfigBlock()) {
-      for (auto Attr : Attributes) {
-        if (isa<ObjCAttr>(Attr))
-          SF->AttrsRequiringFoundation.insert(Attr);
-      }
-    }
-  }
-
   if (DeclResult.isNonNull()) {
     Decl *D = DeclResult.get();
     if (!declWasHandledAlready(D))

lib/Sema/TypeCheckAttr.cpp

@@ -23,6 +23,7 @@
 #include "swift/AST/DiagnosticsParse.h"
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/GenericSignatureBuilder.h"
+#include "swift/AST/ImportCache.h"
 #include "swift/AST/ModuleNameLookup.h"
 #include "swift/AST/NameLookup.h"
 #include "swift/AST/NameLookupRequests.h"
@@ -882,6 +883,41 @@ static bool checkObjCDeclContext(Decl *D) {
   return false;
 }
 
+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.
+    if (SF->Kind == SourceFileKind::SIL)
+      return;
+
+    // Don't diagnose for -disable-objc-attr-requires-foundation-module.
+    if (!ctx.LangOpts.EnableObjCAttrRequiresFoundation)
+      return;
+  }
+
+  // If we have the Foundation module, @objc is okay.
+  auto *foundation = ctx.getLoadedModule(ctx.Id_Foundation);
+  if (foundation && ctx.getImportCache().isImportedBy(foundation, SF))
+    return;
+
+  if (!ctx.LangOpts.EnableObjCInterop) {
+    ctx.Diags.diagnose(attr->getLocation(), diag::objc_interop_disabled)
+      .fixItRemove(attr->getRangeWithAt());
+  }
+
+  ctx.Diags.diagnose(attr->getLocation(),
+                     diag::attr_used_without_required_module, attr,
+                     ctx.Id_Foundation)
+    .highlight(attr->getRangeWithAt());
+}
+
 void AttributeChecker::visitObjCAttr(ObjCAttr *attr) {
   // Only certain decls can be ObjC.
   Optional<Diag<>> error;
@@ -988,6 +1024,9 @@ void AttributeChecker::visitObjCAttr(ObjCAttr *attr) {
     // Enum elements require names.
     diagnoseAndRemoveAttr(attr, diag::objc_enum_case_req_name);
   }
+
+  // Diagnose an @objc attribute used without importing Foundation.
+  diagnoseObjCAttrWithoutFoundation(attr, D);
 }
 
 void AttributeChecker::visitNonObjCAttr(NonObjCAttr *attr) {

lib/Sema/TypeCheckDeclObjC.cpp

@@ -1699,39 +1699,6 @@ void markAsObjC(ValueDecl *D, ObjCReason reason,
   }
 }
 
-void swift::diagnoseAttrsRequiringFoundation(SourceFile &SF) {
-  auto &Ctx = SF.getASTContext();
-
-  bool ImportsFoundationModule = false;
-
-  if (Ctx.LangOpts.EnableObjCInterop) {
-    if (!Ctx.LangOpts.EnableObjCAttrRequiresFoundation)
-      return;
-    if (SF.Kind == SourceFileKind::SIL)
-      return;
-  }
-
-  for (auto import : namelookup::getAllImports(&SF)) {
-    if (import.second->getName() == Ctx.Id_Foundation) {
-      ImportsFoundationModule = true;
-      break;
-    }
-  }
-
-  if (ImportsFoundationModule)
-    return;
-
-  for (auto Attr : SF.AttrsRequiringFoundation) {
-    if (!Ctx.LangOpts.EnableObjCInterop)
-      Ctx.Diags.diagnose(Attr->getLocation(), diag::objc_interop_disabled)
-        .fixItRemove(Attr->getRangeWithAt());
-    Ctx.Diags.diagnose(Attr->getLocation(),
-                       diag::attr_used_without_required_module,
-                       Attr, Ctx.Id_Foundation)
-      .highlight(Attr->getRangeWithAt());
-  }
-}
-
 /// Compute the information used to describe an Objective-C redeclaration.
 std::pair<unsigned, DeclName> swift::getObjCMethodDiagInfo(
                                 AbstractFunctionDecl *member) {

lib/Sema/TypeChecker.cpp

@@ -389,7 +389,6 @@ void swift::performWholeModuleTypeChecking(SourceFile &SF) {
   auto &Ctx = SF.getASTContext();
   FrontendStatsTracer tracer(Ctx.Stats,
                              "perform-whole-module-type-checking");
-  diagnoseAttrsRequiringFoundation(SF);
   diagnoseObjCMethodConflicts(SF);
   diagnoseObjCUnsatisfiedOptReqConflicts(SF);
   diagnoseUnintendedObjCMethodOverrides(SF);

lib/Sema/TypeChecker.h

@@ -1410,9 +1410,6 @@ bool isOverrideBasedOnType(const ValueDecl *decl, Type declTy,
 /// could fulfill a protocol requirement for it.
 bool isMemberOperator(FuncDecl *decl, Type type);
 
-/// Complain if @objc or dynamic is used without importing Foundation.
-void diagnoseAttrsRequiringFoundation(SourceFile &SF);
-
 /// Returns `true` iff `AdditiveArithmetic` derived conformances are enabled.
 bool isAdditiveArithmeticConformanceDerivationEnabled(SourceFile &SF);