Merge pull request #74940 from tshortli/private-module-interface-flags-6.0

tshortli · web-flow · commit de5d500b367b · 2024-07-03T15:27:27.000-07:00
[6.0] Change interface printing behaviors for `-package-name` and `-project-name`
diff --git a/include/swift/Frontend/ModuleInterfaceSupport.h b/include/swift/Frontend/ModuleInterfaceSupport.h
@@ -21,7 +21,6 @@
 #define SWIFT_COMPILER_VERSION_KEY "swift-compiler-version"
 #define SWIFT_MODULE_FLAGS_KEY "swift-module-flags"
 #define SWIFT_MODULE_FLAGS_IGNORABLE_KEY "swift-module-flags-ignorable"
-#define SWIFT_MODULE_FLAGS_IGNORABLE_PRIVATE_KEY "swift-module-flags-ignorable-private"
 
 namespace swift {
 
@@ -42,18 +41,26 @@ struct ModuleInterfaceOptions {
   /// [TODO: Clang-type-plumbing] This check should go away.
   bool PrintFullConvention = false;
 
-  /// Copy of all the command-line flags passed at .swiftinterface
-  /// generation time, re-applied to CompilerInvocation when reading
-  /// back .swiftinterface and reconstructing .swiftmodule.
-  std::string Flags;
+  struct InterfaceFlags {
+    /// Copy of all the command-line flags passed at .swiftinterface
+    /// generation time, re-applied to CompilerInvocation when reading
+    /// back .swiftinterface and reconstructing .swiftmodule.
+    std::string Flags = "";
 
-  /// Flags that should be emitted to the .swiftinterface file but are OK to be
-  /// ignored by the earlier version of the compiler.
-  std::string IgnorableFlags;
+    /// Flags that should be emitted to the .swiftinterface file but are OK to
+    /// be ignored by the earlier version of the compiler.
+    std::string IgnorableFlags = "";
+  };
 
-  /// Ignorable flags that should only be printed in .private.swiftinterface file;
-  /// e.g. -package-name PACKAGE_ID
-  std::string IgnorablePrivateFlags;
+  /// Flags which appear in all .swiftinterface files.
+  InterfaceFlags PublicFlags = {};
+
+  /// Flags which appear in both the private and package .swiftinterface files,
+  /// but not the public interface.
+  InterfaceFlags PrivateFlags = {};
+
+  /// Flags which appear only in the .package.swiftinterface.
+  InterfaceFlags PackageFlags = {};
 
   /// Print imports with both @_implementationOnly and @_spi, only applies
   /// when PrintSPIs is true.
@@ -62,6 +69,10 @@ struct ModuleInterfaceOptions {
   /// Print imports that are missing from the source and used in API.
   bool PrintMissingImports = true;
 
+  /// If true, package-name flag is not printed in either public or private
+  /// interface file.
+  bool DisablePackageNameForNonPackageInterface = false;
+
   /// Intentionally print invalid syntax into the file.
   bool DebugPrintInvalidSyntax = false;
 
diff --git a/include/swift/Option/Options.h b/include/swift/Option/Options.h
@@ -42,9 +42,8 @@ namespace options {
     SwiftAPIDigesterOption = (1 << 17),
     NewDriverOnlyOption = (1 << 18),
     ModuleInterfaceOptionIgnorable = (1 << 19),
-    ModuleInterfaceOptionIgnorablePrivate = (1 << 20),
-    ArgumentIsFileList = (1 << 21),
-    CacheInvariant = (1 << 22),
+    ArgumentIsFileList = (1 << 20),
+    CacheInvariant = (1 << 21),
   };
 
   enum ID {
diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td
@@ -56,12 +56,6 @@ def ModuleInterfaceOption : OptionFlag;
 // The option can be safely ignored by the older compiler.
 def ModuleInterfaceOptionIgnorable : OptionFlag;
 
-// The option should be written into a .private.swiftinterface or
-// .package.swiftinterface module interface file, and read/parsed from
-// there when reconstituting a .swiftmodule from it.
-// The option can be safely ignored by the older compiler.
-def ModuleInterfaceOptionIgnorablePrivate : OptionFlag;
-
 // The option causes the output of a supplementary output, or is the path option
 // for a supplementary output. E.g., `-emit-module` and `-emit-module-path`.
 def SupplementaryOutput : OptionFlag;
@@ -717,6 +711,11 @@ def disable_bridging_pch : Flag<["-"], "disable-bridging-pch">,
   Flags<[HelpHidden]>,
   HelpText<"Disable automatic generation of bridging PCH files">;
 
+def disable_print_package_name_for_non_package_interface :
+  Flag<["-"], "disable-print-package-name-for-non-package-interface">,
+  Flags<[FrontendOption, NoDriverOption, ModuleInterfaceOption, HelpHidden]>,
+  HelpText<"Disable adding package name to public or private interface">;
+
 def lto : Joined<["-"], "lto=">,
   Flags<[FrontendOption, NoInteractiveOption]>,
   HelpText<"Specify the LTO type to either 'llvm-thin' or 'llvm-full'">;
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -419,6 +419,7 @@ static void ParseModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
     Args.hasArg(OPT_debug_emit_invalid_swiftinterface_syntax);
   Opts.PrintMissingImports =
     !Args.hasArg(OPT_disable_print_missing_imports_in_module_interface);
+  Opts.DisablePackageNameForNonPackageInterface |= Args.hasArg(OPT_disable_print_package_name_for_non_package_interface);
 
   if (const Arg *A = Args.getLastArg(OPT_library_level)) {
     StringRef contents = A->getValue();
@@ -434,6 +435,10 @@ static void ParseModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
 /// Checks if an arg is generally allowed to be included
 /// in a module interface
 static bool ShouldIncludeModuleInterfaceArg(const Arg *A) {
+  if (!A->getOption().hasFlag(options::ModuleInterfaceOption) &&
+      !A->getOption().hasFlag(options::ModuleInterfaceOptionIgnorable))
+    return false;
+
   if (!A->getOption().matches(options::OPT_enable_experimental_feature))
     return true;
 
@@ -444,46 +449,69 @@ static bool ShouldIncludeModuleInterfaceArg(const Arg *A) {
   return true;
 }
 
+static bool IsPackageInterfaceFlag(const Arg *A, ArgList &Args) {
+  return A->getOption().matches(options::OPT_package_name) &&
+         Args.hasArg(
+             options::OPT_disable_print_package_name_for_non_package_interface);
+}
+
+static bool IsPrivateInterfaceFlag(const Arg *A, ArgList &Args) {
+  return A->getOption().matches(options::OPT_project_name);
+}
+
 /// Save a copy of any flags marked as ModuleInterfaceOption, if running
 /// in a mode that is going to emit a .swiftinterface file.
 static void SaveModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
                                     FrontendOptions &FOpts,
                                     ArgList &Args, DiagnosticEngine &Diags) {
   if (!FOpts.InputsAndOutputs.hasModuleInterfaceOutputPath())
     return;
-  ArgStringList RenderedArgs;
-  ArgStringList RenderedArgsIgnorable;
-  ArgStringList RenderedArgsIgnorablePrivate;
+
+  struct RenderedInterfaceArgs {
+    ArgStringList Standard = {};
+    ArgStringList Ignorable = {};
+  };
+
+  RenderedInterfaceArgs PublicArgs{};
+  RenderedInterfaceArgs PrivateArgs{};
+  RenderedInterfaceArgs PackageArgs{};
+
+  auto interfaceArgListForArg = [&](Arg *A) -> ArgStringList & {
+    bool ignorable =
+        A->getOption().hasFlag(options::ModuleInterfaceOptionIgnorable);
+    if (IsPackageInterfaceFlag(A, Args))
+      return ignorable ? PackageArgs.Ignorable : PackageArgs.Standard;
+
+    if (IsPrivateInterfaceFlag(A, Args))
+      return ignorable ? PrivateArgs.Ignorable : PrivateArgs.Standard;
+
+    return ignorable ? PublicArgs.Ignorable : PublicArgs.Standard;
+  };
+
   for (auto A : Args) {
     if (!ShouldIncludeModuleInterfaceArg(A))
       continue;
 
-    if (A->getOption().hasFlag(options::ModuleInterfaceOptionIgnorablePrivate)) {
-      A->render(Args, RenderedArgsIgnorablePrivate);
-    } else if (A->getOption().hasFlag(options::ModuleInterfaceOptionIgnorable)) {
-      A->render(Args, RenderedArgsIgnorable);
-    } else if (A->getOption().hasFlag(options::ModuleInterfaceOption)) {
-      A->render(Args, RenderedArgs);
-    }
-  }
-  {
-    llvm::raw_string_ostream OS(Opts.Flags);
-    interleave(RenderedArgs,
-               [&](const char *Argument) { PrintArg(OS, Argument, StringRef()); },
-               [&] { OS << " "; });
-  }
-  {
-    llvm::raw_string_ostream OS(Opts.IgnorablePrivateFlags);
-    interleave(RenderedArgsIgnorablePrivate,
-               [&](const char *Argument) { PrintArg(OS, Argument, StringRef()); },
-               [&] { OS << " "; });
-  }
-  {
-    llvm::raw_string_ostream OS(Opts.IgnorableFlags);
-    interleave(RenderedArgsIgnorable,
-               [&](const char *Argument) { PrintArg(OS, Argument, StringRef()); },
-               [&] { OS << " "; });
-  }
+    ArgStringList &ArgList = interfaceArgListForArg(A);
+    A->render(Args, ArgList);
+  }
+
+  auto updateInterfaceOpts = [](ModuleInterfaceOptions::InterfaceFlags &Flags,
+                                RenderedInterfaceArgs &RenderedArgs) {
+    auto printFlags = [](std::string &str, ArgStringList argList) {
+      llvm::raw_string_ostream OS(str);
+      interleave(
+          argList,
+          [&](const char *Argument) { PrintArg(OS, Argument, StringRef()); },
+          [&] { OS << " "; });
+    };
+    printFlags(Flags.Flags, RenderedArgs.Standard);
+    printFlags(Flags.IgnorableFlags, RenderedArgs.Ignorable);
+  };
+
+  updateInterfaceOpts(Opts.PublicFlags, PublicArgs);
+  updateInterfaceOpts(Opts.PrivateFlags, PrivateArgs);
+  updateInterfaceOpts(Opts.PackageFlags, PackageArgs);
 }
 
 enum class CxxCompatMode {
diff --git a/lib/Frontend/ModuleInterfaceSupport.cpp b/lib/Frontend/ModuleInterfaceSupport.cpp
@@ -59,8 +59,15 @@ static void printToolVersionAndFlagsComment(raw_ostream &out,
       << InterfaceFormatVersion << "\n";
   out << "// " SWIFT_COMPILER_VERSION_KEY ": "
       << ToolsVersion << "\n";
-  out << "// " SWIFT_MODULE_FLAGS_KEY ": "
-      << Opts.Flags;
+  out << "// " SWIFT_MODULE_FLAGS_KEY ": " << Opts.PublicFlags.Flags;
+
+  if (Opts.InterfaceContentMode >= PrintOptions::InterfaceMode::Private &&
+      !Opts.PrivateFlags.Flags.empty())
+    out << " " << Opts.PrivateFlags.Flags;
+
+  if (Opts.InterfaceContentMode >= PrintOptions::InterfaceMode::Package &&
+      !Opts.PackageFlags.Flags.empty())
+    out << " " << Opts.PackageFlags.Flags;
 
   // Insert additional -module-alias flags
   if (Opts.AliasModuleNames) {
@@ -96,15 +103,28 @@ static void printToolVersionAndFlagsComment(raw_ostream &out,
   }
   out << "\n";
 
-  if (!Opts.IgnorableFlags.empty()) {
-    out << "// " SWIFT_MODULE_FLAGS_IGNORABLE_KEY ": "
-        << Opts.IgnorableFlags << "\n";
-  }
+  // Add swift-module-flags-ignorable: if non-empty.
+  {
+    llvm::SmallVector<StringRef, 4> ignorableFlags;
+
+    if (!Opts.PublicFlags.IgnorableFlags.empty())
+      ignorableFlags.push_back(Opts.PublicFlags.IgnorableFlags);
 
-  auto hasPrivateIgnorableFlags = !Opts.printPublicInterface() && !Opts.IgnorablePrivateFlags.empty();
-  if (hasPrivateIgnorableFlags) {
-    out << "// " SWIFT_MODULE_FLAGS_IGNORABLE_PRIVATE_KEY ": "
-        << Opts.IgnorablePrivateFlags << "\n";
+    if (Opts.InterfaceContentMode >= PrintOptions::InterfaceMode::Private &&
+        !Opts.PrivateFlags.IgnorableFlags.empty())
+      ignorableFlags.push_back(Opts.PrivateFlags.IgnorableFlags);
+
+    if (Opts.InterfaceContentMode >= PrintOptions::InterfaceMode::Package &&
+        !Opts.PackageFlags.IgnorableFlags.empty())
+      ignorableFlags.push_back(Opts.PackageFlags.IgnorableFlags);
+
+    if (!ignorableFlags.empty()) {
+      out << "// " SWIFT_MODULE_FLAGS_IGNORABLE_KEY ": ";
+      llvm::interleave(
+          ignorableFlags, [&out](StringRef str) { out << str; },
+          [&out] { out << " "; });
+      out << "\n";
+    }
   }
 }
 
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
@@ -1058,6 +1058,12 @@ bool AttributeChecker::visitAbstractAccessControlAttr(
       D->getASTContext().LangOpts.PackageName.empty() &&
       File && File->Kind != SourceFileKind::Interface) {
     // `package` modifier used outside of a package.
+    // Error if a source file contains a package decl or `package import` but
+    // no package-name is passed.
+    // Note that if the file containing the package decl is a public (or private)
+    // interface file, the decl must be @usableFromInline (or "inlinable"),
+    // effectively getting "public" visibility; in such case, package-name is
+    // not needed, and typecheck on those decls are skipped.
     diagnose(attr->getLocation(), diag::access_control_requires_package_name,
              isa<ValueDecl>(D), D);
     return true;
diff --git a/test/ModuleInterface/package_interface_disable_package_name.swift b/test/ModuleInterface/package_interface_disable_package_name.swift
@@ -0,0 +1,62 @@
+// RUN: %empty-directory(%t)
+
+/// Do not print package-name for public or private interfaces
+// RUN: %target-build-swift -emit-module %s -I %t \
+// RUN:   -module-name Bar -package-name foopkg \
+// RUN:   -enable-library-evolution -swift-version 6 \
+// RUN:   -package-name barpkg \
+// RUN:   -Xfrontend -disable-print-package-name-for-non-package-interface \
+// RUN:   -emit-module-interface-path %t/Bar.swiftinterface \
+// RUN:   -emit-private-module-interface-path %t/Bar.private.swiftinterface \
+// RUN:   -emit-package-module-interface-path %t/Bar.package.swiftinterface
+
+// RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC < %t/Bar.swiftinterface
+// RUN: %FileCheck %s --check-prefix=CHECK-PRIVATE < %t/Bar.private.swiftinterface
+// RUN: %FileCheck %s --check-prefix=CHECK-PACKAGE < %t/Bar.package.swiftinterface
+
+// CHECK-PUBLIC-NOT: -package-name foopkg
+// CHECK-PUBLIC-NOT: -package-name barpkg
+// CHECK-PRIVATE-NOT: -package-name foopkg
+// CHECK-PRIVATE-NOT: -package-name barpkg
+// CHECK-PACKAGE-NOT: -package-name foopkg
+
+// CHECK-PUBLIC:  -enable-library-evolution -swift-version 6 -disable-print-package-name-for-non-package-interface -module-name Bar
+// CHECK-PRIVATE:  -enable-library-evolution -swift-version 6 -disable-print-package-name-for-non-package-interface -module-name Bar
+// CHECK-PACKAGE:  -enable-library-evolution -swift-version 6 -disable-print-package-name-for-non-package-interface -module-name Bar -package-name barpkg
+
+/// Verify building modules from non-package interfaces succeeds without the package-name flag.
+// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
+// RUN: rm -rf %t/Bar.swiftmodule
+// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.private.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
+// RUN: rm -rf %t/Bar.swiftmodule
+// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.package.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
+
+// RUN: rm -rf %t/Bar.swiftmodule
+// RUN: rm -rf %t/Bar.swiftinterface
+// RUN: rm -rf %t/Bar.private.swiftinterface
+// RUN: rm -rf %t/Bar.package.swiftinterface
+
+/// By default, -package-name is printed in all interfaces.
+// RUN: %target-build-swift -emit-module %s -I %t \
+// RUN:   -module-name Bar -package-name barpkg \
+// RUN:   -enable-library-evolution -swift-version 6 \
+// RUN:   -emit-module-interface-path %t/Bar.swiftinterface \
+// RUN:   -emit-private-module-interface-path %t/Bar.private.swiftinterface \
+// RUN:   -emit-package-module-interface-path %t/Bar.package.swiftinterface
+
+// RUN: %FileCheck %s < %t/Bar.swiftinterface
+// RUN: %FileCheck %s < %t/Bar.private.swiftinterface
+// RUN: %FileCheck %s < %t/Bar.package.swiftinterface
+
+// CHECK: -enable-library-evolution -package-name barpkg -swift-version 6 -module-name Bar
+
+/// Building modules from non-package interfaces with package-name (default mode) should succeed.
+// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
+// RUN: rm -rf %t/Bar.swiftmodule
+// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.private.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
+// RUN: rm -rf %t/Bar.swiftmodule
+// RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.package.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
+
+public struct PubStruct {}
+@_spi(bar) public struct SPIStruct {}
+package struct PkgStruct {}
diff --git a/test/ModuleInterface/project-name.swift b/test/ModuleInterface/project-name.swift
@@ -0,0 +1,23 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend -emit-module %s -I %t \
+// RUN:   -module-name Library -project-name ProjectName \
+// RUN:   -enable-library-evolution -swift-version 5 \
+// RUN:   -emit-module-interface-path %t/Library.swiftinterface \
+// RUN:   -emit-private-module-interface-path %t/Library.private.swiftinterface \
+// RUN:   -emit-package-module-interface-path %t/Library.package.swiftinterface
+
+// RUN: %target-swift-typecheck-module-from-interface(%t/Library.swiftinterface) -module-name Library
+// RUN: %target-swift-typecheck-module-from-interface(%t/Library.private.swiftinterface) -module-name Library
+// RUN: %target-swift-typecheck-module-from-interface(%t/Library.package.swiftinterface) -module-name Library
+
+// RUN: %FileCheck %s < %t/Library.swiftinterface --check-prefix CHECK-PUBLIC
+// RUN: %FileCheck %s < %t/Library.private.swiftinterface --check-prefix CHECK-NONPUBLIC
+// RUN: %FileCheck %s < %t/Library.package.swiftinterface --check-prefix CHECK-NONPUBLIC
+
+// CHECK-PUBLIC-NOT: -project-name
+
+// CHECK-NONPUBLIC: swift-module-flags-ignorable:
+// CHECK-NONPUBLIC-SAME: -project-name ProjectName
+
+public func foo() {}
