Merge pull request swiftlang#34651 from slavapestov/assoc-conformance-availability

slavapestov · web-flow · commit fe1c4a4cbb2b · 2020-11-10T17:42:44.000-05:00
Check conformance availability for associated conformances
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -2758,12 +2758,8 @@ ERROR(conformance_from_implementation_only_module,none,
       "the conformance is declared as SPI in %3|"
       "the conformance is declared as SPI}4",
       (Type, Identifier, unsigned, Identifier, unsigned))
-ERROR(assoc_conformance_from_implementation_only_module,none,
-      "cannot use conformance of %0 to %1 in associated type %3 (inferred as "
-      "%4); %select{%2 has been imported as implementation-only|"
-      "the conformance is declared as SPI in %2|"
-      "the conformance is declared as SPI}5",
-      (Type, Identifier, Identifier, Type, Type, unsigned))
+NOTE(assoc_conformance_from_implementation_only_module,none,
+     "in associated type %0 (inferred as %1)", (Type, Type))
 ERROR(unexportable_clang_function_type,none,
       "cannot export the underlying C type of the function type %0; "
       "it may use anonymous types or types defined outside of a module",
diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp
@@ -222,6 +222,17 @@ ExportContext ExportContext::forFunctionBody(DeclContext *DC, SourceLoc loc) {
                        unavailablePlatformKind);
 }
 
