Sema: Refactoring in preparation for conformance availability checking

Today, we check conformance exportability in two places:

1) For inlinable function bodies: in availability checking
2) For types in inlinable declaration signatures: in availability checking
3) For types in non-inlinable declaration signatures: in ExportabilityChecker

I want to merge 2) and 3) together, and also generalize the conformance
exportability check to check conformance availability as well.

This is a preliminary refactoring towards achieving this goal.

Author: slavapestov

lib/Sema/ResilienceDiagnostics.cpp

@@ -27,13 +27,6 @@
 
 using namespace swift;
 
-/// A uniquely-typed boolean to reduce the chances of accidentally inverting
-/// a check.
-enum class DowngradeToWarning: bool {
-  No,
-  Yes
-};
-
 bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
                                            ConcreteDeclRef declRef,
                                            const DeclContext *DC,
@@ -55,16 +48,17 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
   // 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))
+    if (diagnoseDeclRefExportability(loc, declRef, DC,
+                                     None, Kind))
       return true;
 
   return false;
 }
 
 bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
-                                           const ValueDecl *D,
-                                           const DeclContext *DC,
-                                           FragileFunctionKind Kind) {
+                                                 const ValueDecl *D,
+                                                 const DeclContext *DC,
+                                                 FragileFunctionKind Kind) {
   assert(Kind.kind != FragileFunctionKind::None);
 
   // Local declarations are OK.
@@ -150,96 +144,87 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
 static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D,
                                       const SourceFile &userSF,
                                       const DeclContext *userDC,
+                                      Optional<ExportabilityReason> reason,
                                       FragileFunctionKind fragileKind) {
-  assert(fragileKind.kind != FragileFunctionKind::None);
+  if (fragileKind.kind == FragileFunctionKind::None && !reason.hasValue())
+    return false;
 
   auto definingModule = D->getModuleContext();
 
+  auto downgradeToWarning = DowngradeToWarning::No;
   auto originKind = getDisallowedOriginKind(
-      D, userSF, userDC->getInnermostDeclarationDeclContext());
+      D, userSF, userDC->getInnermostDeclarationDeclContext(),
+      downgradeToWarning);
   if (originKind == DisallowedOriginKind::None)
     return false;
 
-  // TODO: different diagnostics
   ASTContext &ctx = definingModule->getASTContext();
-  ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_from_hidden_module,
-                     D->getDescriptiveKind(), D->getName(),
-                     static_cast<unsigned>(fragileKind.kind),
-                     definingModule->getName(),
-                     static_cast<unsigned>(originKind));
-  return true;
-}
 
