[Sema] Diagnose internal(set) from @inlinable functions

This patch mainly consolidates the functions used to check accessors vs.
other decls, and makes sure we check setter access as well as regular
decl access.

rdar://45217648

Author: Harlan Haskins

include/swift/AST/DiagnosticsSema.def

@@ -4106,27 +4106,33 @@ ERROR(usable_from_inline_attr_in_protocol,none,
   "a default argument value|" \
   "a property initializer in a '@_fixed_layout' type}"
 
+#define DECL_OR_ACCESSOR "%select{%0|%0 for}"
+
 ERROR(local_type_in_inlinable_function,
       none, "type %0 cannot be nested inside " FRAGILE_FUNC_KIND "1",
       (DeclName, unsigned))
 
 ERROR(resilience_decl_unavailable,
-      none, "%0 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
+      none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
       "cannot be referenced from " FRAGILE_FUNC_KIND "3",
-      (DescriptiveDeclKind, DeclName, AccessLevel, unsigned))
+      (DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
 
 WARNING(resilience_decl_unavailable_warn,
-        none, "%0 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
+        none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
         "should not be referenced from " FRAGILE_FUNC_KIND "3",
-        (DescriptiveDeclKind, DeclName, AccessLevel, unsigned))
+        (DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
 
 #undef FRAGILE_FUNC_KIND
 
 NOTE(resilience_decl_declared_here_public,
-     none, "%0 %1 is not public", (DescriptiveDeclKind, DeclName))
+     none, DECL_OR_ACCESSOR "2 %1 is not public",
+     (DescriptiveDeclKind, DeclName, bool))
 
 NOTE(resilience_decl_declared_here,
-     none, "%0 %1 is not '@usableFromInline' or public", (DescriptiveDeclKind, DeclName))
+     none, DECL_OR_ACCESSOR "2 %1 is not '@usableFromInline' or public",
+     (DescriptiveDeclKind, DeclName, bool))
+
+#undef DECL_OR_ACCESSOR
 
 ERROR(class_designated_init_inlinable_resilient,none,
       "initializer for class %0 is "

lib/Sema/ResilienceDiagnostics.cpp

@@ -134,21 +134,37 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
       downgradeToWarning = DowngradeToWarning::Yes;
   }
 
+  auto diagName = D->getFullName();
+  bool isAccessor = false;
+
+  // Swift 4.2 did not check accessor accessiblity.
+  if (auto accessor = dyn_cast<AccessorDecl>(D)) {
+    isAccessor = true;
+
+    if (!Context.isSwiftVersionAtLeast(5))
+      downgradeToWarning = DowngradeToWarning::Yes;
+
+    // For accessors, diagnose with the name of the storage instead of the
+    // implicit '_'.
+    diagName = accessor->getStorage()->getFullName();
+  }
+
   auto diagID = diag::resilience_decl_unavailable;
   if (downgradeToWarning == DowngradeToWarning::Yes)
     diagID = diag::resilience_decl_unavailable_warn;
 
   diagnose(loc, diagID,
-           D->getDescriptiveKind(), D->getFullName(),
+           D->getDescriptiveKind(), diagName,
            D->getFormalAccessScope().accessLevelForDiagnostics(),
-           static_cast<unsigned>(Kind));
+           static_cast<unsigned>(Kind),
+           isAccessor);
 
   if (TreatUsableFromInlineAsPublic) {
     diagnose(D, diag::resilience_decl_declared_here,
-             D->getDescriptiveKind(), D->getFullName());
+             D->getDescriptiveKind(), diagName, isAccessor);
   } else {
     diagnose(D, diag::resilience_decl_declared_here_public,
-             D->getDescriptiveKind(), D->getFullName());
+             D->getDescriptiveKind(), diagName, isAccessor);
   }
 
   return (downgradeToWarning == DowngradeToWarning::No);

lib/Sema/TypeCheckAvailability.cpp

