Merge pull request #61919 from artemcm/InheritExtraClangStateForInterfaceSubInvocation

Inherit parent's extra Clang arguments when creating an interface build sub-invocation.

Author: artemcm

include/swift/Frontend/FrontendOptions.h

@@ -345,6 +345,11 @@ class FrontendOptions {
   /// are errors. The resulting serialized AST may include errors types and
   /// skip nodes entirely, depending on the errors involved.
   bool AllowModuleWithCompilerErrors = false;
+  
+  /// Whether or not the compiler must be strict in ensuring that implicit downstream
+  /// module dependency build tasks must inherit the parent compiler invocation's context,
+  /// such as `-Xcc` flags, etc.
+  bool StrictImplicitModuleContext = false;
 
   /// Downgrade all errors emitted in the module interface verification phase
   /// to warnings.

include/swift/Frontend/ModuleInterfaceLoader.h

@@ -300,13 +300,15 @@ struct ModuleInterfaceLoaderOptions {
   bool disableImplicitSwiftModule = false;
   bool disableBuildingInterface = false;
   bool downgradeInterfaceVerificationError = false;
+  bool strictImplicitModuleContext = false;
   std::string mainExecutablePath;
   ModuleInterfaceLoaderOptions(const FrontendOptions &Opts):
     remarkOnRebuildFromInterface(Opts.RemarkOnRebuildFromModuleInterface),
     disableInterfaceLock(Opts.DisableInterfaceFileLock),
     disableImplicitSwiftModule(Opts.DisableImplicitModules),
     disableBuildingInterface(Opts.DisableBuildingInterface),
     downgradeInterfaceVerificationError(Opts.DowngradeInterfaceVerificationError),
+    strictImplicitModuleContext(Opts.StrictImplicitModuleContext),
     mainExecutablePath(Opts.MainExecutablePath)
   {
     switch (Opts.RequestedAction) {
@@ -479,7 +481,8 @@ struct InterfaceSubContextDelegateImpl: InterfaceSubContextDelegate {
   }
   void
   inheritOptionsForBuildingInterface(const SearchPathOptions &SearchPathOpts,
-                                     const LangOptions &LangOpts, bool suppressRemarks,
+                                     const LangOptions &LangOpts,
+                                     bool suppressRemarks,
                                      RequireOSSAModules_t requireOSSAModules);
   bool extractSwiftInterfaceVersionAndArgs(CompilerInvocation &subInvocation,
                                            SmallVectorImpl<const char *> &SubArgs,

include/swift/Option/Options.td

@@ -167,6 +167,16 @@ def verify_incremental_dependencies :
   Flag<["-"], "verify-incremental-dependencies">,
   Flags<[FrontendOption, HelpHidden]>,
   HelpText<"Enable the dependency verifier for each frontend job">;
+  
+def strict_implicit_module_context :
+  Flag<["-"], "strict-implicit-module-context">,
+  Flags<[FrontendOption, HelpHidden]>,
+  HelpText<"Enable the strict forwarding of compilation context to downstream implicit module dependencies">;
+  
+def no_strict_implicit_module_context :
+  Flag<["-"], "no-strict-implicit-module-context">,
+  Flags<[FrontendOption, HelpHidden]>,
+  HelpText<"Disable the strict forwarding of compilation context to downstream implicit module dependencies">;
 
 def disallow_forwarding_driver :
   Flag<["-"], "disallow-use-new-driver">, Flags<[]>,

lib/Frontend/ArgsToFrontendOptionsConverter.cpp

@@ -293,6 +293,10 @@ bool ArgsToFrontendOptionsConverter::convert(
   }
   Opts.EnableExperimentalCxxInteropInClangHeader =
       Args.hasArg(OPT_enable_experimental_cxx_interop_in_clang_header);
+  
+  Opts.StrictImplicitModuleContext = Args.hasArg(OPT_strict_implicit_module_context,
+                                                 OPT_no_strict_implicit_module_context,
+                                                 false);
 
   computeImportObjCHeaderOptions();
   computeImplicitImportModuleNames(OPT_import_module, /*isTestable=*/false);

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -1598,18 +1598,11 @@ InterfaceSubContextDelegateImpl::InterfaceSubContextDelegateImpl(
   subClangImporterOpts.DetailedPreprocessingRecord =
     clangImporterOpts.DetailedPreprocessingRecord;
 
-  // We need to add these extra clang flags because explicit module building
-  // related flags are all there: -fno-implicit-modules, -fmodule-map-file=,
-  // and -fmodule-file=.
-  // If we don't add these flags, the interface will be built with implicit
-  // PCMs.
-  // FIXME: With Implicit Module Builds, if sub-invocations inherit `-fmodule-map-file=` options,
-  // those modulemaps become File dependencies of all downstream PCMs and their depending Swift
-  // modules, triggering unnecessary re-builds. We work around this by only inheriting these options
-  // when building with explicit modules. While this problem will not manifest with Explicit Modules
-  // (which do not use the ClangImporter to build PCMs), we may still need a better way to
-  // decide which options must be inherited here.
-  if (LoaderOpts.disableImplicitSwiftModule) {
+  // If the compiler has been asked to be strict with ensuring downstream dependencies
+  // get the parent invocation's context, or this is an Explicit build, inherit the
+  // extra Clang arguments also.
+  if (LoaderOpts.strictImplicitModuleContext || LoaderOpts.disableImplicitSwiftModule) {
+    // Inherit any clang-specific state of the compilation (macros, clang flags, etc.)
     subClangImporterOpts.ExtraArgs = clangImporterOpts.ExtraArgs;
     for (auto arg : subClangImporterOpts.ExtraArgs) {
       GenericArgs.push_back("-Xcc");

test/ModuleInterface/Inputs/macro-only-module/ImportsMacroSpecificClangModule.swiftinterface

@@ -0,0 +1,4 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -enable-library-evolution -module-name ImportsMacroSpecificClangModule
+
+import OnlyWithMacro

test/ModuleInterface/Inputs/macro-only-module/OnlyWithMacro.h

@@ -0,0 +1,4 @@
+#if TANGERINE
+#else
+#error ("Macro TANGERINE is required to compile.")
+#endif

test/ModuleInterface/Inputs/macro-only-module/module.modulemap

@@ -0,0 +1 @@
+module OnlyWithMacro { header "OnlyWithMacro.h" }

test/ModuleInterface/clang-args-transitive-availability.swift

@@ -0,0 +1,51 @@
+// This test ensures that the parent invocation's '-Xcc X' flags are inherited when building dependency modules
+// RUN: %empty-directory(%t)
+
+// Just running a compile is useful to make sure it succeeds because that means the transitive Clang module dependency
+// received the TANGERINE macro
+// RUN: %target-swift-frontend -typecheck -strict-implicit-module-context %s -I %S/Inputs/macro-only-module -Xcc -DTANGERINE=1 -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import
+
+// RUN: %target-swift-frontend -scan-dependencies -strict-implicit-module-context %s -o %t/deps.json -I %S/Inputs/macro-only-module -Xcc -DTANGERINE=1 -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import
+// RUN: %FileCheck %s < %t/deps.json
+
+import ImportsMacroSpecificClangModule
+
+// CHECK:      "directDependencies": [
+// CHECK-NEXT:        {
+// CHECK-DAG:          "swift": "ImportsMacroSpecificClangModule"
+// CHECK-NEXT:        },
+// CHECK-NEXT:        {
+// CHECK-DAG:          "swift": "Swift"
+// CHECK-NEXT:        },
+// CHECK-NEXT:        {
+// CHECK-DAG:          "swift": "SwiftOnoneSupport"
+// CHECK-NEXT:        }
+// CHECK-NEXT:      ],
+
+//CHECK:      "swift": "ImportsMacroSpecificClangModule"
+//CHECK-NEXT:    },
+//CHECK-NEXT:    {
+//CHECK-NEXT:      "modulePath": "ImportsMacroSpecificClangModule.swiftmodule",
+//CHECK-NEXT:      "sourceFiles": [
+//CHECK-NEXT:      ],
+//CHECK-NEXT:      "directDependencies": [
+//CHECK-NEXT:        {
+//CHECK-NEXT:          "clang": "OnlyWithMacro"
+
+// CHECK:      "clang": "OnlyWithMacro"
+// CHECK-NEXT:    },
+// CHECK-NEXT:    {
+// CHECK-NEXT:      "modulePath": "OnlyWithMacro.pcm",
+// CHECK-NEXT:      "sourceFiles": [
+// CHECK-DAG:        "{{.*}}OnlyWithMacro.h"
+// CHECK-DAG:        "{{.*}}module.modulemap"
+// CHECK-NEXT:      ],
+// CHECK-NEXT:      "directDependencies": [
+// CHECK-NEXT:      ],
+// CHECK-NEXT:      "details": {
+// CHECK-NEXT:        "clang": {
+// CHECK-NEXT:          "moduleMapPath": "{{.*}}module.modulemap",
+// CHECK-NEXT:          "contextHash": "{{.*}}",
+// CHECK-NEXT:          "commandLine": [
+
+// CHECK:                  "TANGERINE=1",