Sema: Targeted fix for bad interaction between resilience checks and -enable-testing

The -enable-testing flag makes ValueDecl::getEffectiveAccess()
say that internal declarations are public.

This would lead us to emit spurious diagnostics if a default
argument of an internal function referenced a private symbol,
for example, which is something we actually want to allow.

Hack around this by adding a new 'forLinkage' parameter to
getEffectiveAccess(). When this is false, we ignore the
-enable-testing flag, and only look for the @_versioned
attribute.

I'm not very happy with the fix, because it only compliates
the subtle behaviors of getFormalAccess(), getEffectiveAccess()
and getFormalAccessScope() further. But refactoring this is
a bigger change than I'm willing to put into swift-4.0-branch.

Fixes <rdar://problem/32592973>.

Author: slavapestov

include/swift/AST/Decl.h

@@ -2201,7 +2201,10 @@ class ValueDecl : public Decl {
   ///
   /// This is the access used when making optimization and code generation
   /// decisions. It should not be used at the AST or semantic level.
-  Accessibility getEffectiveAccess() const;
+  ///
+  /// If \p forLinkage is false, we ignore -enable-testing; only @_versioned
+  /// can increase visibility.
+  Accessibility getEffectiveAccess(bool forLinkage = true) const;
 
   void setAccessibility(Accessibility access) {
     assert(!hasAccessibility() && "accessibility already set");

lib/AST/Decl.cpp

@@ -1359,7 +1359,7 @@ bool AbstractStorageDecl::hasFixedLayout() const {
 
   // Private and (unversioned) internal variables always have a
   // fixed layout.
-  if (getEffectiveAccess() < Accessibility::Public)
+  if (getEffectiveAccess(/*forLinkage=*/false) < Accessibility::Public)
     return true;
 
   // Check for an explicit @_fixed_layout attribute.
@@ -1952,19 +1952,19 @@ static Accessibility getTestableAccess(const ValueDecl *decl) {
   return Accessibility::Public;
 }
 
-Accessibility ValueDecl::getEffectiveAccess() const {
+Accessibility ValueDecl::getEffectiveAccess(bool forLinkage) const {
   Accessibility effectiveAccess = getFormalAccess();
 
   // Handle @testable.
   switch (effectiveAccess) {
   case Accessibility::Public:
-    if (getModuleContext()->isTestingEnabled())
+    if (forLinkage && getModuleContext()->isTestingEnabled())
       effectiveAccess = getTestableAccess(this);
     break;
   case Accessibility::Open:
     break;
   case Accessibility::Internal:
-    if (getModuleContext()->isTestingEnabled())
+    if (forLinkage && getModuleContext()->isTestingEnabled())
       effectiveAccess = getTestableAccess(this);
     else if (isVersionedInternalDecl(this))
       effectiveAccess = Accessibility::Public;
@@ -1978,15 +1978,15 @@ Accessibility ValueDecl::getEffectiveAccess() const {
 
   if (auto enclosingNominal = dyn_cast<NominalTypeDecl>(getDeclContext())) {
     effectiveAccess = std::min(effectiveAccess,
-                               enclosingNominal->getEffectiveAccess());
+                               enclosingNominal->getEffectiveAccess(forLinkage));
 
   } else if (auto enclosingExt = dyn_cast<ExtensionDecl>(getDeclContext())) {
     // Just check the base type. If it's a constrained extension, Sema should
     // have already enforced access more strictly.
     if (auto extendedTy = enclosingExt->getExtendedType()) {
       if (auto nominal = extendedTy->getAnyNominal()) {
         effectiveAccess = std::min(effectiveAccess,
-                                   nominal->getEffectiveAccess());
+                                   nominal->getEffectiveAccess(forLinkage));
       }
     }
 
@@ -2117,7 +2117,7 @@ int TypeDecl::compare(const TypeDecl *type1, const TypeDecl *type2) {
 bool NominalTypeDecl::hasFixedLayout() const {
   // Private and (unversioned) internal types always have a
   // fixed layout.
-  if (getEffectiveAccess() < Accessibility::Public)
+  if (getEffectiveAccess(/*forLinkage=*/false) < Accessibility::Public)
     return true;
 
   // Check for an explicit @_fixed_layout attribute.

lib/AST/DeclContext.cpp

@@ -550,7 +550,7 @@ ResilienceExpansion DeclContext::getResilienceExpansion() const {
 
       // If the function is not externally visible, we will not be serializing
       // its body.
-      if (AFD->getEffectiveAccess() < Accessibility::Public)
+      if (AFD->getEffectiveAccess(/*forLinkage=*/false) < Accessibility::Public)
         break;
 
       // Bodies of public transparent and always-inline functions are

lib/Sema/ResilienceDiagnostics.cpp

@@ -91,7 +91,7 @@ bool TypeChecker::diagnoseInlineableDeclRef(SourceLoc loc,
     return false;
 
   // Public declarations are OK.
-  if (D->getEffectiveAccess() >= Accessibility::Public)
+  if (D->getEffectiveAccess(/*forLinkage=*/false) >= Accessibility::Public)
     return false;
 
   // Enum cases are handled as part of their containing enum.
@@ -110,7 +110,8 @@ bool TypeChecker::diagnoseInlineableDeclRef(SourceLoc loc,
 
   diagnose(loc, diag::resilience_decl_unavailable,
            D->getDescriptiveKind(), D->getFullName(),
-           D->getFormalAccess(), getFragileFunctionKind(DC));
+           D->getFormalAccessScope().accessibilityForDiagnostics(),
+           getFragileFunctionKind(DC));
   diagnose(D, diag::resilience_decl_declared_here,
            D->getDescriptiveKind(), D->getFullName());
   return true;

lib/Sema/TypeCheckAttr.cpp

@@ -1824,19 +1824,14 @@ void AttributeChecker::visitSpecializeAttr(SpecializeAttr *attr) {
                                         /*allowConcreteGenericParams=*/true);
 }
 
-static Accessibility getAccessForDiagnostics(const ValueDecl *D) {
-  return std::min(D->getFormalAccess(),
-                  D->getEffectiveAccess());
-}
-
 void AttributeChecker::visitFixedLayoutAttr(FixedLayoutAttr *attr) {
   auto *VD = cast<ValueDecl>(D);
 
-  if (VD->getEffectiveAccess() < Accessibility::Public) {
+  if (VD->getEffectiveAccess(/*forLinkage=*/false) < Accessibility::Public) {
     TC.diagnose(attr->getLocation(),
                 diag::fixed_layout_attr_on_internal_type,
                 VD->getBaseName(),
-                getAccessForDiagnostics(VD))
+                VD->getFormalAccessScope().accessibilityForDiagnostics())
         .fixItRemove(attr->getRangeWithAt());
     attr->setInvalid();
   }
@@ -1886,13 +1881,11 @@ void AttributeChecker::visitInlineableAttr(InlineableAttr *attr) {
 
   // @_inlineable can only be applied to public or @_versioned
   // declarations.
-  if (VD->getFormalAccess() < Accessibility::Internal ||
-      (!VD->getAttrs().hasAttribute<VersionedAttr>() &&
-       VD->getFormalAccess() < Accessibility::Public)) {
+  if (VD->getEffectiveAccess(/*forLinkage=*/false) < Accessibility::Public) {
     TC.diagnose(attr->getLocation(),
                 diag::inlineable_decl_not_public,
                 VD->getBaseName(),
-                getAccessForDiagnostics(VD))
+                VD->getFormalAccessScope().accessibilityForDiagnostics())
         .fixItRemove(attr->getRangeWithAt());
     attr->setInvalid();
     return;

lib/Sema/TypeCheckStmt.cpp

@@ -1321,7 +1321,7 @@ static void checkDefaultArguments(TypeChecker &tc,
   // caller.
   auto expansion = func->getResilienceExpansion();
   if (!tc.Context.isSwiftVersion3() &&
-      func->getEffectiveAccess() == Accessibility::Public)
+      func->getEffectiveAccess(/*forLinkage=*/false) == Accessibility::Public)
     expansion = ResilienceExpansion::Minimal;
 
   for (auto &param : *params) {

test/attr/attr_fixed_layout.swift

@@ -1,5 +1,7 @@
 // RUN: %target-swift-frontend -typecheck -verify -dump-ast -enable-resilience %s 2>&1 | %FileCheck --check-prefix=RESILIENCE-ON %s
+// RUN: %target-swift-frontend -typecheck -verify -dump-ast -enable-resilience -enable-testing %s 2>&1 | %FileCheck --check-prefix=RESILIENCE-ON %s
 // RUN: %target-swift-frontend -typecheck -verify -dump-ast %s 2>&1 | %FileCheck --check-prefix=RESILIENCE-OFF %s
+// RUN: %target-swift-frontend -typecheck -verify -dump-ast %s -enable-testing 2>&1 | %FileCheck --check-prefix=RESILIENCE-OFF %s
 
 //
 // Public types with @_fixed_layout are always fixed layout

test/attr/attr_inlineable.swift

@@ -1,4 +1,5 @@
 // RUN: %target-typecheck-verify-swift -swift-version 4
+// RUN: %target-typecheck-verify-swift -swift-version 4 -enable-testing
 
 @_inlineable struct TestInlineableStruct {}
 // expected-error@-1 {{@_inlineable cannot be applied to this declaration}}

test/attr/attr_versioned.swift

@@ -1,4 +1,5 @@
 // RUN: %target-typecheck-verify-swift
+// RUN: %target-typecheck-verify-swift -enable-testing
 
 @_versioned private func privateVersioned() {}
 // expected-error@-1 {{'@_versioned' attribute can only be applied to internal declarations, but 'privateVersioned' is private}}