[Modules] Avoid false swift module sharing

cachemeifyoucan · cachemeifyoucan · commit a32dd9539b3d · 2024-07-10T14:48:12.000-07:00
When the swiftmodule is built with different clang importer arguments,
they can have the same module hash, causing them to be wrongly re-used even
they contains different interfaces. Add ReducedExtraArgs to the module hash to
disambiguate them.

However, some Xcc arguments, most commonly `-D` options do not affect the
swiftmodule being generated. Do not pass `-Xcc -DARGS` to swift
interface compilation to reduce the amount of module variants in the
build.

rdar://131408266
diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h
@@ -517,7 +517,8 @@ class ClangImporter final : public ClangModuleLoader {
   std::string getClangModuleHash() const;
 
   /// Get clang import creation cc1 args for swift explicit module build.
-  std::vector<std::string> getSwiftExplicitModuleDirectCC1Args() const;
+  std::vector<std::string>
+  getSwiftExplicitModuleDirectCC1Args(bool isInterface) const;
 
   /// If we already imported a given decl successfully, return the corresponding
   /// Swift decl as an Optional<Decl *>, but if we previously tried and failed
diff --git a/lib/Basic/LangOptions.cpp b/lib/Basic/LangOptions.cpp
@@ -726,22 +726,23 @@ namespace {
         "-working-directory=",
         "-working-directory"};
 
-constexpr std::array<std::string_view, 15> knownClangDependencyIgnorablePrefiexes =
-     {"-I",
-      "-F",
-      "-fmodule-map-file=",
-      "-iquote",
-      "-idirafter",
-      "-iframeworkwithsysroot",
-      "-iframework",
-      "-iprefix",
-      "-iwithprefixbefore",
-      "-iwithprefix",
-      "-isystemafter",
-      "-isystem",
-      "-isysroot",
-      "-working-directory=",
-      "-working-directory"};
+  constexpr std::array<std::string_view, 16>
+      knownClangDependencyIgnorablePrefiexes = {"-I",
+                                                "-F",
+                                                "-fmodule-map-file=",
+                                                "-iquote",
+                                                "-idirafter",
+                                                "-iframeworkwithsysroot",
+                                                "-iframework",
+                                                "-iprefix",
+                                                "-iwithprefixbefore",
+                                                "-iwithprefix",
+                                                "-isystemafter",
+                                                "-isystem",
+                                                "-isysroot",
+                                                "-working-directory=",
+                                                "-working-directory",
+                                                "-D"};
 }
 
 std::vector<std::string> ClangImporterOptions::getRemappedExtraArgs(
diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp
@@ -4070,7 +4070,7 @@ std::string ClangImporter::getClangModuleHash() const {
 }
 
 std::vector<std::string>
-ClangImporter::getSwiftExplicitModuleDirectCC1Args() const {
+ClangImporter::getSwiftExplicitModuleDirectCC1Args(bool isInterface) const {
   llvm::SmallVector<const char*> clangArgs;
   clangArgs.reserve(Impl.ClangArgs.size());
   llvm::for_each(Impl.ClangArgs, [&](const std::string &Arg) {
@@ -4120,6 +4120,14 @@ ClangImporter::getSwiftExplicitModuleDirectCC1Args() const {
   PPOpts.MacroIncludes.clear();
   PPOpts.Includes.clear();
 
+  // Clear specific options that will not affect swiftinterface compilation, but
+  // might affect main Module.
+  if (isInterface) {
+    // Interfacefile should not need `-D` but pass to main module in case it
+    // needs to directly import clang headers.
+    PPOpts.Macros.clear();
+  }
+
   if (Impl.SwiftContext.ClangImporterOpts.UseClangIncludeTree) {
     // FileSystemOptions.
     auto &FSOpts = instance.getFileSystemOpts();
diff --git a/lib/DependencyScan/ModuleDependencyScanner.cpp b/lib/DependencyScan/ModuleDependencyScanner.cpp
@@ -381,7 +381,8 @@ ModuleDependencyScanner::getMainModuleDependencyInfo(ModuleDecl *mainModule) {
   std::vector<std::string> buildArgs;
   if (ScanASTContext.ClangImporterOpts.ClangImporterDirectCC1Scan) {
     buildArgs.push_back("-direct-clang-cc1-module-build");
-    for (auto &arg : clangImporter->getSwiftExplicitModuleDirectCC1Args()) {
+    for (auto &arg : clangImporter->getSwiftExplicitModuleDirectCC1Args(
+             /*isInterface=*/false)) {
       buildArgs.push_back("-Xcc");
       buildArgs.push_back(arg);
     }
diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp
@@ -2072,6 +2072,8 @@ InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath,
   auto normalizedTargetTriple =
       getTargetSpecificModuleTriple(genericSubInvocation.getLangOptions().Target);
   std::string sdkBuildVersion = getSDKBuildVersion(sdkPath);
+  const auto ExtraArgs = genericSubInvocation.getClangImporterOptions()
+                             .getReducedExtraArgsForSwiftModuleDependency();
 
   llvm::hash_code H = hash_combine(
       // Start with the compiler version (which will be either tag names or
@@ -2113,6 +2115,10 @@ InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath,
       // correctly load the dependencies.
       genericSubInvocation.getCASOptions().getModuleScanningHashComponents(),
 
+      // Clang ExtraArgs that affects how clang types are imported into swift
+      // module.
+      llvm::hash_combine_range(ExtraArgs.begin(), ExtraArgs.end()),
+
       // Whether or not OSSA modules are enabled.
       //
       // If OSSA modules are enabled, we use a separate namespace of modules to
diff --git a/lib/Serialization/ScanningLoaders.cpp b/lib/Serialization/ScanningLoaders.cpp
@@ -186,7 +186,8 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath,
           Args.push_back("-direct-clang-cc1-module-build");
           auto *importer =
               static_cast<ClangImporter *>(Ctx.getClangModuleLoader());
-          for (auto &Arg : importer->getSwiftExplicitModuleDirectCC1Args()) {
+          for (auto &Arg : importer->getSwiftExplicitModuleDirectCC1Args(
+                   /*isInterface=*/true)) {
             Args.push_back("-Xcc");
             Args.push_back(Arg);
           }
diff --git a/test/CAS/Xcc_args.swift b/test/CAS/Xcc_args.swift
@@ -36,7 +36,8 @@
 private import _Macro
 
 public func test() {
-  let _ = VERSION
+  // Check the VERSION is from command-line, thus a Int32, not string.
+  let _ : Int32 = VERSION
 }
 
 //--- include/module.modulemap
@@ -49,7 +50,7 @@ module _Macro {
 #if defined(_VERSION)
 #define VERSION _VERSION
 #else
-#define VERSION 0
+#define VERSION "not available"
 #endif
 
 //--- hmap.json
diff --git a/test/ScanDependencies/module_hash.swift b/test/ScanDependencies/module_hash.swift
@@ -0,0 +1,73 @@
+// REQUIRES: objc_interop
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
+// RUN:   -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
+// RUN:   -o %t/deps-1.json -I %t/include
+
+// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
+// RUN:   -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
+// RUN:   -o %t/deps-2.json -Xcc -DHAS_FOO=1 -I %t/include
+
+// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
+// RUN:   -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
+// RUN:   -o %t/deps-3.json -Xcc -fapplication-extension -I %t/include
+
+/// Check module hash for the swiftmodule. 1 and 2 should match, but not 3.
+// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-1.json Library modulePath > %t/path-1
+// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-2.json Library modulePath > %t/path-2
+// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-3.json Library modulePath > %t/path-3
+// RUN: diff %t/path-1 %t/path-2
+// RUN: not diff %t/path-1 %t/path-3
+
+/// Check build command (exclude dependency module file path). 1 and 2 should match, but not 3.
+// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-1.json Library | grep -v fmodule-file= > %t/lib-1.cmd
+// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-2.json Library | grep -v fmodule-file= > %t/lib-2.cmd
+// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-3.json Library | grep -v fmodule-file= > %t/lib-3.cmd
+// RUN: diff %t/lib-1.cmd %t/lib-2.cmd
+// RUN: not diff %t/lib-1.cmd %t/lib-3.cmd
+
+/// Test direct-cc1 mode.
+// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
+// RUN:   -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
+// RUN:   -o %t/deps-4.json -I %t/include -experimental-clang-importer-direct-cc1-scan
+// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
+// RUN:   -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
+// RUN:   -o %t/deps-5.json -Xcc -DHAS_FOO=1 -I %t/include -experimental-clang-importer-direct-cc1-scan
+// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
+// RUN:   -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
+// RUN:   -o %t/deps-6.json -Xcc -fapplication-extension -I %t/include -experimental-clang-importer-direct-cc1-scan
+
+// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-4.json Library modulePath > %t/path-4
+// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-5.json Library modulePath > %t/path-5
+// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-6.json Library modulePath > %t/path-6
+// RUN: diff %t/path-4 %t/path-5
+// RUN: not diff %t/path-4 %t/path-6
+// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-4.json Library | grep -v fmodule-file= > %t/lib-4.cmd
+// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-5.json Library | grep -v fmodule-file= > %t/lib-5.cmd
+// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-6.json Library | grep -v fmodule-file= > %t/lib-6.cmd
+// RUN: diff %t/lib-4.cmd %t/lib-5.cmd
+// RUN: not diff %t/lib-4.cmd %t/lib-6.cmd
+
+//--- main.swift
+import Library
+
+//--- include/Library.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -module-name Library -O -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -user-module-version 1.0
+import Swift
+@_exported import A
+public func test() {}
+
+//--- include/a.h
+#ifdef HAS_FOO
+void foo(void);
+#endif
+void bar(void);
+
+//--- include/module.modulemap
+module A {
+  header "a.h"
+  export *
+}
