Merge pull request #24154 from jrose-apple/5.1-implementation-only-implementation---only

[5.1] Miscellaneous cleanup and consolidation for implementation-only checking

Author: jrose-apple

include/swift/AST/DiagnosticsSema.def

@@ -2395,16 +2395,15 @@ NOTE(construct_raw_representable_from_unwrapped_value,none,
      "construct %0 from unwrapped %1 value", (Type, Type))
 
 ERROR(decl_from_implementation_only_module,none,
-      "cannot use %0 here; %1 has been imported as "
-      "'@_implementationOnly'",
-      (DeclName, Identifier))
+      "cannot use %0 %1 here; %2 has been imported as implementation-only",
+      (DescriptiveDeclKind, DeclName, Identifier))
 ERROR(conformance_from_implementation_only_module,none,
       "cannot use conformance of %0 to %1 here; %2 has been imported as "
-      "'@_implementationOnly'",
+      "implementation-only",
       (Type, DeclName, Identifier))
 ERROR(assoc_conformance_from_implementation_only_module,none,
       "cannot use conformance of %0 to %1 in associated type %3 (inferred as "
-      "%4); %2 has been imported as '@_implementationOnly'",
+      "%4); %2 has been imported as implementation-only",
       (Type, DeclName, Identifier, Type, Type))
 
 // Derived conformances
