Miscellaneous cleanup and consolidation for implementation-only checking

include/swift/AST/DiagnosticsSema.def

@@ -2388,16 +2388,15 @@ NOTE(construct_raw_representable_from_unwrapped_value,none,
      "construct %0 from unwrapped %1 value", (Type, Type))
 
 ERROR(decl_from_implementation_only_module,none,
-      "cannot use %0 here; %1 has been imported as "
-      "'@_implementationOnly'",
-      (DeclName, Identifier))
+      "cannot use %0 %1 here; %2 has been imported as implementation-only",
+      (DescriptiveDeclKind, DeclName, Identifier))
 ERROR(conformance_from_implementation_only_module,none,
       "cannot use conformance of %0 to %1 here; %2 has been imported as "
-      "'@_implementationOnly'",
+      "implementation-only",
       (Type, DeclName, Identifier))
 ERROR(assoc_conformance_from_implementation_only_module,none,
       "cannot use conformance of %0 to %1 in associated type %3 (inferred as "
-      "%4); %2 has been imported as '@_implementationOnly'",
+      "%4); %2 has been imported as implementation-only",
       (Type, DeclName, Identifier, Type, Type))
 
 // Derived conformances
@@ -4084,9 +4083,9 @@ WARNING(resilience_decl_unavailable_warn,
         (DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
 
 ERROR(inlinable_decl_ref_implementation_only,
-      none, "%0 %1 cannot be used in an inlinable "
-      "function because its module was imported implementation-only",
-      (DescriptiveDeclKind, DeclName))
+      none, "%0 %1 cannot be used in " FRAGILE_FUNC_KIND "2 "
+      "because %3 was imported implementation-only",
+      (DescriptiveDeclKind, DeclName, unsigned, Identifier))
 
 #undef FRAGILE_FUNC_KIND
 

lib/Sema/ResilienceDiagnostics.cpp

@@ -119,8 +119,11 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
     return true;
 
   // Check whether the declaration comes from a publically-imported module.
-  if (diagnoseDeclRefExportability(loc, declRef, DC))
-    return true;
+  // Skip this check for accessors because the associated property or subscript
+  // will also be checked, and will provide a better error message.
+  if (!isa<AccessorDecl>(D))
+    if (diagnoseDeclRefExportability(loc, declRef, DC, Kind))
+      return true;
 
   return false;
 }
@@ -208,15 +211,18 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
 }
 
 static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D,
-                                      const SourceFile &userSF) {
+                                      const SourceFile &userSF,
+                                      FragileFunctionKind fragileKind) {
   auto definingModule = D->getModuleContext();
   if (!userSF.isImportedImplementationOnly(definingModule))
     return false;
 
   // TODO: different diagnostics
   ASTContext &ctx = definingModule->getASTContext();
   ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_implementation_only,
-                     D->getDescriptiveKind(), D->getFullName());
+                     D->getDescriptiveKind(), D->getFullName(),
+                     static_cast<unsigned>(fragileKind),
+                     definingModule->getName());
   return true;
 }
 
@@ -288,9 +294,11 @@ void TypeChecker::diagnoseGenericTypeExportability(const TypeLoc &TL,
   }));
 }
 
-bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
-                                               ConcreteDeclRef declRef,
-                                               const DeclContext *DC) {
+bool
+TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
+                                          ConcreteDeclRef declRef,
+                                          const DeclContext *DC,
+                                          FragileFunctionKind fragileKind) {
   // We're only interested in diagnosing uses from source files.
   auto userSF = DC->getParentSourceFile();
   if (!userSF)
@@ -305,7 +313,7 @@ bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
     return false;
 
   const ValueDecl *D = declRef.getDecl();
-  if (diagnoseDeclExportability(loc, D, *userSF))
+  if (diagnoseDeclExportability(loc, D, *userSF, fragileKind))
     return true;
   if (diagnoseGenericArgumentsExportability(loc, declRef.getSubstitutions(),
                                             *userSF)) {

lib/Sema/TypeCheckAccess.cpp

@@ -1401,19 +1401,18 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
   }
 };
 
