Use quoted string for package name input

elsh · elsh · commit 69a2510a5fd5 · 2023-04-12T15:53:25.000-07:00
Print package name flag only in private interface
diff --git a/include/swift/AST/DiagnosticsFrontend.def b/include/swift/AST/DiagnosticsFrontend.def
@@ -189,9 +189,6 @@ ERROR(error_bad_export_as_name,none,
       "export-as name \"%0\" is not a valid identifier",
       (StringRef))
 
-ERROR(error_bad_package_name,none,
-      "package name \"%0\" is not a valid identifier",
-      (StringRef))
 ERROR(error_stdlib_not_found,Fatal,
       "unable to load standard library for target '%0'", (StringRef))
 ERROR(error_module_alias_invalid_format,none,
diff --git a/include/swift/Basic/QuotedString.h b/include/swift/Basic/QuotedString.h
@@ -40,6 +40,8 @@ namespace swift {
       printAsQuotedString(out, string.Text);
       return out;
     }
+
+    std::string getString();
   };
 } // end namespace swift
 
diff --git a/include/swift/Frontend/ModuleInterfaceSupport.h b/include/swift/Frontend/ModuleInterfaceSupport.h
@@ -50,6 +50,10 @@ struct ModuleInterfaceOptions {
   /// 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 PrivateIgnorableFlags;
+
   /// Print for a private swiftinterface file, SPI decls and attributes.
   bool PrintPrivateInterfaceContent = false;
 
diff --git a/include/swift/Option/Options.h b/include/swift/Option/Options.h
@@ -42,6 +42,7 @@ namespace options {
     SwiftAPIDigesterOption = (1 << 17),
     NewDriverOnlyOption = (1 << 18),
     ModuleInterfaceOptionIgnorable = (1 << 19),
+    ModuleInterfaceOptionIgnorablePrivate = (1 << 20),
   };
 
   enum ID {
diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td
@@ -56,6 +56,11 @@ 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 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;
@@ -530,7 +535,7 @@ def module_abi_name : Separate<["-"], "module-abi-name">,
   Flags<[FrontendOption, ModuleInterfaceOption]>,
   HelpText<"ABI name to use for the contents of this module">;
 def package_name : Separate<["-"], "package-name">,
-  Flags<[FrontendOption, ModuleInterfaceOptionIgnorable]>,
+  Flags<[FrontendOption, ModuleInterfaceOptionIgnorablePrivate]>,
   HelpText<"Name of the package the module belongs to">;
 def export_as : Separate<["-"], "export-as">,
   Flags<[FrontendOption, ModuleInterfaceOptionIgnorable]>,
diff --git a/lib/Basic/QuotedString.cpp b/lib/Basic/QuotedString.cpp
@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/ADT/SmallString.h"
 #include "swift/Basic/QuotedString.h"
 
 using namespace swift;
@@ -43,3 +44,10 @@ void swift::printAsQuotedString(llvm::raw_ostream &out, llvm::StringRef text) {
   }
   out << '"';
 }
+
+std::string QuotedString::getString() {
+  llvm::SmallString<128> escapedBuf;
+  llvm::raw_svector_ostream os(escapedBuf);
+  swift::printAsQuotedString(os, Text);
+  return std::string(escapedBuf.str());
+}
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -17,6 +17,7 @@
 #include "swift/AST/DiagnosticsFrontend.h"
 #include "swift/Basic/Feature.h"
 #include "swift/Basic/Platform.h"
+#include "swift/Basic/QuotedString.h"
 #include "swift/Option/Options.h"
 #include "swift/Option/SanitizerOptions.h"
 #include "swift/Parse/ParseVersion.h"
@@ -401,8 +402,19 @@ static void SaveModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
     return;
   ArgStringList RenderedArgs;
   ArgStringList RenderedArgsIgnorable;
+  ArgStringList RenderedArgsIgnorablePrivate;
   for (auto A : Args) {
-    if (A->getOption().hasFlag(options::ModuleInterfaceOptionIgnorable)) {
+    if (A->getOption().hasFlag(options::ModuleInterfaceOptionIgnorablePrivate)) {
+      StringRef str = A->getValue();
+      auto flag = A->getOption();
+      if (flag.getName().startswith("package-name")) {
+        auto quotedStr = QuotedString(str).getString();
+        auto quotedArg = Arg(flag, A->getSpelling(), A->getIndex(), quotedStr.data());
+        quotedArg.render(Args, RenderedArgsIgnorablePrivate);
+      } else {
+        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);
@@ -421,6 +433,12 @@ static void SaveModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
     if (FOpts.ModuleName == "_Concurrency")
       OS << " -disable-availability-checking";
   }
+  {
+    llvm::raw_string_ostream OS(Opts.PrivateIgnorableFlags);
+    interleave(RenderedArgsIgnorablePrivate,
+               [&](const char *Argument) { PrintArg(OS, Argument, StringRef()); },
+               [&] { OS << " "; });
+  }
   {
     llvm::raw_string_ostream OS(Opts.IgnorableFlags);
     interleave(RenderedArgsIgnorable,
@@ -796,10 +814,7 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
 
   if (const Arg *A = Args.getLastArg(OPT_package_name)) {
     auto pkgName = A->getValue();
-    if (!Lexer::isIdentifier(pkgName))
-      Diags.diagnose(SourceLoc(), diag::error_bad_package_name, pkgName);
-    else
-      Opts.PackageName = pkgName;
+    Opts.PackageName = QuotedString(pkgName).getString();
   }
 
   if (const Arg *A = Args.getLastArg(OPT_require_explicit_availability_EQ)) {
diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp
@@ -26,6 +26,7 @@
 #include "swift/Basic/FileTypes.h"
 #include "swift/Basic/SourceManager.h"
 #include "swift/Basic/Statistic.h"
+#include "swift/Basic/QuotedString.h"
 #include "swift/Frontend/ModuleInterfaceLoader.h"
 #include "swift/Parse/Lexer.h"
 #include "swift/SIL/SILModule.h"
@@ -1155,8 +1156,10 @@ ModuleDecl *CompilerInstance::getMainModule() const {
           Invocation.getFrontendOptions().ModuleABIName));
     }
     if (!Invocation.getLangOptions().PackageName.empty()) {
-      MainModule->setPackageName(getASTContext().getIdentifier(
-          Invocation.getLangOptions().PackageName));
+      // Package name input needs to be a quoted string as all unicode chars are allowed
+      // including an escape char and a space.
+      auto pkgName = QuotedString(Invocation.getLangOptions().PackageName).getString();
+      MainModule->setPackageName(getASTContext().getIdentifier(pkgName));
     }
     if (!Invocation.getFrontendOptions().ExportAsName.empty()) {
       MainModule->setExportAsName(getASTContext().getIdentifier(
diff --git a/lib/Frontend/ModuleInterfaceSupport.cpp b/lib/Frontend/ModuleInterfaceSupport.cpp
@@ -94,9 +94,18 @@ static void printToolVersionAndFlagsComment(raw_ostream &out,
   }
   out << "\n";
 
-  if (!Opts.IgnorableFlags.empty()) {
-    out << "// " SWIFT_MODULE_FLAGS_IGNORABLE_KEY ": "
-        << Opts.IgnorableFlags << "\n";
+  auto hasIgnorableFlags = !Opts.IgnorableFlags.empty();
+  auto hasPrivateIgnorableFlags = Opts.PrintPrivateInterfaceContent && !Opts.PrivateIgnorableFlags.empty();
+
+  if (hasIgnorableFlags || hasPrivateIgnorableFlags) {
+    out << "// " SWIFT_MODULE_FLAGS_IGNORABLE_KEY ": ";
+    if (hasIgnorableFlags) {
+      out << Opts.IgnorableFlags;
+    }
+    if (hasPrivateIgnorableFlags) {
+      out << Opts.PrivateIgnorableFlags;
+    }
+    out << "\n";
   }
 }
 
diff --git a/test/Sema/accessibility_package_inline_interface.swift b/test/Sema/accessibility_package_inline_interface.swift
@@ -10,7 +10,6 @@
 
 // RUN: %FileCheck %s -check-prefix CHECK-UTILS < %t/Utils.swiftinterface
 // CHECK-UTILS: -module-name Utils -enable-library-evolution
-// CHECK-UTILS: swift-module-flags-ignorable: -package-name myLib
 // CHECK-UTILS: @usableFromInline
 // CHECK-UTILS: package class PackageKlassProto {
 // CHECK-UTILS:   @usableFromInline
diff --git a/test/Sema/accessibility_package_interface.swift b/test/Sema/accessibility_package_interface.swift
@@ -0,0 +1,94 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -emit-module %t/Utils.swift \
+// RUN:   -module-name Utils -swift-version 5 -I %t \
+// RUN:   -package-name myLib
+// RUN:   -enable-library-evolution \
+// RUN:   -emit-module-path %t/Utils.swiftmodule \
+// RUN:   -emit-module-interface-path %t/Utils.swiftinterface \
+// RUN:   -emit-private-module-interface-path %t/Utils.private.swiftinterface
+
+// RUN: %target-swift-typecheck-module-from-interface(%t/Utils.swiftinterface) -I %t
+// RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC < %t/Utils.swiftinterface
+
+// RUN: %target-swift-typecheck-module-from-interface(%t/Utils.private.swiftinterface) -module-name Utils -I %t
+// RUN: %FileCheck %s --check-prefix=CHECK-PRIVATE < %t/Utils.private.swiftinterface
+
+// CHECK-PUBLIC: -module-name Utils -enable-library-evolution
+// CHECK-PRIVATE: swift-module-flags-ignorable: -package-name myLib
+
+
+//--- Utils.swift
+package protocol PackageProto {
+  var pkgVar: Double { get set }
+  func pkgFunc()
+}
+
+package class PackageKlass {
+  package init() {}
+  package private(set) var pkgVar: Double = 1.0
+  package func pkgFunc() {}
+}
+
+@usableFromInline
+package class PackageKlassProto: PackageProto {
+  @usableFromInline package init() {}
+  @usableFromInline package var pkgVar = 1.0
+  package func pkgFunc() {}
+}
+
+@usableFromInline
+package class PackageKlassForInline {
+  @usableFromInline package init() {}
+  @usableFromInline package func foo1() {}
+  package func foo2() {}
+}
+
+protocol InternalProto {
+  var internalVar: Double { get set }
+  func internalFunc()
+}
+class InternalKlass {
+  init() {}
+}
+@usableFromInline
+class InternalKlassProto: InternalProto {
+  @usableFromInline init() {}
+  @usableFromInline var internalVar = 1.0
+  func internalFunc() {}
+}
+
+@usableFromInline
+class InternalKlassForInline {
+  @usableFromInline init() {}
+  @usableFromInline func bar1() {}
+  func bar2() {}
+}
+
+@inlinable
+public func publicFunc() {
+  let a = PackageKlassProto().pkgVar
+  let b = InternalKlassProto().internalVar
+  PackageKlassForInline().foo1()
+  InternalKlassForInline().bar1()
+  print(a, b)
+}
+
+@inlinable
+func internalFunc() {
+  let a = PackageKlassProto().pkgVar
+  let b = InternalKlassProto().internalVar
+  PackageKlassForInline().foo1()
+  InternalKlassForInline().bar1()
+  print(a, b)
+}
+
+@inlinable
+package func packageFunc() {
+  let a = PackageKlassProto().pkgVar
+  let b = InternalKlassProto().internalVar
+  PackageKlassForInline().foo1()
+  InternalKlassForInline().bar1()
+  print(a, b)
+}
diff --git a/test/diagnostics/package-name-diagnostics.swift b/test/diagnostics/package-name-diagnostics.swift
@@ -18,7 +18,7 @@
 // Package name can be same as the module name
 // RUN: %target-swift-frontend -module-name Logging -package-name Logging %s -emit-module -emit-module-path %t/Logging.swiftmodule
 // RUN: test -f %t/Logging.swiftmodule
-
+ 
 // Package name can be a standard library name
 // RUN: %target-swift-frontend -module-name Logging -package-name Swift %s -emit-module -emit-module-path %t/Logging.swiftmodule
 // RUN: test -f %t/Logging.swiftmodule

Original file line number	Diff line number	Diff line change
`@@ -40,6 +40,8 @@ namespace swift {`
`40`	`40`	`printAsQuotedString(out, string.Text);`
`41`	`41`	`return out;`
`42`	`42`	`}`
	`43`	`+`
	`44`	`+ std::string getString();`
`43`	`45`	`};`
`44`	`46`	`} // end namespace swift`
`45`	`47`