Merge pull request #64290 from hyp/eng/fix-interop-xmodule-imports

[interop][SwiftToCxx] do not expose APIs with imported declarations w…

Author: hyp

include/swift/Frontend/FrontendOptions.h

@@ -413,6 +413,16 @@ class FrontendOptions {
   /// header.
   llvm::Optional<ClangHeaderExposeBehavior> ClangHeaderExposedDecls;
 
+  struct ClangHeaderExposedImportedModule {
+    std::string moduleName;
+    std::string headerName;
+  };
+
+  /// Indicates which imported modules have a generated header associated with
+  /// them that can be imported into the generated header for the current
+  /// module.
+  std::vector<ClangHeaderExposedImportedModule> clangHeaderExposedImports;
+
   /// \return true if the given action only parses without doing other compilation steps.
   static bool shouldActionOnlyParse(ActionType);
 

include/swift/Option/FrontendOptions.td

@@ -1106,6 +1106,12 @@ def clang_header_expose_decls:
   Flags<[FrontendOption, HelpHidden]>,
   MetaVarName<"all-public|has-expose-attr">;
 
+def clang_header_expose_module:
+  Separate<["-"], "clang-header-expose-module">,
+  HelpText<"Allow the compiler to assume that APIs from the specified module are exposed to C/C++/Objective-C in another generated header, so that APIs in the current module that depend on declarations from the specified module can be exposed in the generated header.">,
+  Flags<[FrontendOption, HelpHidden]>,
+  MetaVarName<"<imported-module-name>=<generated-header-name>">;
+
 def weak_link_at_target :
   Flag<["-"], "weak-link-at-target">,
   HelpText<"Weakly link symbols for declarations that were introduced at the "

lib/Frontend/ArgsToFrontendOptionsConverter.cpp

@@ -313,7 +313,16 @@ bool ArgsToFrontendOptionsConverter::convert(
                       HasExposeAttrOrImplicitDeps)
             .Default(llvm::None);
   }
-  
+  for (const auto &arg :
+       Args.getAllArgValues(options::OPT_clang_header_expose_module)) {
+    auto splitArg = StringRef(arg).split('=');
+    if (splitArg.second.empty()) {
+      continue;
+    }
+    Opts.clangHeaderExposedImports.push_back(
+        {splitArg.first.str(), splitArg.second.str()});
+  }
+
   Opts.StrictImplicitModuleContext = Args.hasArg(OPT_strict_implicit_module_context,
                                                  OPT_no_strict_implicit_module_context,
                                                  false);

lib/PrintAsClang/DeclAndTypePrinter.cpp

@@ -2779,10 +2779,21 @@ static bool excludeForObjCImplementation(const ValueDecl *VD) {
   return false;
 }
 