-static bool
-diagnoseGenericArgumentsExportability(SourceLoc loc,
-                                      SubstitutionMap subs,
-                                      const SourceFile &userSF,
-                                      const DeclContext *userDC) {
-  bool hadAnyIssues = false;
-  for (ProtocolConformanceRef conformance : subs.getConformances()) {
-    if (!conformance.isConcrete())
-      continue;
-    const ProtocolConformance *concreteConf = conformance.getConcrete();
-
-    SubstitutionMap subConformanceSubs =
-        concreteConf->getSubstitutions(userSF.getParentModule());
-    diagnoseGenericArgumentsExportability(loc, subConformanceSubs, userSF, userDC);
-
-    const RootProtocolConformance *rootConf =
-        concreteConf->getRootConformance();
-    ModuleDecl *M = rootConf->getDeclContext()->getParentModule();
-
-    auto originKind = getDisallowedOriginKind(
-        rootConf->getDeclContext()->getAsDecl(),
-        userSF, userDC->getInnermostDeclarationDeclContext());
-    if (originKind == DisallowedOriginKind::None)
-      continue;
-
-    ASTContext &ctx = M->getASTContext();
-    ctx.Diags.diagnose(loc, diag::conformance_from_implementation_only_module,
-                       rootConf->getType(),
-                       rootConf->getProtocol()->getName(), 0, M->getName(),
+  if (fragileKind.kind == FragileFunctionKind::None) {
+    auto errorOrWarning = downgradeToWarning == DowngradeToWarning::Yes?
+                              diag::decl_from_hidden_module_warn:
+                              diag::decl_from_hidden_module;
+    ctx.Diags.diagnose(loc, errorOrWarning,
+                       D->getDescriptiveKind(),
+                       D->getName(),
+                       static_cast<unsigned>(*reason),
+                       definingModule->getName(),
+                       static_cast<unsigned>(originKind));
+
+    D->diagnose(diag::kind_declared_here, DescriptiveDeclKind::Type);
+  } else {
+    ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_from_hidden_module,
+                       D->getDescriptiveKind(), D->getName(),
+                       static_cast<unsigned>(fragileKind.kind),
+                       definingModule->getName(),
                        static_cast<unsigned>(originKind));
-    hadAnyIssues = true;
   }
-  return hadAnyIssues;
+  return true;
 }
 
-void TypeChecker::diagnoseGenericTypeExportability(SourceLoc Loc, Type T,
-                                                   const DeclContext *DC) {
-  const SourceFile *SF = DC->getParentSourceFile();
-  if (!SF)
-    return;
-
-  // FIXME: It would be nice to highlight just the part of the type that's
-  // problematic, but unfortunately the TypeRepr doesn't have the
-  // information we need and the Type doesn't easily map back to it.
-  if (auto *BGT = dyn_cast<BoundGenericType>(T.getPointer())) {
-    ModuleDecl *useModule = SF->getParentModule();
-    auto subs = T->getContextSubstitutionMap(useModule, BGT->getDecl());
-    (void)diagnoseGenericArgumentsExportability(Loc, subs, *SF, DC);
-  } else if (auto *TAT = dyn_cast<TypeAliasType>(T.getPointer())) {
-    auto subs = TAT->getSubstitutionMap();
-    (void)diagnoseGenericArgumentsExportability(Loc, subs, *SF, DC);
-  }
+bool
+TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
+                                              const RootProtocolConformance *rootConf,
+                                              const SourceFile &userSF,
+                                              const DeclContext *userDC,
+                                              Optional<ExportabilityReason> reason,
+                                              FragileFunctionKind fragileKind) {
+  if (fragileKind.kind == FragileFunctionKind::None && !reason.hasValue())
+    return false;
+
+  auto originKind = getDisallowedOriginKind(
+      rootConf->getDeclContext()->getAsDecl(),
+      userSF, userDC->getInnermostDeclarationDeclContext());
+  if (originKind == DisallowedOriginKind::None)
+    return false;
+
+  if (!reason.hasValue())
+    reason = ExportabilityReason::General;
+
+  ModuleDecl *M = rootConf->getDeclContext()->getParentModule();
+  ASTContext &ctx = M->getASTContext();
+  ctx.Diags.diagnose(loc, diag::conformance_from_implementation_only_module,
+                     rootConf->getType(),
+                     rootConf->getProtocol()->getName(),
+                     static_cast<unsigned>(*reason),
+                     M->getName(),
+                     static_cast<unsigned>(originKind));
+  return true;
 }
 
 bool
 TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
                                           ConcreteDeclRef declRef,
                                           const DeclContext *DC,
+                                          Optional<ExportabilityReason> reason,
                                           FragileFunctionKind fragileKind) {
   // We're only interested in diagnosing uses from source files.
   auto userSF = DC->getParentSourceFile();
   if (!userSF)
     return false;
 
   const ValueDecl *D = declRef.getDecl();
-  if (diagnoseDeclExportability(loc, D, *userSF, DC, fragileKind))
-    return true;
-  if (diagnoseGenericArgumentsExportability(loc, declRef.getSubstitutions(),
-                                            *userSF, DC)) {
+  if (diagnoseDeclExportability(loc, D, *userSF, DC, reason, fragileKind))
     return true;
-  }
   return false;
 }

lib/Sema/TypeCheckAccess.cpp