+ExportContext ExportContext::forConformance(DeclContext *DC,
+                                            ProtocolDecl *proto) {
+  assert(isa<ExtensionDecl>(DC) || isa<NominalTypeDecl>(DC));
+  auto where = forDeclSignature(DC->getInnermostDeclarationDeclContext());
+
+  where.Exported &= proto->getFormalAccessScope(
+      DC, /*usableFromInlineAsPublic*/true).isPublic();
+
+  return where;
+}
+
 ExportContext ExportContext::withReason(ExportabilityReason reason) const {
   auto copy = *this;
   copy.Reason = unsigned(reason);
@@ -1226,7 +1237,8 @@ static const Decl *ancestorMemberLevelDeclForAvailabilityFixit(const Decl *D) {
   while (D) {
     D = relatedDeclForAvailabilityFixit(D);
 
-    if (D->getDeclContext()->isTypeContext() &&
+    if (!D->isImplicit() &&
+        D->getDeclContext()->isTypeContext() &&
         DeclAttribute::canAttributeAppearOnDecl(DeclAttrKind::DAK_Available,
                                                 D)) {
       break;
@@ -2172,20 +2184,19 @@ void TypeChecker::diagnoseIfDeprecated(SourceRange ReferenceRange,
   }
 }
 
-void TypeChecker::diagnoseIfDeprecated(
-    SourceLoc loc,
-    const RootProtocolConformance *rootConf,
-    const ExtensionDecl *ext,
-    const ExportContext &where) {
+bool TypeChecker::diagnoseIfDeprecated(SourceLoc loc,
+                                       const RootProtocolConformance *rootConf,
+                                       const ExtensionDecl *ext,
+                                       const ExportContext &where) {
   const AvailableAttr *attr = TypeChecker::getDeprecated(ext);
   if (!attr)
-    return;
+    return false;
 
   // We match the behavior of clang to not report deprecation warnings
   // inside declarations that are themselves deprecated on all deployment
   // targets.
   if (where.isDeprecated()) {
-    return;
+    return false;
   }
 
   auto *dc = where.getDeclContext();
@@ -2196,7 +2207,7 @@ void TypeChecker::diagnoseIfDeprecated(
       // Suppress a deprecation warning if the availability checking machinery
       // thinks the reference program location will not execute on any
       // deployment target for the current platform.
-      return;
+      return false;
     }
   }
 
@@ -2215,7 +2226,7 @@ void TypeChecker::diagnoseIfDeprecated(
              attr->Deprecated.hasValue(), deprecatedVersion,
              /*message*/ StringRef())
         .highlight(attr->getRange());
-    return;
+    return true;
   }
 
   EncodedDiagnosticMessage encodedMessage(attr->Message);
@@ -2225,6 +2236,7 @@ void TypeChecker::diagnoseIfDeprecated(
       attr->Deprecated.hasValue(), deprecatedVersion,
       encodedMessage.Message)
     .highlight(attr->getRange());
+  return true;
 }
 
 void swift::diagnoseUnavailableOverride(ValueDecl *override,
@@ -3374,7 +3386,8 @@ void swift::diagnoseTypeAvailability(const TypeRepr *TR, Type T, SourceLoc loc,
 bool
 swift::diagnoseConformanceAvailability(SourceLoc loc,
                                        ProtocolConformanceRef conformance,
-                                       const ExportContext &where) {
+                                       const ExportContext &where,
+                                       Type depTy, Type replacementTy) {
   assert(!where.isImplicit());
 
   if (!conformance.isConcrete())
@@ -3385,29 +3398,55 @@ swift::diagnoseConformanceAvailability(SourceLoc loc,
 
   auto *DC = where.getDeclContext();
 
+  auto maybeEmitAssociatedTypeNote = [&]() {
+    if (!depTy && !replacementTy)
+      return;
+
+    Type selfTy = rootConf->getProtocol()->getProtocolSelfType();
+    if (!depTy->isEqual(selfTy)) {
+      auto &ctx = DC->getASTContext();
+      ctx.Diags.diagnose(
+          loc,
+          diag::assoc_conformance_from_implementation_only_module,
+          depTy, replacementTy->getCanonicalType());
+    }
+  };
+
   if (auto *ext = dyn_cast<ExtensionDecl>(rootConf->getDeclContext())) {
-    if (TypeChecker::diagnoseConformanceExportability(loc, rootConf, ext, where))
+    if (TypeChecker::diagnoseConformanceExportability(loc, rootConf, ext, where)) {
+      maybeEmitAssociatedTypeNote();
       return true;
+    }
 
-    if (diagnoseExplicitUnavailability(loc, rootConf, ext, where))
+    if (diagnoseExplicitUnavailability(loc, rootConf, ext, where)) {
+      maybeEmitAssociatedTypeNote();
       return true;
-
-    // Diagnose for deprecation
-    TypeChecker::diagnoseIfDeprecated(loc, rootConf, ext, where);
+    }
 
     // Diagnose (and possibly signal) for potential unavailability
     auto maybeUnavail = TypeChecker::checkConformanceAvailability(
         rootConf, ext, where);
     if (maybeUnavail.hasValue()) {
       TypeChecker::diagnosePotentialUnavailability(rootConf, ext, loc, DC,
                                                    maybeUnavail.getValue());
+      maybeEmitAssociatedTypeNote();
+      return true;
+    }
+
+    // Diagnose for deprecation
+    if (TypeChecker::diagnoseIfDeprecated(loc, rootConf, ext, where)) {
+      maybeEmitAssociatedTypeNote();
+
+      // Deprecation is just a warning, so keep going with checking the
+      // substitution map below.
     }
   }
 
   // Now, check associated conformances.
   SubstitutionMap subConformanceSubs =
       concreteConf->getSubstitutions(DC->getParentModule());
-  if (diagnoseSubstitutionMapAvailability(loc, subConformanceSubs, where))
+  if (diagnoseSubstitutionMapAvailability(loc, subConformanceSubs, where,
+                                          depTy, replacementTy))
     return true;
 
   return false;
@@ -3416,10 +3455,12 @@ swift::diagnoseConformanceAvailability(SourceLoc loc,
 bool
 swift::diagnoseSubstitutionMapAvailability(SourceLoc loc,
                                            SubstitutionMap subs,
-                                           const ExportContext &where) {
+                                           const ExportContext &where,
+                                           Type depTy, Type replacementTy) {
   bool hadAnyIssues = false;
   for (ProtocolConformanceRef conformance : subs.getConformances()) {
-    if (diagnoseConformanceAvailability(loc, conformance, where))
+    if (diagnoseConformanceAvailability(loc, conformance, where,
+                                        depTy, replacementTy))
       hadAnyIssues = true;
   }
   return hadAnyIssues;
diff --git a/lib/Sema/TypeCheckAvailability.h b/lib/Sema/TypeCheckAvailability.h
@@ -129,6 +129,11 @@ class ExportContext {
   /// it can reference anything.
   static ExportContext forFunctionBody(DeclContext *DC, SourceLoc loc);
 
+  /// Create an instance describing associated conformances that can be
+  /// referenced from the the conformance defined by the given DeclContext,
+  /// which must be a NominalTypeDecl or ExtensionDecl.
+  static ExportContext forConformance(DeclContext *DC, ProtocolDecl *proto);
+
   /// Produce a new context with the same properties as this one, except
   /// changing the ExportabilityReason. This only affects diagnostics.
   ExportContext withReason(ExportabilityReason reason) const;
@@ -212,12 +217,16 @@ void diagnoseTypeAvailability(const TypeRepr *TR, Type T, SourceLoc loc,
 bool
 diagnoseConformanceAvailability(SourceLoc loc,
                                 ProtocolConformanceRef conformance,
-                                const ExportContext &context);
+                                const ExportContext &context,
+                                Type depTy=Type(),
+                                Type replacementTy=Type());
 
 bool
 diagnoseSubstitutionMapAvailability(SourceLoc loc,
                                     SubstitutionMap subs,
-                                    const ExportContext &context);
+                                    const ExportContext &context,
+                                    Type depTy=Type(),
+                                    Type replacementTy=Type());
 
 /// Diagnose uses of unavailable declarations. Returns true if a diagnostic
 /// was emitted.
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -4308,56 +4308,6 @@ ResolveWitnessResult ConformanceChecker::resolveTypeWitnessViaLookup(
   return ResolveWitnessResult::ExplicitFailed;
 }
 
-static void checkExportability(Type depTy, Type replacementTy,
-                               const ProtocolConformance *conformance,
-                               NormalProtocolConformance *conformanceBeingChecked,
-                               DeclContext *DC) {
-  SourceFile *SF = DC->getParentSourceFile();
-  if (!SF)
-    return;
-
-  SubstitutionMap subs =
-      conformance->getSubstitutions(SF->getParentModule());
-  for (auto &subConformance : subs.getConformances()) {
-    if (!subConformance.isConcrete())
-      continue;
-    checkExportability(depTy, replacementTy, subConformance.getConcrete(),
-                       conformanceBeingChecked, DC);
-  }
-
-  const RootProtocolConformance *rootConformance =
-      conformance->getRootConformance();
-  ModuleDecl *M = rootConformance->getDeclContext()->getParentModule();
-
-  auto where = ExportContext::forDeclSignature(
-      DC->getInnermostDeclarationDeclContext());
-  auto originKind = getDisallowedOriginKind(
-      rootConformance->getDeclContext()->getAsDecl(),
-      where);
-  if (originKind == DisallowedOriginKind::None)
-    return;
-
-  ASTContext &ctx = SF->getASTContext();
-
-  Type selfTy = rootConformance->getProtocol()->getProtocolSelfType();
-  if (depTy->isEqual(selfTy)) {
-    ctx.Diags.diagnose(
-        conformanceBeingChecked->getLoc(),
-        diag::conformance_from_implementation_only_module,
-        rootConformance->getType(),
-        rootConformance->getProtocol()->getName(), 0, M->getName(),
-        static_cast<unsigned>(originKind));
-  } else {
-    ctx.Diags.diagnose(
-        conformanceBeingChecked->getLoc(),
-        diag::assoc_conformance_from_implementation_only_module,
-        rootConformance->getType(),
-        rootConformance->getProtocol()->getName(), M->getName(),
-        depTy, replacementTy->getCanonicalType(),
-        static_cast<unsigned>(originKind));
-  }
-}
-
 void ConformanceChecker::ensureRequirementsAreSatisfied() {
   Conformance->finishSignatureConformances();
   auto proto = Conformance->getProtocol();
@@ -4403,18 +4353,21 @@ void ConformanceChecker::ensureRequirementsAreSatisfied() {
 
   // Now check that our associated conformances are at least as visible as
   // the conformance itself.
-  if (getRequiredAccessScope().isPublic() || isUsableFromInlineRequired()) {
-    for (auto req : proto->getRequirementSignature()) {
-      if (req.getKind() == RequirementKind::Conformance) {
-        auto depTy = req.getFirstType();
-        auto *proto = req.getSecondType()->castTo<ProtocolType>()->getDecl();
-        auto conformance = Conformance->getAssociatedConformance(depTy, proto);
-        if (conformance.isConcrete()) {
-          auto *concrete = conformance.getConcrete();
-          auto replacementTy = DC->mapTypeIntoContext(concrete->getType());
-          checkExportability(depTy, replacementTy, concrete,
-                             Conformance, DC);
-        }
+  auto where = ExportContext::forConformance(DC, proto);
+  if (where.isImplicit())
+    return;
+
+  for (auto req : proto->getRequirementSignature()) {
+    if (req.getKind() == RequirementKind::Conformance) {
+      auto depTy = req.getFirstType();
+      auto *proto = req.getSecondType()->castTo<ProtocolType>()->getDecl();
+      auto conformance = Conformance->getAssociatedConformance(depTy, proto);
+      if (conformance.isConcrete()) {
+        auto *concrete = conformance.getConcrete();
+        auto replacementTy = DC->mapTypeIntoContext(concrete->getType());
+        diagnoseConformanceAvailability(Conformance->getLoc(),
+                                        conformance, where,
+                                        depTy, replacementTy);
       }
     }
   }
diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
@@ -1066,10 +1066,10 @@ void diagnoseIfDeprecated(SourceRange SourceRange,
                           const ApplyExpr *Call);
 
 /// Emits a diagnostic for a reference to a conformnace that is deprecated.
-void diagnoseIfDeprecated(SourceLoc Loc,
-                          const RootProtocolConformance *DeprecatedConf,
-                          const ExtensionDecl *Ext,
-                          const ExportContext &Where);
+bool diagnoseIfDeprecated(SourceLoc loc,
+                          const RootProtocolConformance *rootConf,
+                          const ExtensionDecl *ext,
+                          const ExportContext &where);
 /// @}
 
 /// If LangOptions::DebugForbidTypecheckPrefix is set and the given decl
diff --git a/test/SPI/protocol_requirement.swift b/test/SPI/protocol_requirement.swift
@@ -104,7 +104,8 @@ public protocol Proto {
 
 public struct BadStruct {}
 @_spi(Horse) extension BadStruct : OtherProto {}
-public struct BadConforms : Proto { // expected-error {{cannot use conformance of 'BadStruct' to 'OtherProto' in associated type 'Self.A.Element' (inferred as 'BadStruct'); the conformance is declared as SPI}}
+public struct BadConforms : Proto { // expected-error {{cannot use conformance of 'BadStruct' to 'OtherProto' here; the conformance is declared as SPI}}
+// expected-note@-1 {{in associated type 'Self.A.Element' (inferred as 'BadStruct')}}
   public typealias A = [BadStruct]
 }
 
diff --git a/test/Sema/conformance_availability.swift b/test/Sema/conformance_availability.swift
@@ -16,7 +16,7 @@ public struct HasUnavailableConformance1 {}
 
 @available(*, unavailable)
 extension HasUnavailableConformance1 : Horse {}
-// expected-note@-1 6{{conformance of 'HasUnavailableConformance1' to 'Horse' has been explicitly marked unavailable here}}
+// expected-note@-1 7{{conformance of 'HasUnavailableConformance1' to 'Horse' has been explicitly marked unavailable here}}
 
 func passUnavailableConformance1(x: HasUnavailableConformance1) {
   takesHorse(x) // expected-error {{conformance of 'HasUnavailableConformance1' to 'Horse' is unavailable}}
@@ -212,4 +212,51 @@ func passAvailableConformance1a(x: HasAvailableConformance1) {
   takesHorse(x)
   x.giddyUp()
   _ = UsesHorse<HasAvailableConformance1>.self
+}
+
+// Associated conformance with unavailability
+protocol Rider {
+	associatedtype H : Horse
+}
+
+struct AssocConformanceUnavailable : Rider {
+// expected-error@-1 {{conformance of 'HasUnavailableConformance1' to 'Horse' is unavailable}}
+// expected-note@-2 {{in associated type 'Self.H' (inferred as 'HasUnavailableConformance1')}}
+	typealias H = HasUnavailableConformance1
+}
+
+// Associated conformance with deprecation
+struct AssocConformanceDeprecated : Rider {
+// expected-warning@-1 {{conformance of 'HasDeprecatedConformance1' to 'Horse' is deprecated}}
+// expected-note@-2 {{in associated type 'Self.H' (inferred as 'HasDeprecatedConformance1')}}
+	typealias H = HasDeprecatedConformance1
+}
+
+// Associated conformance with availability
+struct AssocConformanceAvailable1 : Rider {
+// expected-error@-1 {{conformance of 'HasAvailableConformance1' to 'Horse' is only available in macOS 100 or newer}}
+// expected-note@-2 {{in associated type 'Self.H' (inferred as 'HasAvailableConformance1')}}
+// expected-note@-3 {{add @available attribute to enclosing struct}}
+	typealias H = HasAvailableConformance1
+}
+
+@available(macOS 100, *)
+struct AssocConformanceAvailable2 : Rider {
+	typealias H = HasAvailableConformance1
+}
+
+struct AssocConformanceAvailable3 {}
+
+extension AssocConformanceAvailable3 : Rider {
+// expected-error@-1 {{conformance of 'HasAvailableConformance1' to 'Horse' is only available in macOS 100 or newer}}
+// expected-note@-2 {{in associated type 'Self.H' (inferred as 'HasAvailableConformance1')}}
+// expected-note@-3 {{add @available attribute to enclosing extension}}
+	typealias H = HasAvailableConformance1
+}
+
+struct AssocConformanceAvailable4 {}
+
+@available(macOS 100, *)
+extension AssocConformanceAvailable4 : Rider {
+	typealias H = HasAvailableConformance1
 }
diff --git a/test/Sema/implementation-only-import-in-decls.swift b/test/Sema/implementation-only-import-in-decls.swift
diff --git a/test/Sema/spi-in-decls.swift b/test/Sema/spi-in-decls.swift

Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,8 @@ public protocol Proto {`
`104`	`104`
`105`	`105`	`public struct BadStruct {}`
`106`	`106`	`@_spi(Horse) extension BadStruct : OtherProto {}`
`107`		`-public struct BadConforms : Proto { // expected-error {{cannot use conformance of 'BadStruct' to 'OtherProto' in associated type 'Self.A.Element' (inferred as 'BadStruct'); the conformance is declared as SPI}}`
	`107`	`+public struct BadConforms : Proto { // expected-error {{cannot use conformance of 'BadStruct' to 'OtherProto' here; the conformance is declared as SPI}}`
	`108`	`+// expected-note@-1 {{in associated type 'Self.A.Element' (inferred as 'BadStruct')}}`
`108`	`109`	`public typealias A = [BadStruct]`
`109`	`110`	`}`
`110`	`111`