-class ImplementationOnlyImportChecker
-    : public DeclVisitor<ImplementationOnlyImportChecker> {
-  using CheckImplementationOnlyTypeCallback =
+class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
+  using CheckExportabilityTypeCallback =
       llvm::function_ref<void(const TypeDecl *, const TypeRepr *)>;
-  using CheckImplementationOnlyConformanceCallback =
+  using CheckExportabilityConformanceCallback =
       llvm::function_ref<void(const ProtocolConformance *)>;
 
   TypeChecker &TC;
 
   void checkTypeImpl(
       Type type, const TypeRepr *typeRepr, const SourceFile &SF,
-      CheckImplementationOnlyTypeCallback diagnoseType,
-      CheckImplementationOnlyConformanceCallback diagnoseConformance) {
+      CheckExportabilityTypeCallback diagnoseType,
+      CheckExportabilityConformanceCallback diagnoseConformance) {
     // Don't bother checking errors.
     if (type && type->hasError())
       return;
@@ -1448,13 +1447,13 @@ class ImplementationOnlyImportChecker
 
     class ProblematicTypeFinder : public TypeDeclFinder {
       const SourceFile &SF;
-      CheckImplementationOnlyTypeCallback diagnoseType;
-      CheckImplementationOnlyConformanceCallback diagnoseConformance;
+      CheckExportabilityTypeCallback diagnoseType;
+      CheckExportabilityConformanceCallback diagnoseConformance;
     public:
       ProblematicTypeFinder(
           const SourceFile &SF,
-          CheckImplementationOnlyTypeCallback diagnoseType,
-          CheckImplementationOnlyConformanceCallback diagnoseConformance)
+          CheckExportabilityTypeCallback diagnoseType,
+          CheckExportabilityConformanceCallback diagnoseConformance)
         : SF(SF), diagnoseType(diagnoseType),
           diagnoseConformance(diagnoseConformance) {}
 
@@ -1510,8 +1509,8 @@ class ImplementationOnlyImportChecker
 
   void checkType(
       Type type, const TypeRepr *typeRepr, const Decl *context,
-      CheckImplementationOnlyTypeCallback diagnoseType,
-      CheckImplementationOnlyConformanceCallback diagnoseConformance) {
+      CheckExportabilityTypeCallback diagnoseType,
+      CheckExportabilityConformanceCallback diagnoseConformance) {
     auto *SF = context->getDeclContext()->getParentSourceFile();
     assert(SF && "checking a non-source declaration?");
     return checkTypeImpl(type, typeRepr, *SF, diagnoseType,
@@ -1520,8 +1519,8 @@ class ImplementationOnlyImportChecker
 
   void checkType(
       const TypeLoc &TL, const Decl *context,
-      CheckImplementationOnlyTypeCallback diagnoseType,
-      CheckImplementationOnlyConformanceCallback diagnoseConformance) {
+      CheckExportabilityTypeCallback diagnoseType,
+      CheckExportabilityConformanceCallback diagnoseConformance) {
     checkType(TL.getType(), TL.getTypeRepr(), context, diagnoseType,
               diagnoseConformance);
   }
@@ -1558,6 +1557,7 @@ class ImplementationOnlyImportChecker
                     const TypeRepr *complainRepr) {
       ModuleDecl *M = offendingType->getModuleContext();
       auto diag = TC.diagnose(D, diag::decl_from_implementation_only_module,
+                              offendingType->getDescriptiveKind(),
                               offendingType->getFullName(), M->getName());
       highlightOffendingType(TC, diag, complainRepr);
     }
@@ -1573,19 +1573,19 @@ class ImplementationOnlyImportChecker
 
   static_assert(
       std::is_convertible<DiagnoseGenerically,
-                          CheckImplementationOnlyTypeCallback>::value,
+                          CheckExportabilityTypeCallback>::value,
       "DiagnoseGenerically has wrong call signature");
   static_assert(
       std::is_convertible<DiagnoseGenerically,
-                          CheckImplementationOnlyConformanceCallback>::value,
+                          CheckExportabilityConformanceCallback>::value,
       "DiagnoseGenerically has wrong call signature for conformance diags");
 
   DiagnoseGenerically getDiagnoseCallback(const Decl *D) {
     return DiagnoseGenerically(TC, D);
   }
 
 public:
-  explicit ImplementationOnlyImportChecker(TypeChecker &TC) : TC(TC) {}
+  explicit ExportabilityChecker(TypeChecker &TC) : TC(TC) {}
 
   static bool shouldSkipChecking(const ValueDecl *VD) {
     // Is this part of the module's API or ABI?
@@ -1620,7 +1620,7 @@ class ImplementationOnlyImportChecker
       if (shouldSkipChecking(VD))
         return;
 
-    DeclVisitor<ImplementationOnlyImportChecker>::visit(D);
+    DeclVisitor<ExportabilityChecker>::visit(D);
 
     if (auto *extension = dyn_cast<ExtensionDecl>(D->getDeclContext())) {
       checkType(extension->getExtendedTypeLoc(), extension,
@@ -1830,7 +1830,8 @@ class ImplementationOnlyImportChecker
       return;
 
     auto diag = TC.diagnose(diagLoc, diag::decl_from_implementation_only_module,
-                            PGD->getName(), M->getName());
+                            PGD->getDescriptiveKind(), PGD->getName(),
+                            M->getName());
     if (refRange.isValid())
       diag.highlight(refRange);
     diag.flush();
@@ -1904,5 +1905,5 @@ void swift::checkAccessControl(TypeChecker &TC, Decl *D) {
     checkExtensionGenericParamAccess(TC, ED);
   }
 
-  ImplementationOnlyImportChecker(TC).visit(D);
+  ExportabilityChecker(TC).visit(D);
 }

lib/Sema/TypeCheckProtocol.cpp

@@ -3643,17 +3643,17 @@ void ConformanceChecker::ensureRequirementsAreSatisfied(
   std::function<void(ProtocolConformanceRef)> writer
     = Conformance->populateSignatureConformances();
 
-  SourceFile *fileForCheckingImplementationOnlyUse = nullptr;
+  SourceFile *fileForCheckingExportability = nullptr;
   if (getRequiredAccessScope().isPublic() || isUsableFromInlineRequired())
-    fileForCheckingImplementationOnlyUse = DC->getParentSourceFile();
+    fileForCheckingExportability = DC->getParentSourceFile();
 
   class GatherConformancesListener : public GenericRequirementsCheckListener {
     NormalProtocolConformance *conformanceBeingChecked;
     SourceFile *SF;
     std::function<void(ProtocolConformanceRef)> &writer;
 
-    void checkForImplementationOnlyUse(Type depTy, Type replacementTy,
-                                       const ProtocolConformance *conformance) {
+    void checkExportability(Type depTy, Type replacementTy,
+                            const ProtocolConformance *conformance) {
       if (!SF)
         return;
 
@@ -3662,8 +3662,7 @@ void ConformanceChecker::ensureRequirementsAreSatisfied(
       for (auto &subConformance : subs.getConformances()) {
         if (!subConformance.isConcrete())
           continue;
-        checkForImplementationOnlyUse(depTy, replacementTy,
-                                      subConformance.getConcrete());
+        checkExportability(depTy, replacementTy, subConformance.getConcrete());
       }
 
       const RootProtocolConformance *rootConformance =
@@ -3695,9 +3694,9 @@ void ConformanceChecker::ensureRequirementsAreSatisfied(
     GatherConformancesListener(
         NormalProtocolConformance *conformance,
         std::function<void(ProtocolConformanceRef)> &writer,
-        SourceFile *fileForCheckingImplementationOnlyUse)
+        SourceFile *fileForCheckingExportability)
       : conformanceBeingChecked(conformance),
-        SF(fileForCheckingImplementationOnlyUse), writer(writer) { }
+        SF(fileForCheckingExportability), writer(writer) { }
 
     void satisfiedConformance(Type depTy, Type replacementTy,
                               ProtocolConformanceRef conformance) override {
@@ -3720,8 +3719,7 @@ void ConformanceChecker::ensureRequirementsAreSatisfied(
           conformance = ProtocolConformanceRef(concreteConformance);
         }
 
-        checkForImplementationOnlyUse(depTy, replacementTy,
-                                      concreteConformance);
+        checkExportability(depTy, replacementTy, concreteConformance);
       }
 
       writer(conformance);
@@ -3737,7 +3735,7 @@ void ConformanceChecker::ensureRequirementsAreSatisfied(
 
       return false;
     }
-  } listener(Conformance, writer, fileForCheckingImplementationOnlyUse);
+  } listener(Conformance, writer, fileForCheckingExportability);
 
   auto result = TC.checkGenericArguments(
       DC, Loc, Loc,

lib/Sema/TypeChecker.h

@@ -1891,7 +1891,8 @@ class TypeChecker final : public LazyResolver {
   /// exposes it in the interface of the current module, diagnose if it cannot
   /// reasonably be shared.
   bool diagnoseDeclRefExportability(SourceLoc loc, ConcreteDeclRef declRef,
-                                    const DeclContext *DC);
+                                    const DeclContext *DC,
+                                    FragileFunctionKind fragileKind);
 
 public:
   /// Given that a type is used from a particular context which