@@ -1355,12 +1355,12 @@ void TypeChecker::diagnosePotentialUnavailability(
 }
 
 void TypeChecker::diagnosePotentialAccessorUnavailability(
-    AccessorDecl *Accessor, SourceRange ReferenceRange,
+    const AccessorDecl *Accessor, SourceRange ReferenceRange,
     const DeclContext *ReferenceDC, const UnavailabilityReason &Reason,
     bool ForInout) {
   assert(Accessor->isGetterOrSetter());
 
-  AbstractStorageDecl *ASD = Accessor->getStorage();
+  const AbstractStorageDecl *ASD = Accessor->getStorage();
   DeclName Name = ASD->getFullName();
 
   auto &diag = ForInout ? diag::availability_inout_accessor_only_version_newer
@@ -2355,7 +2355,8 @@ class AvailabilityWalker : public ASTWalker {
   bool diagAvailability(const ValueDecl *D, SourceRange R,
                         const ApplyExpr *call = nullptr,
                         bool AllowPotentiallyUnavailableProtocol = false,
-                        bool SignalOnPotentialUnavailability = true);
+                        bool SignalOnPotentialUnavailability = true,
+                        bool ForInout = false);
 
 private:
   bool diagnoseIncDecRemoval(const ValueDecl *D, SourceRange R,
@@ -2511,32 +2512,13 @@ class AvailabilityWalker : public ASTWalker {
   void diagAccessorAvailability(AccessorDecl *D, SourceRange ReferenceRange,
                                 const DeclContext *ReferenceDC,
                                 bool ForInout) const {
-    if (!D) {
+    if (diagnoseDeclAvailability(D, TC,
+                                 const_cast<DeclContext*>(ReferenceDC),
+                                 ReferenceRange,
+                                 /*AllowPotentiallyUnavailableProtocol*/false,
+                                 /*SignalOnPotentialUnavailability*/false,
+                                 ForInout))
       return;
-    }
-
-    // If the property/subscript is unconditionally unavailable, don't bother
-    // with any of the rest of this.
-    if (AvailableAttr::isUnavailable(D->getStorage()))
-      return;
-
-    if (diagnoseExplicitUnavailability(D, ReferenceRange, ReferenceDC,
-                                       /*call*/nullptr)) {
-      return;
-    }
-
-    // Make sure not to diagnose an accessor's deprecation if we already
-    // complained about the property/subscript.
-    if (!TypeChecker::getDeprecated(D->getStorage()))
-      TC.diagnoseIfDeprecated(ReferenceRange, ReferenceDC, D, /*call*/nullptr);
-
-    auto MaybeUnavail = TC.checkDeclarationAvailability(D, ReferenceRange.Start,
-                                                        DC);
-    if (MaybeUnavail.hasValue()) {
-      TC.diagnosePotentialAccessorUnavailability(D, ReferenceRange, ReferenceDC,
-                                                 MaybeUnavail.getValue(),
-                                                 ForInout);
-    }
   }
 };
 } // end anonymous namespace
@@ -2546,7 +2528,8 @@ class AvailabilityWalker : public ASTWalker {
 bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
                                           const ApplyExpr *call,
                                           bool AllowPotentiallyUnavailableProtocol,
-                                          bool SignalOnPotentialUnavailability) {
+                                          bool SignalOnPotentialUnavailability,
+                                          bool ForInout) {
   if (!D)
     return false;
 
@@ -2557,6 +2540,16 @@ bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
       return true;
   }
 
+  // Keep track if this is an accessor.
+  auto accessor = dyn_cast<AccessorDecl>(D);
+
+  if (accessor) {
+    // If the property/subscript is unconditionally unavailable, don't bother
+    // with any of the rest of this.
+    if (AvailableAttr::isUnavailable(accessor->getStorage()))
+      return false;
+  }
+
   if (FragileKind)
     if (R.isValid())
       if (TC.diagnoseInlinableDeclRef(R.Start, D, DC, *FragileKind,
@@ -2566,16 +2559,28 @@ bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
   if (diagnoseExplicitUnavailability(D, R, DC, call))
     return true;
 
+  // Make sure not to diagnose an accessor's deprecation if we already
+  // complained about the property/subscript.
+  bool isAccessorWithDeprecatedStorage =
+    accessor && TypeChecker::getDeprecated(accessor->getStorage());
+
   // Diagnose for deprecation
-  TC.diagnoseIfDeprecated(R, DC, D, call);
+  if (!isAccessorWithDeprecatedStorage)
+    TC.diagnoseIfDeprecated(R, DC, D, call);
 
   if (AllowPotentiallyUnavailableProtocol && isa<ProtocolDecl>(D))
     return false;
 
   // Diagnose (and possibly signal) for potential unavailability
   auto maybeUnavail = TC.checkDeclarationAvailability(D, R.Start, DC);
   if (maybeUnavail.hasValue()) {
-    TC.diagnosePotentialUnavailability(D, R, DC, maybeUnavail.getValue());
+    if (accessor) {
+      TC.diagnosePotentialAccessorUnavailability(accessor, R, DC,
+                                                 maybeUnavail.getValue(),
+                                                 ForInout);
+    } else {
+      TC.diagnosePotentialUnavailability(D, R, DC, maybeUnavail.getValue());
+    }
     if (SignalOnPotentialUnavailability)
       return true;
   }
@@ -2751,10 +2756,12 @@ bool swift::diagnoseDeclAvailability(const ValueDecl *Decl,
                                      DeclContext *DC,
                                      SourceRange R,
                                      bool AllowPotentiallyUnavailableProtocol,
-                                     bool SignalOnPotentialUnavailability)
+                                     bool SignalOnPotentialUnavailability,
+                                     bool ForInout)
 {
   AvailabilityWalker AW(TC, DC);
   return AW.diagAvailability(Decl, R, nullptr,
                              AllowPotentiallyUnavailableProtocol,
-                             SignalOnPotentialUnavailability);
+                             SignalOnPotentialUnavailability,
+                             ForInout);
 }

lib/Sema/TypeCheckAvailability.h

@@ -41,7 +41,8 @@ bool diagnoseDeclAvailability(const ValueDecl *Decl,
                               DeclContext *DC,
                               SourceRange R,
                               bool AllowPotentiallyUnavailableProtocol,
-                              bool SignalOnPotentialUnavailability);
+                              bool SignalOnPotentialUnavailability,
+                              bool ForInout);
 
 void diagnoseUnavailableOverride(ValueDecl *override,
                                  const ValueDecl *base,

lib/Sema/TypeCheckType.cpp

@@ -1515,7 +1515,8 @@ static bool diagnoseAvailability(IdentTypeRepr *IdType,
       TypeChecker &tc = static_cast<TypeChecker &>(*ctx.getLazyResolver());
       if (diagnoseDeclAvailability(typeDecl, tc, DC, comp->getIdLoc(),
                                    AllowPotentiallyUnavailableProtocol,
-                                   /*SignalOnPotentialUnavailability*/false)) {
+                                   /*SignalOnPotentialUnavailability*/false,
+                                   /*ForInout*/false)) {
         return true;
       }
     }