@@ -4091,9 +4090,9 @@ WARNING(resilience_decl_unavailable_warn,
         (DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
 
 ERROR(inlinable_decl_ref_implementation_only,
-      none, "%0 %1 cannot be used in an inlinable "
-      "function because its module was imported implementation-only",
-      (DescriptiveDeclKind, DeclName))
+      none, "%0 %1 cannot be used in " FRAGILE_FUNC_KIND "2 "
+      "because %3 was imported implementation-only",
+      (DescriptiveDeclKind, DeclName, unsigned, Identifier))
 
 #undef FRAGILE_FUNC_KIND
 

include/swift/AST/AccessScopeChecker.h renamed to include/swift/AST/TypeDeclFinder.h

@@ -1,53 +1,28 @@
-//===--- AccessScopeChecker.h - Access calculation helpers -----*- C++ -*-===//
+//===--- TypeDeclFinder.h - Finds TypeDecls in Types/TypeReprs --*- C++ -*-===//
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
-//
-//  This file defines helpers for access-control calculation.
-//
-//===----------------------------------------------------------------------===//
-#ifndef SWIFT_ACCESS_SCOPE_CHECKER_H
-#define SWIFT_ACCESS_SCOPE_CHECKER_H
+#ifndef SWIFT_AST_TYPEDECLFINDER_H
+#define SWIFT_AST_TYPEDECLFINDER_H
 
-#include "swift/AST/AccessScope.h"
-#include "swift/AST/AttrKind.h"
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/TypeWalker.h"
+#include "llvm/ADT/STLExtras.h"
 
 namespace swift {
 
 class BoundGenericType;
 class ComponentIdentTypeRepr;
 class NominalType;
-class SourceFile;
 class TypeAliasType;
 
-class AccessScopeChecker {
-  const SourceFile *File;
-  bool TreatUsableFromInlineAsPublic;
-
-  Optional<AccessScope> Scope = AccessScope::getPublic();
-
-  AccessScopeChecker(const DeclContext *useDC,
-                     bool treatUsableFromInlineAsPublic);
-  bool visitDecl(const ValueDecl *VD);
-
-public:
-  static Optional<AccessScope>
-  getAccessScope(TypeRepr *TR, const DeclContext *useDC,
-                 bool treatUsableFromInlineAsPublic = false);
-  static Optional<AccessScope>
-  getAccessScope(Type T, const DeclContext *useDC,
-                 bool treatUsableFromInlineAsPublic = false);
-};
-
 /// Walks a Type to find all NominalTypes, BoundGenericTypes, and
 /// TypeAliasTypes.
 class TypeDeclFinder : public TypeWalker {

lib/AST/AccessScopeChecker.cpp

@@ -37,7 +37,6 @@ else()
 endif()
 
 add_swift_host_library(swiftAST STATIC
-  AccessScopeChecker.cpp
   AccessRequests.cpp
   ASTContext.cpp
   ASTDemangler.cpp
@@ -91,6 +90,7 @@ add_swift_host_library(swiftAST STATIC
   SwiftNameTranslation.cpp
   Type.cpp
   TypeCheckRequests.cpp
+  TypeDeclFinder.cpp
   TypeJoinMeet.cpp
   TypeRefinementContext.cpp
   TypeRepr.cpp

lib/AST/CMakeLists.txt

@@ -0,0 +1,57 @@
+//===--- TypeDeclFinder.cpp - Finds TypeDecls inside Types/TypeReprs ------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#include "swift/AST/TypeDeclFinder.h"
+#include "swift/AST/Decl.h"
+#include "swift/AST/TypeRepr.h"
+#include "swift/AST/Types.h"
+
+using namespace swift;
+
+TypeWalker::Action TypeDeclFinder::walkToTypePre(Type T) {
+  if (auto *TAT = dyn_cast<TypeAliasType>(T.getPointer()))
+    return visitTypeAliasType(TAT);
+
+  // FIXME: We're looking through sugar here so that we visit, e.g.,
+  // Swift.Array when we see `[Int]`. But that means we do redundant work when
+  // we see sugar that's purely structural, like `(Int)`. Fortunately, paren
+  // types are the only such purely structural sugar at the time this comment
+  // was written, and they're not so common in the first place.
+  if (auto *BGT = T->getAs<BoundGenericType>())
+    return visitBoundGenericType(BGT);
+  if (auto *NT = T->getAs<NominalType>())
+    return visitNominalType(NT);
+
+  return Action::Continue;
+}
+
+TypeWalker::Action
+SimpleTypeDeclFinder::visitNominalType(NominalType *ty) {
+  return Callback(ty->getDecl());
+}
+
+TypeWalker::Action
+SimpleTypeDeclFinder::visitBoundGenericType(BoundGenericType *ty) {
+  return Callback(ty->getDecl());
+}
+
+TypeWalker::Action
+SimpleTypeDeclFinder::visitTypeAliasType(TypeAliasType *ty) {
+  return Callback(ty->getDecl());
+}
+
+bool TypeReprIdentFinder::walkToTypeReprPost(TypeRepr *TR) {
+  auto CITR = dyn_cast<ComponentIdentTypeRepr>(TR);
+  if (!CITR || !CITR->getBoundDecl())
+    return true;
+  return Callback(CITR);
+}

lib/AST/TypeDeclFinder.cpp

@@ -16,12 +16,12 @@
 
 #include "TypeChecker.h"
 #include "TypeCheckAvailability.h"
-#include "swift/AST/AccessScopeChecker.h"
 #include "swift/AST/Attr.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/DeclContext.h"
 #include "swift/AST/Initializer.h"
 #include "swift/AST/ProtocolConformance.h"
+#include "swift/AST/TypeDeclFinder.h"
 
 using namespace swift;
 using FragileFunctionKind = TypeChecker::FragileFunctionKind;
@@ -119,8 +119,11 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
     return true;
 
   // Check whether the declaration comes from a publically-imported module.
-  if (diagnoseDeclRefExportability(loc, declRef, DC))
-    return true;
+  // Skip this check for accessors because the associated property or subscript
+  // will also be checked, and will provide a better error message.
+  if (!isa<AccessorDecl>(D))
+    if (diagnoseDeclRefExportability(loc, declRef, DC, Kind))
+      return true;
 
   return false;
 }
@@ -208,15 +211,18 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
 }
 
 static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D,
-                                      const SourceFile &userSF) {
+                                      const SourceFile &userSF,
+                                      FragileFunctionKind fragileKind) {
   auto definingModule = D->getModuleContext();
   if (!userSF.isImportedImplementationOnly(definingModule))
     return false;
 
   // TODO: different diagnostics
   ASTContext &ctx = definingModule->getASTContext();
   ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_implementation_only,
-                     D->getDescriptiveKind(), D->getFullName());
+                     D->getDescriptiveKind(), D->getFullName(),
+                     static_cast<unsigned>(fragileKind),
+                     definingModule->getName());
   return true;
 }
 
@@ -288,9 +294,11 @@ void TypeChecker::diagnoseGenericTypeExportability(const TypeLoc &TL,
   }));
 }
 
-bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
-                                               ConcreteDeclRef declRef,
-                                               const DeclContext *DC) {
+bool
+TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
+                                          ConcreteDeclRef declRef,
+                                          const DeclContext *DC,
+                                          FragileFunctionKind fragileKind) {
   // We're only interested in diagnosing uses from source files.
   auto userSF = DC->getParentSourceFile();
   if (!userSF)
@@ -305,7 +313,7 @@ bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
     return false;
 
   const ValueDecl *D = declRef.getDecl();
-  if (diagnoseDeclExportability(loc, D, *userSF))
+  if (diagnoseDeclExportability(loc, D, *userSF, fragileKind))
     return true;
   if (diagnoseGenericArgumentsExportability(loc, declRef.getSubstitutions(),
                                             *userSF)) {