Merge pull request #23949 from jrose-apple/5.1-conformance-conformance

[5.1] Implementation-only import checking for conformances

Author: jrose-apple

include/swift/AST/AccessScopeChecker.h

@@ -54,13 +54,13 @@ class TypeDeclFinder : public TypeWalker {
   Action walkToTypePre(Type T) override;
 
 public:
-  virtual Action visitNominalType(const NominalType *ty) {
+  virtual Action visitNominalType(NominalType *ty) {
     return Action::Continue;
   }
-  virtual Action visitBoundGenericType(const BoundGenericType *ty) {
+  virtual Action visitBoundGenericType(BoundGenericType *ty) {
     return Action::Continue;
   }
-  virtual Action visitTypeAliasType(const TypeAliasType *ty) {
+  virtual Action visitTypeAliasType(TypeAliasType *ty) {
     return Action::Continue;
   }
 };
@@ -72,9 +72,9 @@ class SimpleTypeDeclFinder : public TypeDeclFinder {
   /// The function to call when a ComponentIdentTypeRepr is seen.
   llvm::function_ref<Action(const TypeDecl *)> Callback;
 
-  Action visitNominalType(const NominalType *ty) override;
-  Action visitBoundGenericType(const BoundGenericType *ty) override;
-  Action visitTypeAliasType(const TypeAliasType *ty) override;
+  Action visitNominalType(NominalType *ty) override;
+  Action visitBoundGenericType(BoundGenericType *ty) override;
+  Action visitTypeAliasType(TypeAliasType *ty) override;
 
 public:
   explicit SimpleTypeDeclFinder(

include/swift/AST/DiagnosticsSema.def

@@ -2396,6 +2396,14 @@ ERROR(decl_from_implementation_only_module,none,
       "cannot use %0 here; %1 has been imported as "
       "'@_implementationOnly'",
       (DeclName, Identifier))
+ERROR(conformance_from_implementation_only_module,none,
+      "cannot use conformance of %0 to %1 here; %2 has been imported as "
+      "'@_implementationOnly'",
+      (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'",
+      (Type, DeclName, Identifier, Type, Type))
 
 // Derived conformances
 ERROR(cannot_synthesize_init_in_extension_of_nonfinal,none,

lib/AST/AccessScopeChecker.cpp

@@ -71,17 +71,17 @@ TypeWalker::Action TypeDeclFinder::walkToTypePre(Type T) {
 }
 
 TypeWalker::Action
-SimpleTypeDeclFinder::visitNominalType(const NominalType *ty) {
+SimpleTypeDeclFinder::visitNominalType(NominalType *ty) {
   return Callback(ty->getDecl());
 }
 
 TypeWalker::Action
-SimpleTypeDeclFinder::visitBoundGenericType(const BoundGenericType *ty) {
+SimpleTypeDeclFinder::visitBoundGenericType(BoundGenericType *ty) {
   return Callback(ty->getDecl());
 }
 
 TypeWalker::Action
-SimpleTypeDeclFinder::visitTypeAliasType(const TypeAliasType *ty) {
+SimpleTypeDeclFinder::visitTypeAliasType(TypeAliasType *ty) {
   return Callback(ty->getDecl());
 }
 

lib/Sema/ResilienceDiagnostics.cpp

@@ -15,10 +15,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "TypeChecker.h"
+#include "TypeCheckAvailability.h"
+#include "swift/AST/AccessScopeChecker.h"
 #include "swift/AST/Attr.h"
 #include "swift/AST/Decl.h"
-#include "swift/AST/Initializer.h"
 #include "swift/AST/DeclContext.h"
+#include "swift/AST/Initializer.h"
+#include "swift/AST/ProtocolConformance.h"
 
 using namespace swift;
 using FragileFunctionKind = TypeChecker::FragileFunctionKind;
@@ -99,16 +102,13 @@ enum class DowngradeToWarning: bool {
 };
 
 bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
-                                           const ValueDecl *D,
+                                           ConcreteDeclRef declRef,
                                            const DeclContext *DC,
                                            FragileFunctionKind Kind,
                                            bool TreatUsableFromInlineAsPublic) {
+  const ValueDecl *D = declRef.getDecl();
   // Do some important fast-path checks that apply to all cases.
 
-  // Local declarations are OK.
-  if (D->getDeclContext()->isLocalContext())
-    return false;
-
   // Type parameters are OK.
   if (isa<AbstractTypeParamDecl>(D))
     return false;
@@ -119,7 +119,7 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
     return true;
 
   // Check whether the declaration comes from a publically-imported module.
-  if (diagnoseDeclRefExportability(loc, D, DC))
+  if (diagnoseDeclRefExportability(loc, declRef, DC))
     return true;
 
   return false;
@@ -130,6 +130,10 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
                                            const DeclContext *DC,
                                            FragileFunctionKind Kind,
                                            bool TreatUsableFromInlineAsPublic) {
+  // Local declarations are OK.
+  if (D->getDeclContext()->isLocalContext())
+    return false;
+
   // Public declarations are OK.
   if (D->getFormalAccessScope(/*useDC=*/nullptr,
                               TreatUsableFromInlineAsPublic).isPublic())
@@ -203,8 +207,89 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
   return (downgradeToWarning == DowngradeToWarning::No);
 }
 
+static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D,
+                                      const SourceFile &userSF) {
+  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());
+  return true;
+}
+
+static bool
+diagnoseGenericArgumentsExportability(SourceLoc loc,
+                                      const SubstitutionMap &subs,
+                                      const SourceFile &userSF) {
+  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);
+
+    const RootProtocolConformance *rootConf =
+        concreteConf->getRootConformance();
+    ModuleDecl *M = rootConf->getDeclContext()->getParentModule();
+    if (!userSF.isImportedImplementationOnly(M))
+      continue;
+
+    ASTContext &ctx = M->getASTContext();
+    ctx.Diags.diagnose(loc, diag::conformance_from_implementation_only_module,
+                       rootConf->getType(),
+                       rootConf->getProtocol()->getFullName(), M->getName());
+    hadAnyIssues = true;
+  }
+  return hadAnyIssues;
+}
+
+void TypeChecker::diagnoseGenericTypeExportability(const TypeLoc &TL,
+                                                   const DeclContext *DC) {
+  class GenericTypeFinder : public TypeDeclFinder {
+    using Callback = llvm::function_ref<void(SubstitutionMap)>;
+
+    const SourceFile &SF;
+    Callback callback;
+  public:
+    GenericTypeFinder(const SourceFile &SF, Callback callback)
+        : SF(SF), callback(callback) {}
+
+    Action visitBoundGenericType(BoundGenericType *ty) override {
+      ModuleDecl *useModule = SF.getParentModule();
+      SubstitutionMap subs = ty->getContextSubstitutionMap(useModule,
+                                                           ty->getDecl());
+      callback(subs);
+      return Action::Continue;
+    }
+
+    Action visitTypeAliasType(TypeAliasType *ty) override {
+      callback(ty->getSubstitutionMap());
+      return Action::Continue;
+    }
+  };
+
+  assert(TL.getType() && "type not validated yet");
+
+  const SourceFile *SF = DC->getParentSourceFile();
+  if (!SF)
+    return;
+
+  TL.getType().walk(GenericTypeFinder(*SF, [&](SubstitutionMap subs) {
+    // 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.
+    (void)diagnoseGenericArgumentsExportability(TL.getLoc(), subs, *SF);
+  }));
+}
+
 bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
-                                               const ValueDecl *D,
+                                               ConcreteDeclRef declRef,
                                                const DeclContext *DC) {
   // We're only interested in diagnosing uses from source files.
   auto userSF = DC->getParentSourceFile();
@@ -219,22 +304,12 @@ bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
   if (!userSF->hasImplementationOnlyImports())
     return false;
 
-  auto userModule = userSF->getParentModule();
-  auto definingModule = D->getModuleContext();
-
-  // Nothing to diagnose in the very common case of the same module.
-  if (userModule == definingModule)
-    return false;
-
-  // Nothing to diagnose in the very common case that the module is
-  // imported for use in signatures.
-  if (!userSF->isImportedImplementationOnly(definingModule))
-    return false;
-
-  // TODO: different diagnostics
-  diagnose(loc, diag::inlinable_decl_ref_implementation_only,
-           D->getDescriptiveKind(), D->getFullName());
-
-  // TODO: notes explaining why
-  return true;
+  const ValueDecl *D = declRef.getDecl();
+  if (diagnoseDeclExportability(loc, D, *userSF))
+    return true;
+  if (diagnoseGenericArgumentsExportability(loc, declRef.getSubstitutions(),
+                                            *userSF)) {
+    return true;
+  }
+  return false;
 }