lib/Sema/TypeChecker.h

@@ -2077,7 +2077,7 @@ class TypeChecker final : public LazyResolver {
   /// Emits a diagnostic for a reference to a storage accessor that is
   /// potentially unavailable.
   void diagnosePotentialAccessorUnavailability(
-      AccessorDecl *Accessor, SourceRange ReferenceRange,
+      const AccessorDecl *Accessor, SourceRange ReferenceRange,
       const DeclContext *ReferenceDC, const UnavailabilityReason &Reason,
       bool ForInout);
 

test/Compatibility/attr_inlinable_swift42.swift

@@ -17,6 +17,14 @@ enum InternalEnum {
   case persimmon(String)
 }
 
+public struct HasInternalSetProperty {
+  public internal(set) var x: Int // expected-note {{setter for 'x' is not '@usableFromInline' or public}}
+
+  @inlinable public mutating func setsX() {
+    x = 10 // expected-warning {{setter for 'x' is internal and should not be referenced from an '@inlinable' function}}
+  }
+}
+
 @usableFromInline protocol P {
   typealias T = Int
 }

test/attr/attr_inlinable.swift

@@ -248,6 +248,15 @@ public struct KeypathStruct {
     // expected-error@-1 {{property 'x' is internal and cannot be referenced from an '@inlinable' function}}
   }
 }
+
+public struct HasInternalSetProperty {
+  public internal(set) var x: Int // expected-note {{setter for 'x' is not '@usableFromInline' or public}}
+
+  @inlinable public mutating func setsX() {
+    x = 10 // expected-error {{setter for 'x' is internal and cannot be referenced from an '@inlinable' function}}
+  }
+}
+
 @usableFromInline protocol P {
   typealias T = Int
 }