@@ -35,15 +35,6 @@ using namespace swift;
 
 namespace {
 
-/// A uniquely-typed boolean to reduce the chances of accidentally inverting
-/// a check.
-///
-/// \see checkTypeAccess
-enum class DowngradeToWarning: bool {
-  No,
-  Yes
-};
-
 /// Calls \p callback for each type in each requirement provided by
 /// \p source.
 static void forAllRequirementTypes(
@@ -1464,15 +1455,17 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
   }
 };
 
+} // end anonymous namespace
+
 /// Returns the kind of origin, implementation-only import or SPI declaration,
 /// that restricts exporting \p decl from the given file and context.
 ///
 /// Local variant to swift::getDisallowedOriginKind for downgrade to warnings.
 DisallowedOriginKind
-getDisallowedOriginKind(const Decl *decl,
-                        const SourceFile &userSF,
-                        const Decl *userContext,
-                        DowngradeToWarning &downgradeToWarning) {
+swift::getDisallowedOriginKind(const Decl *decl,
+                               const SourceFile &userSF,
+                               const Decl *userContext,
+                               DowngradeToWarning &downgradeToWarning) {
   downgradeToWarning = DowngradeToWarning::No;
   ModuleDecl *M = decl->getModuleContext();
   if (userSF.isImportedImplementationOnly(M)) {
@@ -1493,6 +1486,8 @@ getDisallowedOriginKind(const Decl *decl,
   return DisallowedOriginKind::None;
 };
 
+namespace {
+
 // Diagnose public APIs exposing types that are either imported as
 // implementation-only or declared as SPI.
 class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
@@ -2015,6 +2010,8 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
 /// Diagnose declarations whose signatures refer to unavailable types.
 class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
   DeclContext *DC;
+  Optional<ExportabilityReason> ExportReason;
+  FragileFunctionKind FragileKind;
 
   void checkType(Type type, const TypeRepr *typeRepr, const Decl *context,
                  bool allowUnavailableProtocol=false) {
@@ -2036,12 +2033,14 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
       if (allowUnavailableProtocol)
         flags |= DeclAvailabilityFlag::AllowPotentiallyUnavailableProtocol;
 
-      diagnoseTypeReprAvailability(typeRepr, DC, flags);
+      diagnoseTypeReprAvailability(typeRepr, DC, ExportReason, FragileKind,
+                                   flags);
     }
 
     // Check the type for references to unavailable conformances.
     if (type)
-      diagnoseTypeAvailability(type, context->getLoc(), DC);
+      diagnoseTypeAvailability(type, context->getLoc(), DC,
+                               ExportReason, FragileKind);
   }
 
   void checkGenericParams(const GenericContext *ownerCtx,
@@ -2070,7 +2069,9 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
 
 public:
   explicit DeclAvailabilityChecker(Decl *D)
-    : DC(D->getInnermostDeclContext()) {}
+    : DC(D->getInnermostDeclContext()) {
+    FragileKind = DC->getFragileFunctionKind();
+  }
 
   void visit(Decl *D) {
     if (D->isImplicit())

lib/Sema/TypeCheckAccess.h

@@ -43,12 +43,26 @@ enum class DisallowedOriginKind : uint8_t {
   None
 };
 
+/// A uniquely-typed boolean to reduce the chances of accidentally inverting
+/// a check.
+///
+/// \see checkTypeAccess
+enum class DowngradeToWarning: bool {
+  No,
+  Yes
+};
+
 /// Returns the kind of origin, implementation-only import or SPI declaration,
 /// that restricts exporting \p decl from the given file and context.
 DisallowedOriginKind getDisallowedOriginKind(const Decl *decl,
                                              const SourceFile &userSF,
                                              const Decl *userContext);
 
+DisallowedOriginKind getDisallowedOriginKind(const Decl *decl,
+                                             const SourceFile &userSF,
+                                             const Decl *userContext,
+                                             DowngradeToWarning &downgradeToWarning);
+
 } // end namespace swift
 
 #endif