[Deserialization] Output a diagnostic when deserializing an invalid decl

include/swift/AST/DiagnosticsSema.def

@@ -776,6 +776,9 @@ NOTE(serialization_compatibility_version_mismatch,none,
      "(this is supported but may expose additional compiler issues)",
      (StringRef, StringRef, StringRef))
 
+ERROR(serialization_invalid_decl,none,
+      "deserialization of invalid declaration %0 from module '%1'",
+      (DeclName, StringRef))
 ERROR(serialization_allowing_invalid_decl,none,
       "allowing deserialization of invalid declaration %0 from module '%1'",
       (DeclName, StringRef))

lib/AST/Decl.cpp

@@ -439,15 +439,9 @@ bool Decl::isInvalid() const {
   case DeclKind::Constructor:
   case DeclKind::Destructor:
   case DeclKind::Func:
+  case DeclKind::Accessor:
   case DeclKind::EnumElement:
     return cast<ValueDecl>(this)->getInterfaceType()->hasError();
-
-  case DeclKind::Accessor: {
-    auto *AD = cast<AccessorDecl>(this);
-    if (AD->hasInterfaceType() && AD->getInterfaceType()->hasError())
-      return true;
-    return AD->getStorage()->isInvalid();
-  }
   }
 
   llvm_unreachable("Unknown decl kind");

lib/Sema/MiscDiagnostics.cpp

@@ -3242,9 +3242,9 @@ performTopLevelDeclDiagnostics(TopLevelCodeDecl *TLCD) {
 /// Perform diagnostics for func/init/deinit declarations.
 void swift::performAbstractFuncDeclDiagnostics(AbstractFunctionDecl *AFD) {
   // Don't produce these diagnostics for implicitly generated code.
-  if (AFD->getLoc().isInvalid() || AFD->isImplicit() || AFD->isInvalid())
+  if (AFD->getLoc().isInvalid() || AFD->isImplicit())
     return;
-  
+
   // Check for unused variables, as well as variables that are could be
   // declared as constants. Skip local functions though, since they will
   // be checked as part of their parent function or TopLevelCodeDecl.

lib/Sema/TypeCheckDeclObjC.cpp

@@ -1248,18 +1248,6 @@ Optional<ObjCReason> shouldMarkAsObjC(const ValueDecl *VD, bool allowImplicit) {
 
   // Local function to determine whether we can implicitly infer @objc.
   auto canInferImplicitObjC = [&](bool allowAnyAccess) {
-    if (VD->isInvalid())
-      return false;
-    if (VD->isOperator())
-      return false;
-
-    // Implicitly generated declarations are not @objc, except for constructors.
-    if (!allowImplicit && VD->isImplicit())
-      return false;
-
-    if (!allowAnyAccess && VD->getFormalAccess() <= AccessLevel::FilePrivate)
-      return false;
-
     if (auto accessor = dyn_cast<AccessorDecl>(VD)) {
       switch (accessor->getAccessorKind()) {
       case AccessorKind::DidSet:
@@ -1275,6 +1263,19 @@ Optional<ObjCReason> shouldMarkAsObjC(const ValueDecl *VD, bool allowImplicit) {
         break;
       }
     }
+
+    if (VD->isInvalid())
+      return false;
+    if (VD->isOperator())
+      return false;
+
+    // Implicitly generated declarations are not @objc, except for constructors.
+    if (!allowImplicit && VD->isImplicit())
+      return false;
+
+    if (!allowAnyAccess && VD->getFormalAccess() <= AccessLevel::FilePrivate)
+      return false;
+
     return true;
   };
 

lib/Serialization/Deserialization.cpp

@@ -4085,15 +4085,23 @@ ModuleFile::getDeclChecked(
       return deserialized;
 
     auto *decl = declOrOffset.get();
-    if (isAllowModuleWithCompilerErrorsEnabled() && decl->isInvalid()) {
-      if (!isa<ParamDecl>(decl) && !decl->isImplicit()) {
-        // The parent function will be invalid if the parameter is invalid,
-        // implicits should have an invalid explicit as well
-        if (auto *VD = dyn_cast<ValueDecl>(decl)) {
-          getContext().Diags.diagnose(
-              VD->getLoc(), diag::serialization_allowing_invalid_decl,
-              VD->getName(), VD->getModuleContext()->getNameStr());
-        }
+    if (decl->isInvalid() && isa<ValueDecl>(decl)) {
+      auto *VD = cast<ValueDecl>(decl);
+      if (!isAllowModuleWithCompilerErrorsEnabled()) {
+        getContext().Diags.diagnose(
+            SourceLoc(), diag::serialization_invalid_decl,
+            VD->getName(), VD->getModuleContext()->getNameStr());
+      } else if (!isa<ParamDecl>(decl) && !decl->isImplicit()) {
+        // Grabbing the location of a parameter gets the location of its
+        // context, which isn't deserialized yet. Since the parent will be
+        // invalid if the parameter is, just skip it and wait for the
+        // parent to output the diagnostic.
+        //
+        // Any invalid implicit should have an invalid explicit, so ignore in
+        // preference for the explicit.
+        getContext().Diags.diagnose(
+            VD->getLoc(), diag::serialization_allowing_invalid_decl,
+            VD->getName(), VD->getModuleContext()->getNameStr());
       }
     }
   } else if (matchAttributes) {