Merge pull request #64373 from apple/es-tests

Update package name input and access scope check

Author: elsh

include/swift/AST/AccessScope.h

@@ -107,14 +107,12 @@ class AccessScope {
   /// \see AccessScope::checkAccessUsingAccessScope
   /// \see DeclContext::ASTHierarchy
   bool isChildOf(AccessScope AS) const {
-    if (isPackage()) { // This needs to be checked first before isInContext
-      return AS.isPublic();
-    } else if (isInContext()) {
+    if (isInContext()) {
       if (AS.isInContext())
         return allowsPrivateAccess(getDeclContext(), AS.getDeclContext());
       else
         return AS.isPublic();
-    } else { // It's public, so can't be a child of the argument scope
+    } else { // It's public, so can't be a child of public or less argument scope
       return false;
     }
   }

include/swift/AST/DiagnosticsFrontend.def

@@ -192,9 +192,6 @@ ERROR(error_bad_export_as_name,none,
 ERROR(error_bad_package_name,none,
       "package name \"%0\" is not a valid identifier",
       (StringRef))
-ERROR(error_stdlib_package_name,none,
-      "package name \"%0\" is reserved for the standard library",
-      (StringRef))
 ERROR(error_stdlib_not_found,Fatal,
       "unable to load standard library for target '%0'", (StringRef))
 ERROR(error_module_alias_invalid_format,none,

include/swift/AST/DiagnosticsSema.def

@@ -1681,8 +1681,9 @@ ERROR(access_control_open_bad_decl,none,
 WARNING(access_control_non_objc_open_member,none,
       "non-'@objc' %0 in extensions cannot be overridden; use 'public' instead",
       (DescriptiveDeclKind))
-ERROR(access_control_requires_package_name, none, "decl has a package access level but no -package-name was passed", ())
-
+ERROR(access_control_requires_package_name, none,
+      "decl has a package access level but no -package-name was passed",
+      ())
 ERROR(invalid_decl_attribute,none,
       "'%0' attribute cannot be applied to this declaration", (DeclAttribute))
 ERROR(attr_invalid_on_decl_kind,none,

include/swift/AST/Module.h

@@ -160,18 +160,27 @@ class OverlayFile;
 class ModuleSourceFileLocationMap;
 
 /// A unit that allows grouping of modules by a package name.
-/// It's set as a property in ModuleDecl, instead of as its parent decl context,
-/// since otherwise it will break the existing decl context lookups that assume
-/// ModuleDecl as the top level context. See \c ModuleDecl
+///
+/// PackageUnit is treated as an enclosing scope of ModuleDecl. Unlike other
+/// DeclContext subclasses where a parent context is set in ctor, PackageUnit
+/// (parent context) is set as a field in ModuleDecl (child context). It also has a
+/// pointer back to the ModuleDecl, so that it can be used to return the module
+/// in the existing DeclContext lookup functions, which assume ModuleDecl as
+/// the top level context. Since both PackageUnit and ModuleDecl are created
+/// in the ASTContext memory arena, i.e. they will be destroyed when the
+/// ASTContext is destroyed, both pointng to each other is not considered risky.
+///
+/// See \c ModuleDecl
 class PackageUnit: public DeclContext {
   /// Identifies this package and used for the equality check
   Identifier PackageName;
-  /// Weakly references ModuleDecl that points to this package.
+  /// Non-null reference to ModuleDecl that points to this package.
   /// Instead of having multiple ModuleDecls pointing to one PackageUnit, we
-  /// create and set one PackageUnit per ModuleDecl, to make it easier to look
-  /// up the module pointing to this package; this look up is needed in existing
-  /// DeclContext functions, e.g. \c DeclContext::getModuleScopeContext,
-  /// and \c DeclContext::getParentModule
+  /// create one PackageUnit per ModuleDecl, to make it easier to look up the
+  /// module pointing to this package context, which is needed in the existing
+  /// DeclContext look up functions.
+  /// \see DeclContext::getModuleScopeContext
+  /// \see DeclContext::getParentModule
   ModuleDecl &SourceModule;
 
   PackageUnit(Identifier name, ModuleDecl &src)
@@ -219,7 +228,7 @@ class ModuleDecl
 
   /// A package this module belongs to. It's set as a property instead of a
   /// parent decl context; otherwise it will break the existing decl context
-  /// lookups that assume ModuleDecl as the top level context.
+  /// lookup functions that assume ModuleDecl as the top level context.
   PackageUnit *Package = nullptr;
 
   /// Module name to use when referenced in clients module interfaces.

lib/AST/Decl.cpp

@@ -3982,12 +3982,11 @@ ValueDecl::getFormalAccessScope(const DeclContext *useDC,
 /// (useDC) and the decl (VD) site, and returns true in this case, since
 /// FileUnit is a child of nullptr based on the DeclContext hierarchy. The
 /// hierarchy is created when subclasses of DeclContext such as FileUnit or
-/// ModuleDecl are constructed. For example, FileUnit ctor takes ModuleDecl as
-/// its parent DeclContext. There's an exception, however; the parent of
-/// ModuleDecl is nullptr, not set to PackageUnit; ModuleDecl has a pointer to
-/// PackageUnit as its field, and it is treated as the enclosing scope of
-/// ModuleDecl in the `isChildOf` call.
-///
+/// ModuleDecl are constructed. For example, a top ClassDecl ctor takes FileUnit
+/// as its parent DeclContext and FileUnit ctor takes ModuleDecl as its parent
+/// DeclContext. There's an exception, however, for the case of PackageUnit.
+/// \see PackageUnit for details on how the hierachy between that and ModuleDecl
+///      is created.
 /// \see DeclContext::ASTHierarchy
 /// \see AccessScope::getAccessScopeForFormalAccess
 /// \see ValueDecl::isAccessibleFrom for a description of \p forConformance.

lib/Frontend/CompilerInvocation.cpp

@@ -762,8 +762,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
     auto pkgName = A->getValue();
     if (!Lexer::isIdentifier(pkgName))
       Diags.diagnose(SourceLoc(), diag::error_bad_package_name, pkgName);
-    else if (pkgName == STDLIB_NAME)
-      Diags.diagnose(SourceLoc(), diag::error_stdlib_package_name, pkgName);
     else
       Opts.PackageName = pkgName;
   }

test/Sema/accessibility_compound.swift

@@ -1,13 +1,15 @@
-// RUN: %target-typecheck-verify-swift -swift-version 4
+// RUN: %target-typecheck-verify-swift -swift-version 4 -package-name mypkg
 
 public struct Pair<A, B> {}
 
 public struct PublicStruct {
   public struct Inner {}
-  internal struct Internal {}
+  package struct PkgInner {}
+  internal struct Internal {} // expected-note * {{type declared here}}
 }
 
 private typealias PrivateAlias = PublicStruct // expected-note * {{type declared here}}
+package typealias PackageAlias = PublicStruct // expected-note * {{type declared here}}
 
 public let a0 = nil as PrivateAlias.Inner? // expected-error {{constant cannot be declared public because its type 'PrivateAlias.Inner?' (aka 'Optional<PublicStruct.Inner>') uses a private type}}
 public let a: PrivateAlias.Inner? // expected-error {{constant cannot be declared public because its type uses a private type}}
@@ -16,6 +18,26 @@ public let c: Pair<PrivateAlias.Inner, PublicStruct.Internal>? // expected-error
 public let c2: Pair<PublicStruct.Internal, PrivateAlias.Inner>? // expected-error {{constant cannot be declared public because its type uses a private type}}
 public let d: PrivateAlias? // expected-error {{constant cannot be declared public because its type uses a private type}}
 
+package let e = nil as PrivateAlias.Inner? // expected-error {{constant cannot be declared package because its type 'PrivateAlias.Inner?' (aka 'Optional<PublicStruct.Inner>') uses a private type}}
+package let f: PrivateAlias.Inner? // expected-error {{constant cannot be declared package because its type uses a private type}}
+package let g: PrivateAlias.PkgInner? // expected-error {{constant cannot be declared package because its type uses a private type}}
+package let h: Pair<PrivateAlias.PkgInner, PublicStruct.Inner>? // expected-error {{constant cannot be declared package because its type uses a private type}}
+package let i: Pair<PublicStruct.PkgInner, PrivateAlias.Inner>? // expected-error {{constant cannot be declared package because its type uses a private type}}
+package let j: PrivateAlias? // expected-error {{constant cannot be declared package because its type uses a private type}}
+
+public let k = nil as PackageAlias.Inner? // expected-error {{constant cannot be declared public because its type 'PackageAlias.Inner?' (aka 'Optional<PublicStruct.Inner>') uses a package type}}
+public let l: PackageAlias.Inner? // expected-error {{constant cannot be declared public because its type uses a package type}}
+public let m: PackageAlias.Internal? // expected-error {{constant cannot be declared public because its type uses an internal type}}
+public let n: Pair<PackageAlias.Inner, PublicStruct.Internal>? // expected-error {{constant cannot be declared public because its type uses an internal type}}
+public let o: Pair<PublicStruct.Internal, PackageAlias.Inner>? // expected-error {{constant cannot be declared public because its type uses an internal type}}
+public let p: PackageAlias? // expected-error {{constant cannot be declared public because its type uses a package type}}
+
+package let q = nil as PackageAlias.Inner? // no-error
+package let r: PackageAlias.Inner? // no-error
+package let s: PackageAlias.Internal? // expected-error {{constant cannot be declared package because its type uses an internal type}}
+package let t: Pair<PackageAlias.Inner, PublicStruct.Internal>? // expected-error {{constant cannot be declared package because its type uses an internal type}}
+package let u: Pair<PublicStruct.Internal, PackageAlias.Inner>? // expected-error {{constant cannot be declared package because its type uses an internal type}}
+package let v: PackageAlias? // no-error
 
 // rdar://problem/21408035
 private class PrivateBox<T> { // expected-note 2 {{type declared here}}

test/Sema/accessibility_package.swift

@@ -0,0 +1,48 @@
+// RUN: %empty-directory(%t)
+// RUN: %{python} %utils/split_file.py -o %t %s
+
+// RUN: %target-swift-frontend -module-name Utils %t/Utils.swift -emit-module -emit-module-path %t/Utils.swiftmodule -package-name myLib
+// RUN: test -f %t/Utils.swiftmodule
+
+// RUN: %target-swift-frontend -module-name Lib %t/Lib.swift -emit-module -emit-module-path %t/Lib.swiftmodule -package-name myLib -I %t
+// RUN: test -f %t/Lib.swiftmodule
+
+// RUN: not %target-swift-frontend -typecheck %t/Lib.swift -package-name "otherLib" -I %t 2>&1 | %FileCheck %s
+// CHECK: error: cannot find type 'PackageProto' in scope
+// CHECK: error: 'pkgFunc' is inaccessible due to 'package' protection level
+// CHECK: error: cannot find 'PackageKlass' in scope
+
+
+// BEGIN Utils.swift
+package protocol PackageProto {
+  var pkgVar: String { get set }
+}
+
+package class PackageKlass {
+  package init() {}
+  package func pkgFunc() {}
+}
+
+public class PublicKlass {
+  public init() {}
+  public func publicFunc() {}
+  package func pkgFunc() {}
+}
+
+
+// BEGIN Lib.swift
+import Utils
+
+public func start() {
+  let x = PublicKlass()
+  x.publicFunc()
+  x.pkgFunc()
+
+  let y = PackageKlass()
+  y.pkgFunc()
+}
+
+package struct LibStruct : PackageProto {
+  package var pkgVar: String = "lib"
+}
+

test/Sema/accessibility_protocol_extension.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -swift-version 4
+// RUN: %target-typecheck-verify-swift -swift-version 4 -package-name myPkg
 
 fileprivate struct FilePrivateStruct {}
 // expected-note@-1 4{{type declared here}}
@@ -7,7 +7,10 @@ private struct PrivateStruct {}
 // expected-note@-1 4{{type declared here}}
 
 internal struct InternalStruct {}
-// expected-note@-1 4{{type declared here}}
+// expected-note@-1 7{{type declared here}}
+
+package struct PackageStruct {}
+// expected-note@-1 *{{type declared here}}
 
 public protocol P {
   typealias TFP = FilePrivateStruct
@@ -18,6 +21,9 @@ public protocol P {
 
   typealias TI = InternalStruct
   // expected-error@-1 {{type alias cannot be declared public because its underlying type uses an internal type}}
+
+  typealias TPkg = PackageStruct
+  // expected-error@-1 {{type alias cannot be declared public because its underlying type uses a package type}}
 }
 
 extension P {
@@ -49,4 +55,24 @@ extension P {
 
   public var usesInternalStructProp: InternalStruct { get { } set { } }
   // expected-error@-1 {{property cannot be declared public because its type uses an internal type}}
+
+
+  package func pkgUsesInternalStruct(_: InternalStruct) {}
+  // expected-error@-1 {{method cannot be declared package because its parameter uses an internal type}}
+
+  package typealias PkgUsesInternalStructAlias = InternalStruct
+  // expected-error@-1 {{type alias cannot be declared package because its underlying type uses an internal type}}
+
+  package var pkgUsesInternalStructProp: InternalStruct { get { } set { } }
+  // expected-error@-1 {{property cannot be declared package because its type uses an internal type}}
+
+
+  public func usesPackageStruct(_: PackageStruct) {}
+  // expected-error@-1 {{method cannot be declared public because its parameter uses a package type}}
+
+  public typealias UsesPackageStructAlias = PackageStruct
+  // expected-error@-1 {{type alias cannot be declared public because its underlying type uses a package type}}
+
+  public var usesPackageStructProp: PackageStruct { get { } set { } }
+  // expected-error@-1 {{property cannot be declared public because its type uses a package type}}
 }