[Sema] Type-check the use and exposability of SPI decls

xymus · xymus · commit 2e73cb44fd57 · 2020-02-19T14:17:14.000-08:00
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -148,7 +148,7 @@ FIXIT(insert_type_qualification,"%0.",(Type))
 
 ERROR(candidate_inaccessible,none,
       "%0 is inaccessible due to "
-      "'%select{private|fileprivate|internal|%error|%error}1' protection level",
+      "'%select{private|fileprivate|internal|SPI|SPI}1' protection level",
       (DeclBaseName, AccessLevel))
 
 NOTE(note_candidate_inaccessible,none,
@@ -158,7 +158,7 @@ NOTE(note_candidate_inaccessible,none,
 
 ERROR(init_candidate_inaccessible,none,
       "%0 initializer is inaccessible due to "
-      "'%select{private|fileprivate|internal|%error|%error}1' protection level",
+      "'%select{private|fileprivate|internal|SPI|SPI}1' protection level",
       (Type, AccessLevel))
 
 ERROR(cannot_pass_rvalue_mutating_subelement,none,
@@ -1681,6 +1681,13 @@ ERROR(function_type_access,none,
       "because its %select{parameter|result}5 uses "
       "%select{a private|a fileprivate|an internal|%error|%error}3 type",
       (bool, AccessLevel, bool, AccessLevel, unsigned, bool))
+ERROR(function_type_spi,none,
+      "%select{function|method|initializer}0 "
+      "cannot be declared '@_spi' "
+      "because its %select{parameter|result}1 uses "
+      "%select{a private|a fileprivate|an internal|%error|%error}2 type "
+      "without a compatible '@_spi'",
+      (unsigned, bool, AccessLevel))
 WARNING(function_type_access_warn,none,
         "%select{function|method|initializer}4 "
         "%select{should be declared %select{private|fileprivate|internal|%error|%error}1"
@@ -2637,12 +2644,13 @@ NOTE(enum_raw_value_incrementing_from_zero,none,
 NOTE(construct_raw_representable_from_unwrapped_value,none,
      "construct %0 from unwrapped %1 value", (Type, Type))
 
-ERROR(decl_from_implementation_only_module,none,
+ERROR(decl_from_hidden_module,none,
       "cannot use %0 %1 %select{here|"
       "in an extension with public or '@usableFromInline' members|"
-      "in an extension with conditional conformances}2; %3 has been imported "
-      "as implementation-only",
-      (DescriptiveDeclKind, DeclName, unsigned, Identifier))
+      "in an extension with conditional conformances}2; "
+      "%select{%3 has been imported as implementation-only|"
+      "it is an SPI imported from %3}4",
+      (DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
 ERROR(conformance_from_implementation_only_module,none,
       "cannot use conformance of %0 to %1 %select{here|"
       "in an extension with public or '@usableFromInline' members|"
@@ -4593,10 +4601,16 @@ WARNING(resilience_decl_unavailable_warn,
         "should not be referenced from " FRAGILE_FUNC_KIND "3",
         (DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
 
-ERROR(inlinable_decl_ref_implementation_only,
+ERROR(resilience_decl_unavailable_spi,
+      none, DECL_OR_ACCESSOR "4 %1 is imported as SPI; "
+      "it cannot be referenced from " FRAGILE_FUNC_KIND "3",
+      (DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
+
+ERROR(inlinable_decl_ref_from_hidden_module,
       none, "%0 %1 cannot be used in " FRAGILE_FUNC_KIND "2 "
-      "because %3 was imported implementation-only",
-      (DescriptiveDeclKind, DeclName, unsigned, Identifier))
+      "because %select{%3 was imported implementation-only|"
+      "it is an SPI imported from %3}4",
+      (DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
 
 #undef FRAGILE_FUNC_KIND
 
diff --git a/include/swift/AST/SourceFile.h b/include/swift/AST/SourceFile.h
@@ -287,6 +287,9 @@ class SourceFile final : public FileUnit {
   lookupImportedSPIs(const ModuleDecl *importedModule,
                      SmallVectorImpl<Identifier> &spis) const override;
 
+  // Is \p targetDecl accessible as an explictly imported SPI from this file?
+  bool isImportedAsSPI(const ValueDecl *targetDecl) const;
+
   bool shouldCrossImport() const;
 
   /// Register a separately-imported overlay as shadowing the module that
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -3150,8 +3150,8 @@ static AccessLevel getMaximallyOpenAccessFor(const ValueDecl *decl) {
   return AccessLevel::Public;
 }
 
-/// Adjust \p access based on whether \p VD is \@usableFromInline or has been
-/// testably imported from \p useDC.
+/// Adjust \p access based on whether \p VD is \@usableFromInline, has been
+/// testably imported from \p useDC or \p VD is an imported SPI.
 ///
 /// \p access isn't always just `VD->getFormalAccess()` because this adjustment
 /// may be for a write, in which case the setter's access might be used instead.
@@ -3177,6 +3177,9 @@ static AccessLevel getAdjustedFormalAccess(const ValueDecl *VD,
     if (!useSF) return access;
     if (useSF->hasTestableOrPrivateImport(access, VD))
       return getMaximallyOpenAccessFor(VD);
+  } else if (VD->getAttrs().hasAttribute<SPIAccessControlAttr>()) {
+    // Restrict access to SPI decls.
+    return AccessLevel::Internal;
   }
 
   return access;
@@ -3267,7 +3270,7 @@ bool ValueDecl::hasOpenAccess(const DeclContext *useDC) const {
 
 /// Given the formal access level for using \p VD, compute the scope where
 /// \p VD may be accessed, taking \@usableFromInline, \@testable imports,
-/// and enclosing access levels into account.
+/// \@_spi imports, and enclosing access levels into account.
 ///
 /// \p access isn't always just `VD->getFormalAccess()` because this adjustment
 /// may be for a write, in which case the setter's access might be used instead.
@@ -3322,9 +3325,17 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
   case AccessLevel::Internal:
     return AccessScope(resultDC->getParentModule());
   case AccessLevel::Public:
-  case AccessLevel::Open:
+  case AccessLevel::Open: {
+    if (useDC) {
+      auto *useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
+      if (useSF &&
+          VD->getAttrs().hasAttribute<SPIAccessControlAttr>() &&
+          !useSF->isImportedAsSPI(VD))
+        return AccessScope(VD->getModuleContext(), /*private*/false);
+    }
     return AccessScope::getPublic();
   }
+  }
 
   llvm_unreachable("unknown access level");
 }
@@ -3356,8 +3367,8 @@ static bool checkAccessUsingAccessScopes(const DeclContext *useDC,
          AccessScope(useDC).isChildOf(accessScope);
 }
 
-/// Checks if \p VD may be used from \p useDC, taking \@testable imports into
-/// account.
+/// Checks if \p VD may be used from \p useDC, taking \@testable and \@_spi
+/// imports into account.
 ///
 /// When \p access is the same as `VD->getFormalAccess()` and the enclosing
 /// context of \p VD is usable from \p useDC, this ought to be the same as
@@ -3428,9 +3439,17 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
     return useSF && useSF->hasTestableOrPrivateImport(access, sourceModule);
   }
   case AccessLevel::Public:
-  case AccessLevel::Open:
+  case AccessLevel::Open: {
+    if (useDC) {
+      auto *useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
+      if (useSF &&
+          VD->getAttrs().hasAttribute<SPIAccessControlAttr>() &&
+          !useSF->isImportedAsSPI(VD))
+        return false;
+    }
     return true;
   }
+  }
   llvm_unreachable("bad access level");
 }
 
diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp
@@ -1793,6 +1793,23 @@ void SourceFile::lookupImportedSPIs(const ModuleDecl *importedModule,
   }
 }
 
+bool SourceFile::isImportedAsSPI(const ValueDecl *targetDecl) const {
+  if (!targetDecl->getAttrs().hasAttribute<SPIAccessControlAttr>())
+    return false;
+
+  auto targetModule = targetDecl->getModuleContext();
+  SmallVector<Identifier, 4> importedSpis;
+  lookupImportedSPIs(targetModule, importedSpis);
+
+  for (auto attr : targetDecl->getAttrs().getAttributes<SPIAccessControlAttr>())
+    for (auto declSPI : attr->getSPINames())
+      for (auto importedSPI : importedSpis)
+        if (importedSPI == declSPI)
+          return true;
+
+  return false;
+}
+
 bool SourceFile::shouldCrossImport() const {
   return Kind != SourceFileKind::SIL && Kind != SourceFileKind::Interface &&
          getASTContext().LangOpts.EnableCrossImportOverlays;
diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp
@@ -184,6 +184,11 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
   if (downgradeToWarning == DowngradeToWarning::Yes)
     diagID = diag::resilience_decl_unavailable_warn;
 
+  // If SPI, don't mention the access level.
+  const SourceFile *SF = DC->getParentSourceFile();
+  if (SF && SF->isImportedAsSPI(D))
+    diagID = diag::resilience_decl_unavailable_spi;
+
   Context.Diags.diagnose(
            loc, diagID,
            D->getDescriptiveKind(), diagName,
@@ -206,15 +211,19 @@ static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D,
                                       const SourceFile &userSF,
                                       FragileFunctionKind fragileKind) {
   auto definingModule = D->getModuleContext();
-  if (!userSF.isImportedImplementationOnly(definingModule))
+
+  bool isImplementationOnly =
+    userSF.isImportedImplementationOnly(definingModule);
+  if (!isImplementationOnly && !userSF.isImportedAsSPI(D))
     return false;
 
   // TODO: different diagnostics
   ASTContext &ctx = definingModule->getASTContext();
-  ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_implementation_only,
+  ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_from_hidden_module,
                      D->getDescriptiveKind(), D->getFullName(),
                      static_cast<unsigned>(fragileKind),
-                     definingModule->getName());
+                     definingModule->getName(),
+                     static_cast<unsigned>(!isImplementationOnly));
   return true;
 }
 
diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp
