Merge pull request #20384 from jrose-apple/witless

Require @usableFromInline on associated type witnesses

Author: jrose-apple

include/swift/AST/DiagnosticsSema.def

@@ -1691,6 +1691,14 @@ ERROR(type_witness_not_accessible_type,none,
       "%0 %1 must be as accessible as its enclosing type because it "
       "matches a requirement in protocol %3",
       (DescriptiveDeclKind, DeclName, AccessLevel, DeclName))
+ERROR(witness_not_usable_from_inline,none,
+      "%0 %1 must be declared '@usableFromInline' because "
+      "because it matches a requirement in protocol %2",
+      (DescriptiveDeclKind, DeclName, DeclName))
+WARNING(witness_not_usable_from_inline_warn,none,
+        "%0 %1 should be declared '@usableFromInline' because "
+        "because it matches a requirement in protocol %2",
+        (DescriptiveDeclKind, DeclName, DeclName))
 ERROR(type_witness_objc_generic_parameter,none,
       "type %0 involving Objective-C type parameter%select{| %1}2 cannot be "
       "used for associated type %3 of protocol %4",

lib/Sema/TypeCheckProtocol.cpp

@@ -18,7 +18,6 @@
 #include "ConstraintSystem.h"
 #include "DerivedConformances.h"
 #include "MiscDiagnostics.h"
-#include "TypeChecker.h"
 #include "TypeCheckAvailability.h"
 #include "swift/Basic/SourceManager.h"
 #include "swift/Basic/StringExtras.h"
@@ -1233,23 +1232,51 @@ bool WitnessChecker::findBestWitness(
   return isReallyBest;
 }
 
-bool WitnessChecker::checkWitnessAccess(AccessScope &requiredAccessScope,
-                                        ValueDecl *requirement,
+AccessScope WitnessChecker::getRequiredAccessScope() {
+  if (RequiredAccessScopeAndUsableFromInline.hasValue())
+    return RequiredAccessScopeAndUsableFromInline.getValue().first;
+
+  AccessScope result = Proto->getFormalAccessScope(DC);
+
+  bool witnessesMustBeUsableFromInline = false;
+  if (Adoptee) {
+    const NominalTypeDecl *adoptingNominal = Adoptee->getAnyNominal();
+
+    // Compute the intersection of the conforming type's access scope
+    // and the protocol's access scope.
+    auto scopeIntersection =
+        result.intersectWith(adoptingNominal->getFormalAccessScope(DC));
+    assert(scopeIntersection.hasValue());
+    result = scopeIntersection.getValue();
+
+    if (!result.isPublic()) {
+      witnessesMustBeUsableFromInline =
+          Proto->getFormalAccessScope(
+            DC, /*usableFromInlineAsPublic*/true).isPublic() &&
+          adoptingNominal->getFormalAccessScope(
+            DC, /*usableFromInlineAsPublic*/true).isPublic();
+    }
+  } else {
+    if (!result.isPublic()) {
+      witnessesMustBeUsableFromInline =
+          Proto->getFormalAccessScope(
+            DC, /*usableFromInlineAsPublic*/true).isPublic();
+    }
+  }
+
+  RequiredAccessScopeAndUsableFromInline =
+      std::make_pair(result, witnessesMustBeUsableFromInline);
+  return result;
+}
+
+bool WitnessChecker::checkWitnessAccess(ValueDecl *requirement,
                                         ValueDecl *witness,
                                         bool *isSetter) {
   *isSetter = false;
   if (!TC.getLangOpts().EnableAccessControl)
     return false;
 
-  // Compute the intersection of the conforming type's access scope
-  // and the protocol's access scope.
-  auto scopeIntersection =
-    requiredAccessScope.intersectWith(Proto->getFormalAccessScope(DC));
-  assert(scopeIntersection.hasValue());
-
-  requiredAccessScope = *scopeIntersection;
-
-  AccessScope actualScopeToCheck = requiredAccessScope;
+  AccessScope actualScopeToCheck = getRequiredAccessScope();
 
   // Setting the 'forConformance' flag means that we admit witnesses in
   // protocol extensions that we can see, but are not necessarily as
@@ -1270,7 +1297,7 @@ bool WitnessChecker::checkWitnessAccess(AccessScope &requiredAccessScope,
       }
     }
 
-    if (actualScopeToCheck.hasEqualDeclContextWith(requiredAccessScope))
+    if (actualScopeToCheck.hasEqualDeclContextWith(getRequiredAccessScope()))
       return true;
   }
 
@@ -1297,20 +1324,17 @@ checkWitnessAvailability(ValueDecl *requirement,
                                                DC, *requiredAvailability));
 }
 
-RequirementCheck WitnessChecker::
-checkWitness(AccessScope requiredAccessScope,
-             ValueDecl *requirement,
-             const RequirementMatch &match) {
+RequirementCheck WitnessChecker::checkWitness(ValueDecl *requirement,
+                                              const RequirementMatch &match) {
   if (!match.OptionalAdjustments.empty())
     return CheckKind::OptionalityConflict;
 
   bool isSetter = false;
-  if (checkWitnessAccess(requiredAccessScope, requirement, match.Witness,
-                         &isSetter)) {
+  if (checkWitnessAccess(requirement, match.Witness, &isSetter)) {
     CheckKind kind = (isSetter
                       ? CheckKind::AccessOfSetter
                       : CheckKind::Access);
-    return RequirementCheck(kind, requiredAccessScope);
+    return RequirementCheck(kind, getRequiredAccessScope());
   }
 
   auto requiredAvailability = AvailabilityContext::alwaysAvailable();
@@ -2340,6 +2364,36 @@ bool ConformanceChecker::checkObjCTypeErasedGenerics(
   return true;
 }
 
+namespace {
+/// Helper class for use with ConformanceChecker::diagnoseOrDefer when a witness
+/// needs to be marked as '\@usableFromInline'.
+class DiagnoseUsableFromInline {
+  const ValueDecl *witness;
+
+public:
+  explicit DiagnoseUsableFromInline(const ValueDecl *witness)
+      : witness(witness) {
+    assert(witness);
+  }
+
+  void operator()(const NormalProtocolConformance *conformance) {
+    auto proto = conformance->getProtocol();
+    ASTContext &ctx = proto->getASTContext();
+
+    auto diagID = diag::witness_not_usable_from_inline;
+    if (!ctx.isSwiftVersionAtLeast(5))
+      diagID = diag::witness_not_usable_from_inline_warn;
+
+    SourceLoc diagLoc = getLocForDiagnosingWitness(conformance, witness);
+    ctx.Diags.diagnose(diagLoc, diagID,
+                       witness->getDescriptiveKind(),
+                       witness->getFullName(),
+                       proto->getName());
+    emitDeclaredHereIfNeeded(ctx.Diags, diagLoc, witness);
+  }
+};
+}
+
 void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType,
                                            Type type,
                                            TypeDecl *typeDecl) {
@@ -2357,24 +2411,20 @@ void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType,
 
   if (typeDecl) {
     // Check access.
-    AccessScope requiredAccessScope =
-        Adoptee->getAnyNominal()->getFormalAccessScope(DC);
     bool isSetter = false;
-    if (checkWitnessAccess(requiredAccessScope, assocType, typeDecl,
-                           &isSetter)) {
+    if (checkWitnessAccess(assocType, typeDecl, &isSetter)) {
       assert(!isSetter);
 
       // Avoid relying on the lifetime of 'this'.
       const DeclContext *DC = this->DC;
       diagnoseOrDefer(assocType, false,
-        [DC, typeDecl, requiredAccessScope](
-          NormalProtocolConformance *conformance) {
+          [this, DC, typeDecl](NormalProtocolConformance *conformance) {
         AccessLevel requiredAccess =
-          requiredAccessScope.requiredAccessForDiagnostics();
+            getRequiredAccessScope().requiredAccessForDiagnostics();
         auto proto = conformance->getProtocol();
         auto protoAccessScope = proto->getFormalAccessScope(DC);
         bool protoForcesAccess =
-          requiredAccessScope.hasEqualDeclContextWith(protoAccessScope);
+            getRequiredAccessScope().hasEqualDeclContextWith(protoAccessScope);
         auto diagKind = protoForcesAccess
                           ? diag::type_witness_not_accessible_proto
                           : diag::type_witness_not_accessible_type;
@@ -2391,6 +2441,13 @@ void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType,
         fixItAccess(fixItDiag, typeDecl, requiredAccess);
       });
     }
+
+    if (isUsableFromInlineRequired()) {
+      bool witnessIsUsableFromInline = typeDecl->getFormalAccessScope(
+          DC, /*usableFromInlineAsPublic*/true).isPublic();
+      if (!witnessIsUsableFromInline)
+        diagnoseOrDefer(assocType, false, DiagnoseUsableFromInline(typeDecl));
+    }
   } else {
     // If there was no type declaration, synthesize one.
     auto aliasDecl = new (TC.Context) TypeAliasDecl(SourceLoc(),
@@ -2939,8 +2996,7 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
         });
     }
 
-    auto nominalAccessScope = nominal->getFormalAccessScope(DC);
-    auto check = checkWitness(nominalAccessScope, requirement, best);
+    auto check = checkWitness(requirement, best);
 
     switch (check.Kind) {
     case CheckKind::Success:
@@ -5422,7 +5478,7 @@ DefaultWitnessChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
 
     // Perform the same checks as conformance witness matching, but silently
     // ignore the candidate instead of diagnosing anything.
-    auto check = checkWitness(AccessScope::getPublic(), requirement, best);
+    auto check = checkWitness(requirement, best);
     if (check.Kind != CheckKind::Success)
       return ResolveWitnessResult::ExplicitFailed;
 

lib/Sema/TypeCheckProtocol.h

@@ -18,6 +18,8 @@
 #ifndef SWIFT_SEMA_PROTOCOL_H
 #define SWIFT_SEMA_PROTOCOL_H
 
+#include "TypeChecker.h"
+#include "swift/AST/AccessScope.h"
 #include "swift/AST/RequirementEnvironment.h"
 #include "swift/AST/Type.h"
 #include "swift/AST/Types.h"
@@ -475,11 +477,24 @@ class WitnessChecker {
 
   RequirementEnvironmentCache ReqEnvironmentCache;
 
+  Optional<std::pair<AccessScope, bool>> RequiredAccessScopeAndUsableFromInline;
+
   WitnessChecker(TypeChecker &tc, ProtocolDecl *proto,
                  Type adoptee, DeclContext *dc);
 
   bool isMemberOperator(FuncDecl *decl, Type type);
 
+  AccessScope getRequiredAccessScope();
+
+  bool isUsableFromInlineRequired() {
+    if (!TC.getLangOpts().EnableAccessControl)
+      return false;
+
+    assert(RequiredAccessScopeAndUsableFromInline.hasValue() &&
+           "must check access first using getRequiredAccessScope");
+    return RequiredAccessScopeAndUsableFromInline.getValue().second;
+  }
+
   /// Gather the value witnesses for the given requirement.
   ///
   /// \param ignoringNames If non-null and there are no value
@@ -501,17 +516,15 @@ class WitnessChecker {
                        unsigned &bestIdx,
                        bool &doNotDiagnoseMatches);
 
-  bool checkWitnessAccess(AccessScope &requiredAccessScope,
-                          ValueDecl *requirement,
+  bool checkWitnessAccess(ValueDecl *requirement,
                           ValueDecl *witness,
                           bool *isSetter);
 
   bool checkWitnessAvailability(ValueDecl *requirement,
                                 ValueDecl *witness,
                                 AvailabilityContext *requirementInfo);
 
-  RequirementCheck checkWitness(AccessScope requiredAccessScope,
-                                ValueDecl *requirement,
+  RequirementCheck checkWitness(ValueDecl *requirement,
                                 const RequirementMatch &match);
 };
 

stdlib/public/core/ArrayBuffer.swift

@@ -504,6 +504,7 @@ extension _ArrayBuffer {
     return count
   }
 
+  @usableFromInline
   internal typealias Indices = Range<Int>
 
   //===--- private --------------------------------------------------------===//

stdlib/public/core/CocoaArray.swift

@@ -28,7 +28,9 @@ import SwiftShims
 @usableFromInline
 @_fixed_layout
 internal struct _CocoaArrayWrapper : RandomAccessCollection {
+  @usableFromInline
   typealias Indices = Range<Int>
+
   @inlinable
   internal var startIndex: Int {
     return 0

stdlib/public/core/ContiguousArrayBuffer.swift

@@ -551,6 +551,7 @@ extension _ContiguousArrayBuffer : RandomAccessCollection {
     return count
   }
 
+  @usableFromInline
   internal typealias Indices = Range<Int>
 }
 

stdlib/public/core/SliceBuffer.swift

@@ -344,6 +344,7 @@ internal struct _SliceBuffer<Element>
     }
   }
 
+  @usableFromInline
   internal typealias Indices = Range<Int>
 
   //===--- misc -----------------------------------------------------------===//

test/Compatibility/attr_usableFromInline_protocol.swift

@@ -0,0 +1,42 @@
+// RUN: %target-typecheck-verify-swift -swift-version 4
+// RUN: %target-typecheck-verify-swift -swift-version 4.2
+
+public protocol PublicProtoWithReqs {
+  associatedtype Assoc
+  func foo()
+}
+
+@usableFromInline struct UFIAdopter<T> : PublicProtoWithReqs {}
+// expected-warning@-1 {{type alias 'Assoc' should be declared '@usableFromInline' because because it matches a requirement in protocol 'PublicProtoWithReqs'}} {{none}}
+extension UFIAdopter {
+  typealias Assoc = Int
+  // expected-note@-1 {{'Assoc' declared here}}
+  func foo() {}
+}
+
+@usableFromInline struct UFIAdopterAllInOne<T> : PublicProtoWithReqs {
+  typealias Assoc = Int
+  // expected-warning@-1 {{type alias 'Assoc' should be declared '@usableFromInline' because because it matches a requirement in protocol 'PublicProtoWithReqs'}} {{none}}
+  func foo() {}
+}
+
+internal struct InternalAdopter<T> : PublicProtoWithReqs {}
+extension InternalAdopter {
+  typealias Assoc = Int // okay
+  func foo() {} // okay
+}
+
+
+@usableFromInline protocol UFIProtoWithReqs {
+  associatedtype Assoc
+  func foo()
+}
+
+public struct PublicAdopter<T> : UFIProtoWithReqs {}
+// expected-warning@-1 {{type alias 'Assoc' should be declared '@usableFromInline' because because it matches a requirement in protocol 'UFIProtoWithReqs'}} {{none}}
+extension PublicAdopter {
+  typealias Assoc = Int
+  // expected-note@-1 {{'Assoc' declared here}}
+  func foo() {}
+}
+extension InternalAdopter: UFIProtoWithReqs {} // okay

test/attr/attr_usableFromInline_protocol.swift

@@ -0,0 +1,42 @@
+// RUN: %target-typecheck-verify-swift -swift-version 5
+// RUN: %target-swift-frontend -typecheck -disable-access-control %s
+
+public protocol PublicProtoWithReqs {
+  associatedtype Assoc
+  func foo()
+}
+
+@usableFromInline struct UFIAdopter<T> : PublicProtoWithReqs {}
+// expected-error@-1 {{type alias 'Assoc' must be declared '@usableFromInline' because because it matches a requirement in protocol 'PublicProtoWithReqs'}} {{none}}
+extension UFIAdopter {
+  typealias Assoc = Int
+  // expected-note@-1 {{'Assoc' declared here}}
+  func foo() {}
+}
+
+@usableFromInline struct UFIAdopterAllInOne<T> : PublicProtoWithReqs {
+  typealias Assoc = Int
+  // expected-error@-1 {{type alias 'Assoc' must be declared '@usableFromInline' because because it matches a requirement in protocol 'PublicProtoWithReqs'}} {{none}}
+  func foo() {}
+}
+
+internal struct InternalAdopter<T> : PublicProtoWithReqs {}
+extension InternalAdopter {
+  typealias Assoc = Int // okay
+  func foo() {} // okay
+}
+
+
+@usableFromInline protocol UFIProtoWithReqs {
+  associatedtype Assoc
+  func foo()
+}
+
+public struct PublicAdopter<T> : UFIProtoWithReqs {}
+// expected-error@-1 {{type alias 'Assoc' must be declared '@usableFromInline' because because it matches a requirement in protocol 'UFIProtoWithReqs'}} {{none}}
+extension PublicAdopter {
+  typealias Assoc = Int
+  // expected-note@-1 {{'Assoc' declared here}}
+  func foo() {}
+}
+extension InternalAdopter: UFIProtoWithReqs {} // okay