+static bool isExposedToThisModule(const ModuleDecl &M, const ValueDecl *VD,
+                                  const llvm::StringSet<> &exposedModules) {
+  if (VD->hasClangNode())
+    return true;
+  auto *mc = VD->getModuleContext();
+  if (mc == &M || mc->isStdlibModule())
+    return true;
+  return exposedModules.count(mc->getName().str());
+}
+
 bool DeclAndTypePrinter::shouldInclude(const ValueDecl *VD) {
   return !VD->isInvalid() && (!requiresExposedAttribute || hasExposeAttr(VD)) &&
          (outputLang == OutputLanguageMode::Cxx
               ? cxx_translation::isVisibleToCxx(VD, minRequiredAccess) &&
+                    isExposedToThisModule(M, VD, exposedModules) &&
                     cxx_translation::isExposableToCxx(VD)
               : isVisibleToObjC(VD, minRequiredAccess)) &&
          !VD->getAttrs().hasAttribute<ImplementationOnlyAttr>() &&

lib/PrintAsClang/DeclAndTypePrinter.h

@@ -18,6 +18,7 @@
 #include "swift/AST/Type.h"
 // for OptionalTypeKind
 #include "swift/ClangImporter/ClangImporter.h"
+#include "llvm/ADT/StringSet.h"
 
 namespace clang {
   class NamedDecl;
@@ -48,6 +49,7 @@ class DeclAndTypePrinter {
   SwiftToClangInteropContext &interopContext;
   AccessLevel minRequiredAccess;
   bool requiresExposedAttribute;
+  llvm::StringSet<> &exposedModules;
   OutputLanguageMode outputLang;
 
   /// The name 'CFTypeRef'.
@@ -64,13 +66,14 @@ class DeclAndTypePrinter {
                      PrimitiveTypeMapping &typeMapping,
                      SwiftToClangInteropContext &interopContext,
                      AccessLevel access, bool requiresExposedAttribute,
+                     llvm::StringSet<> &exposedModules,
                      OutputLanguageMode outputLang)
       : M(mod), os(out), prologueOS(prologueOS),
         outOfLineDefinitionsOS(outOfLineDefinitionsOS), delayedMembers(delayed),
         typeMapping(typeMapping), interopContext(interopContext),
         minRequiredAccess(access),
         requiresExposedAttribute(requiresExposedAttribute),
-        outputLang(outputLang) {}
+        exposedModules(exposedModules), outputLang(outputLang) {}
 
   SwiftToClangInteropContext &getInteropContext() { return interopContext; }
 

lib/PrintAsClang/ModuleContentsWriter.cpp

@@ -140,12 +140,13 @@ class ModuleWriter {
   ModuleWriter(raw_ostream &os, raw_ostream &prologueOS,
                llvm::SmallPtrSetImpl<ImportModuleTy> &imports, ModuleDecl &mod,
                SwiftToClangInteropContext &interopContext, AccessLevel access,
-               bool requiresExposedAttribute, OutputLanguageMode outputLang)
+               bool requiresExposedAttribute, llvm::StringSet<> &exposedModules,
+               OutputLanguageMode outputLang)
       : os(os), imports(imports), M(mod),
         outOfLineDefinitionsOS(outOfLineDefinitions),
         printer(M, os, prologueOS, outOfLineDefinitionsOS, delayedMembers,
                 typeMapping, interopContext, access, requiresExposedAttribute,
-                outputLang),
+                exposedModules, outputLang),
         outputLangMode(outputLang) {}
 
   PrimitiveTypeMapping &getTypeMapping() { return typeMapping; }
@@ -190,11 +191,15 @@ class ModuleWriter {
     }
 
     if (outputLangMode == OutputLanguageMode::Cxx) {
-      // Only add C++ imports in C++ mode for now.
-      if (!D->hasClangNode())
-        return true;
+      // Do not expose compiler private '_ObjC' module.
       if (otherModule->getName().str() == CLANG_HEADER_MODULE_NAME)
         return true;
+      // Add C++ module imports in C++ mode explicitly, to ensure that their
+      // import is always emitted in the header.
+      if (D->hasClangNode()) {
+        if (auto *clangMod = otherModule->findUnderlyingClangModule())
+          imports.insert(clangMod);
+      }
     }
 
     imports.insert(otherModule);
@@ -778,15 +783,16 @@ void swift::printModuleContentsAsObjC(
     raw_ostream &os, llvm::SmallPtrSetImpl<ImportModuleTy> &imports,
     ModuleDecl &M, SwiftToClangInteropContext &interopContext) {
   llvm::raw_null_ostream prologueOS;
+  llvm::StringSet<> exposedModules;
   ModuleWriter(os, prologueOS, imports, M, interopContext, getRequiredAccess(M),
-               /*requiresExposedAttribute=*/false, OutputLanguageMode::ObjC)
+               /*requiresExposedAttribute=*/false, exposedModules,
+               OutputLanguageMode::ObjC)
       .write();
 }
 
 EmittedClangHeaderDependencyInfo swift::printModuleContentsAsCxx(
-    raw_ostream &os,
-    ModuleDecl &M, SwiftToClangInteropContext &interopContext,
-    bool requiresExposedAttribute) {
+    raw_ostream &os, ModuleDecl &M, SwiftToClangInteropContext &interopContext,
+    bool requiresExposedAttribute, llvm::StringSet<> &exposedModules) {
   std::string moduleContentsBuf;
   llvm::raw_string_ostream moduleOS{moduleContentsBuf};
   std::string modulePrologueBuf;
@@ -804,7 +810,7 @@ EmittedClangHeaderDependencyInfo swift::printModuleContentsAsCxx(
   // FIXME: Use getRequiredAccess once @expose is supported.
   ModuleWriter writer(moduleOS, prologueOS, info.imports, M, interopContext,
                       AccessLevel::Public, requiresExposedAttribute,
-                      OutputLanguageMode::Cxx);
+                      exposedModules, OutputLanguageMode::Cxx);
   writer.write();
   info.dependsOnStandardLibrary = writer.isStdlibRequired();
   if (M.isStdlibModule()) {

lib/PrintAsClang/ModuleContentsWriter.h

@@ -17,6 +17,7 @@
 #include "swift/Basic/LLVM.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/StringSet.h"
 
 namespace clang {
   class Module;
@@ -46,10 +47,9 @@ struct EmittedClangHeaderDependencyInfo {
 /// Prints the declarations of \p M to \p os in C++ language mode.
 ///
 /// \returns Dependencies required by this module.
-EmittedClangHeaderDependencyInfo printModuleContentsAsCxx(raw_ostream &os,
-                              ModuleDecl &M,
-                              SwiftToClangInteropContext &interopContext,
-                              bool requiresExposedAttribute);
+EmittedClangHeaderDependencyInfo printModuleContentsAsCxx(
+    raw_ostream &os, ModuleDecl &M, SwiftToClangInteropContext &interopContext,
+    bool requiresExposedAttribute, llvm::StringSet<> &exposedModules);
 
 } // end namespace swift
 

lib/PrintAsClang/PrintAsClang.cpp

@@ -197,18 +197,18 @@ static void writePrologue(raw_ostream &out, ASTContext &ctx,
 }
 
 static int compareImportModulesByName(const ImportModuleTy *left,
-                                      const ImportModuleTy *right) {
+                                      const ImportModuleTy *right, bool isCxx) {
   auto *leftSwiftModule = left->dyn_cast<ModuleDecl *>();
   auto *rightSwiftModule = right->dyn_cast<ModuleDecl *>();
 
   if (leftSwiftModule && !rightSwiftModule)
-    return -compareImportModulesByName(right, left);
+    return -compareImportModulesByName(right, left, isCxx);
 
   if (leftSwiftModule && rightSwiftModule)
     return leftSwiftModule->getName().compare(rightSwiftModule->getName());
 
   auto *leftClangModule = left->get<const clang::Module *>();
-  assert(leftClangModule->isSubModule() &&
+  assert((isCxx || leftClangModule->isSubModule()) &&
          "top-level modules should use a normal swift::ModuleDecl");
   if (rightSwiftModule) {
     // Because the Clang module is a submodule, its full name will never be
@@ -222,7 +222,7 @@ static int compareImportModulesByName(const ImportModuleTy *left,
   }
 
   auto *rightClangModule = right->get<const clang::Module *>();
-  assert(rightClangModule->isSubModule() &&
+  assert((isCxx || rightClangModule->isSubModule()) &&
          "top-level modules should use a normal swift::ModuleDecl");
 
   SmallVector<StringRef, 8> leftReversePath(
@@ -363,12 +363,13 @@ static void collectClangModuleHeaderIncludes(
   }
 }
 
-static void writeImports(raw_ostream &out,
-                         llvm::SmallPtrSetImpl<ImportModuleTy> &imports,
-                         ModuleDecl &M, StringRef bridgingHeader,
-                         const FrontendOptions &frontendOpts,
-                         clang::HeaderSearch &clangHeaderSearchInfo,
-                         bool useCxxImport = false) {
+static void
+writeImports(raw_ostream &out, llvm::SmallPtrSetImpl<ImportModuleTy> &imports,
+             ModuleDecl &M, StringRef bridgingHeader,
+             const FrontendOptions &frontendOpts,
+             clang::HeaderSearch &clangHeaderSearchInfo,
+             const llvm::StringMap<StringRef> &exposedModuleHeaderNames,
+             bool useCxxImport = false) {
   // Note: we can't use has_feature(modules) as it's always enabled in C++20
   // mode.
   out << "#if __has_feature(objc_modules)\n";
@@ -380,8 +381,11 @@ static void writeImports(raw_ostream &out,
   // Sort alphabetically for determinism and consistency.
   SmallVector<ImportModuleTy, 8> sortedImports{imports.begin(),
                                                imports.end()};
-  llvm::array_pod_sort(sortedImports.begin(), sortedImports.end(),
-                       &compareImportModulesByName);
+  std::stable_sort(
+      sortedImports.begin(), sortedImports.end(),
+      [&](const ImportModuleTy &left, const ImportModuleTy &right) -> bool {
+        return compareImportModulesByName(&left, &right, useCxxImport) < 0;
+      });
 
   auto isUnderlyingModule = [&M, bridgingHeader](ModuleDecl *import) -> bool {
     if (bridgingHeader.empty())
@@ -395,7 +399,9 @@ static void writeImports(raw_ostream &out,
   clang::FileSystemOptions fileSystemOptions;
   clang::FileManager fileManager{fileSystemOptions};
 
-  llvm::SmallSet<llvm::SmallString<128>, 10> requiredTextualIncludes;
+  llvm::SmallSet<llvm::SmallString<128>, 10>
+      requiredTextualIncludes; // Only included without modules.
+  llvm::SmallVector<StringRef, 1> textualIncludes; // always included.
   llvm::SmallSet<const clang::Module *, 10> visitedModules;
   llvm::SmallSet<llvm::SmallString<128>, 10> includeDirs;
 
@@ -438,6 +444,14 @@ static void writeImports(raw_ostream &out,
       useCxxImport ? "#pragma clang module import" : "@import";
   for (auto import : sortedImports) {
     if (auto *swiftModule = import.dyn_cast<ModuleDecl *>()) {
+      if (useCxxImport) {
+        // Do not import Swift modules into the C++ section of the generated
+        // header unless explicitly exposed.
+        auto it = exposedModuleHeaderNames.find(swiftModule->getName().str());
+        if (it != exposedModuleHeaderNames.end())
+          textualIncludes.push_back(it->getValue());
+        continue;
+      }
       auto Name = swiftModule->getName();
       if (isUnderlyingModule(swiftModule)) {
         includeUnderlying = true;
@@ -463,7 +477,7 @@ static void writeImports(raw_ostream &out,
       }
     } else {
       const auto *clangModule = import.get<const clang::Module *>();
-      assert(clangModule->isSubModule() &&
+      assert((useCxxImport || clangModule->isSubModule()) &&
              "top-level modules should use a normal swift::ModuleDecl");
       out << importDirective << ' ';
       ModuleDecl::ReverseFullNameIterator(clangModule).printForward(out);
@@ -484,6 +498,9 @@ static void writeImports(raw_ostream &out,
     }
   }
   out << "#endif\n\n";
+  for (const auto header : textualIncludes) {
+    out << "#include <" << header << ">\n";
+  }
 
   if (includeUnderlying) {
     if (bridgingHeader.empty())
@@ -548,8 +565,9 @@ bool swift::printAsClangHeader(raw_ostream &os, ModuleDecl *M,
   printModuleContentsAsObjC(objcModuleContents, imports, *M, interopContext);
   writePrologue(os, M->getASTContext(), computeMacroGuard(M));
   emitObjCConditional(os, [&] {
+    llvm::StringMap<StringRef> exposedModuleHeaderNames;
     writeImports(os, imports, *M, bridgingHeader, frontendOpts,
-                 clangHeaderSearchInfo);
+                 clangHeaderSearchInfo, exposedModuleHeaderNames);
   });
   writePostImportPrologue(os, *M);
   emitObjCConditional(os, [&] { os << objcModuleContents.str(); });
@@ -559,6 +577,11 @@ bool swift::printAsClangHeader(raw_ostream &os, ModuleDecl *M,
                      M->DeclContext::getASTContext().LangOpts.EnableCXXInterop;
     if (!enableCxx)
       return;
+
+    llvm::StringSet<> exposedModules;
+    for (const auto &mod : frontendOpts.clangHeaderExposedImports)
+      exposedModules.insert(mod.moduleName);
+
     // Include the shim header only in the C++ mode.
     ClangSyntaxPrinter(os).printIncludeForShimHeader(
         "_SwiftCxxInteroperability.h");
@@ -586,10 +609,14 @@ bool swift::printAsClangHeader(raw_ostream &os, ModuleDecl *M,
     llvm::raw_string_ostream moduleContents{moduleContentsBuf};
     auto deps = printModuleContentsAsCxx(
         moduleContents, *M, interopContext,
-        /*requiresExposedAttribute=*/requiresExplicitExpose);
+        /*requiresExposedAttribute=*/requiresExplicitExpose, exposedModules);
     // FIXME: In ObjC++ mode, we do not need to reimport duplicate modules.
+    llvm::StringMap<StringRef> exposedModuleHeaderNames;
+    for (const auto &mod : frontendOpts.clangHeaderExposedImports)
+      exposedModuleHeaderNames.insert({mod.moduleName, mod.headerName});
     writeImports(os, deps.imports, *M, bridgingHeader, frontendOpts,
-                 clangHeaderSearchInfo, /*useCxxImport=*/true);
+                 clangHeaderSearchInfo, exposedModuleHeaderNames,
+                 /*useCxxImport=*/true);
     // Embed the standard library directly.
     if (addStdlibDepsInline && deps.dependsOnStandardLibrary) {
       assert(!M->isStdlibModule());
@@ -598,9 +625,9 @@ bool swift::printAsClangHeader(raw_ostream &os, ModuleDecl *M,
       auto macroGuard = computeMacroGuard(M->getASTContext().getStdlibModule());
       os << "#ifndef " << macroGuard << "\n";
       os << "#define " << macroGuard << "\n";
-      printModuleContentsAsCxx(os, *M->getASTContext().getStdlibModule(),
-                               interopContext,
-                               /*requiresExposedAttribute=*/true);
+      printModuleContentsAsCxx(
+          os, *M->getASTContext().getStdlibModule(), interopContext,
+          /*requiresExposedAttribute=*/true, exposedModules);
       os << "#endif // " << macroGuard << "\n";
     }
 

test/Interop/SwiftToCxx/cross-module-refs/do-not-expose-imported-api-by-default.swift

@@ -0,0 +1,27 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend %S/Inputs/structs.swift -module-name Structs -emit-module -emit-module-path %t/Structs.swiftmodule
+
+// RUN: %target-swift-frontend %s -typecheck -module-name UsesStructs -I %t -clang-header-expose-decls=all-public -emit-clang-header-path %t/uses-structs.h
+
+// RUN: %check-interop-cxx-header-in-clang(%t/uses-structs.h)
+// RUN: %FileCheck %s < %t/uses-structs.h
+
+import Structs
+
+public struct StructExposed {
+    public func availableInHeader() -> Int {
+        return 0
+    }
+
+    public func unavailableInHeader(_ y: StructSeveralI64) -> StructSeveralI64 {
+        return y
+    }
+
+    public let unavailableInHeaderProp: StructSeveralI64
+}
+
+public func unavailableInHeaderFunc(_ x: StructSeveralI64) -> StructSeveralI64 {
+    return Structs.passThroughStructSeveralI64(i: 0, x, j: 2)
+}
+
+// CHECK-NOT: unavailableInHeaderFunc

test/Interop/SwiftToCxx/cross-module-refs/imported-enum-refs-in-cxx.swift

@@ -1,15 +1,11 @@
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend %S/Inputs/enums.swift -module-name Enums -emit-module -emit-module-path %t/Enums.swiftmodule -clang-header-expose-decls=all-public -emit-clang-header-path %t/enums.h
 
-// RUN: %target-swift-frontend %s -typecheck -module-name UsesEnums -I %t -clang-header-expose-decls=all-public -emit-clang-header-path %t/uses-enums.h
-
-// FIXME: add import automatically?
-// RUN: echo '#include "enums.h"' > %t/fixed-uses-enums.h
-// RUN: cat %t/uses-enums.h     >> %t/fixed-uses-enums.h
+// RUN: %target-swift-frontend %s -typecheck -module-name UsesEnums -I %t -clang-header-expose-decls=all-public -emit-clang-header-path %t/uses-enums.h -clang-header-expose-module Enums=enums.h
 
 // RUN: %FileCheck %s < %t/uses-enums.h
 
-// RUN: %check-interop-cxx-header-in-clang(%t/fixed-uses-enums.h)
+// RUN: %check-interop-cxx-header-in-clang(-I %t %t/uses-enums.h)
 
 import Enums