[Modules] Allow implicit conversions when loading interfaces with invalid os versions.

Initially, the compiler rejected building dependencies that contained OS
versions in an invalid range. This happens to be quite disruptive, so
instead allow it and request that these versions be implicitly bumped
based on what `llvm::getCanonicalVersionForOS` computes

Author: cyndyishida

include/swift/AST/DiagnosticsSema.def

@@ -895,10 +895,6 @@ ERROR(map_os_version_from_textual_interface_failed,none,
       "failed to map OS version from %0 to %1 in %2",
       (StringRef, StringRef, StringRef))
 
-ERROR(target_os_version_from_textual_interface_invalid,none,
-      "invalid target triple %0 in %1",
-      (StringRef, StringRef))
-
 ERROR(serialization_load_failed,Fatal,
       "failed to load module '%0'", (StringRef))
 ERROR(module_interface_build_failed,Fatal,

include/swift/Basic/Platform.h

@@ -123,6 +123,16 @@ namespace swift {
   llvm::VersionTuple getTargetSDKVersion(clang::DarwinSDKInfo &SDKInfo,
                                          const llvm::Triple &triple);
 
+  /// Compute a target triple that is canonicalized using the passed triple.
+  /// \returns nullopt if computation fails.
+  std::optional<llvm::Triple> getCanonicalTriple(const llvm::Triple &triple);
+
+  /// Compare triples for equality but also including OSVersion.
+  inline bool areTriplesEqual(const llvm::Triple &lhs,
+                              const llvm::Triple &rhs) {
+    return (lhs == rhs) && (lhs.getOSVersion() == rhs.getOSVersion());
+  }
+
   /// Get SDK build version.
   std::string getSDKBuildVersion(StringRef SDKPath);
   std::string getSDKBuildVersionFromPlist(StringRef Path);

lib/Basic/Platform.cpp

@@ -848,6 +848,37 @@ llvm::VersionTuple swift::getTargetSDKVersion(clang::DarwinSDKInfo &SDKInfo,
   return SDKVersion;
 }
 
+std::optional<llvm::Triple>
+swift::getCanonicalTriple(const llvm::Triple &triple) {
+  llvm::Triple Result = triple;
+  // Non-darwin targets do not require canonicalization.
+  if (!triple.isOSDarwin())
+    return Result;
+
+  // If the OS versions stay the same, return back the same triple.
+  const llvm::VersionTuple inputOSVersion = triple.getOSVersion();
+  const bool isOSVersionInValidRange =
+      llvm::Triple::isValidVersionForOS(triple.getOS(), inputOSVersion);
+  const llvm::VersionTuple canonicalVersion =
+      llvm::Triple::getCanonicalVersionForOS(
+          triple.getOS(), triple.getOSVersion(), isOSVersionInValidRange);
+  if (canonicalVersion == triple.getOSVersion())
+    return Result;
+
+  const std::string inputOSName = triple.getOSName().str();
+  const std::string inputOSVersionAsStr = inputOSVersion.getAsString();
+  const int platformNameLength =
+      inputOSName.size() - inputOSVersionAsStr.size();
+  if (!StringRef(inputOSName).ends_with(inputOSVersionAsStr) ||
+      (platformNameLength <= 0))
+    return std::nullopt;
+
+  llvm::SmallString<64> buffer(inputOSName.substr(0, platformNameLength));
+  buffer.append(canonicalVersion.getAsString());
+  Result.setOSName(buffer.str());
+  return Result;
+}
+
 static std::string getPlistEntry(const llvm::Twine &Path, StringRef KeyName) {
   auto BufOrErr = llvm::MemoryBuffer::getFile(Path);
   if (!BufOrErr) {

lib/ClangImporter/ClangImporter.cpp

@@ -478,6 +478,12 @@ void importer::getNormalInvocationArguments(
   if (LangOpts.ClangTarget.has_value() && !ignoreClangTarget) {
     triple = LangOpts.ClangTarget.value();
   }
+  auto canonicalTriple = getCanonicalTriple(triple);
+  const bool updateTargetTriple =
+      canonicalTriple.has_value() && !areTriplesEqual(*canonicalTriple, triple);
+  if (updateTargetTriple)
+    triple = *canonicalTriple;
+
   SearchPathOptions &searchPathOpts = ctx.SearchPathOpts;
   ClangImporterOptions &importerOpts = ctx.ClangImporterOpts;
   auto languageVersion = ctx.LangOpts.EffectiveLanguageVersion;
@@ -578,8 +584,9 @@ void importer::getNormalInvocationArguments(
   if (LangOpts.hasFeature(Feature::SafeInteropWrappers))
     invocationArgStrs.push_back("-fexperimental-bounds-safety-attributes");
 
-  // Set C language options.
+  std::optional<llvm::Triple> variantTripleToUpdate = std::nullopt;
   if (triple.isOSDarwin()) {
+    // Set C language options.
     invocationArgStrs.insert(invocationArgStrs.end(), {
       // Avoid including the iso646.h header because some headers from OS X
       // frameworks are broken by it.
@@ -648,6 +655,17 @@ void importer::getNormalInvocationArguments(
                                               1}), // b
       });
     }
+
+    if (auto variantTriple = ctx.LangOpts.TargetVariant) {
+      if (ctx.LangOpts.ClangTargetVariant.has_value() && !ignoreClangTarget)
+        variantTriple = ctx.LangOpts.ClangTargetVariant.value();
+
+      auto canonicalVariantTriple = getCanonicalTriple(*variantTriple);
+      if (canonicalVariantTriple.has_value() &&
+          !areTriplesEqual(*canonicalVariantTriple, *variantTriple)) {
+        *variantTripleToUpdate = *canonicalVariantTriple;
+      }
+    }
   } else {
     // Ideally we should turn this on for all Glibc targets that are actually
     // using Glibc or a libc that respects that flag. This will cause some
@@ -781,6 +799,17 @@ void importer::getNormalInvocationArguments(
     invocationArgStrs.push_back("-iapinotes-modules");
     invocationArgStrs.push_back(path.str().str());
   }
+
+  // If the target triple needs to be replaced, pass the argument at the end of
+  // the invocation.
+  if (updateTargetTriple) {
+    invocationArgStrs.push_back("-target");
+    invocationArgStrs.push_back(triple.str());
+  }
+  if (variantTripleToUpdate.has_value()) {
+    invocationArgStrs.push_back("-darwin-target-variant");
+    invocationArgStrs.push_back(variantTripleToUpdate->str());
+  }
 }
 
 static void
@@ -808,6 +837,10 @@ importer::addCommonInvocationArguments(
   if (ctx.LangOpts.ClangTarget.has_value() && !ignoreClangTarget) {
     triple = ctx.LangOpts.ClangTarget.value();
   }
+  auto canonicalTriple = getCanonicalTriple(triple);
+  if (canonicalTriple.has_value() && !areTriplesEqual(*canonicalTriple, triple))
+    triple = *canonicalTriple;
+
   SearchPathOptions &searchPathOpts = ctx.SearchPathOpts;
   const ClangImporterOptions &importerOpts = ctx.ClangImporterOpts;
 
@@ -856,6 +889,12 @@ importer::addCommonInvocationArguments(
       invocationArgStrs.push_back("-darwin-target-variant");
       if (ctx.LangOpts.ClangTargetVariant.has_value() && !ignoreClangTarget)
         variantTriple = ctx.LangOpts.ClangTargetVariant.value();
+
+      auto canonicalVariantTriple = getCanonicalTriple(*variantTriple);
+      if (canonicalVariantTriple.has_value() &&
+          !areTriplesEqual(*canonicalVariantTriple, *variantTriple))
+        *variantTriple = *canonicalVariantTriple;
+
       invocationArgStrs.push_back(variantTriple->str());
     }
 

lib/Serialization/SerializedModuleLoader.cpp

@@ -1497,43 +1497,27 @@ bool swift::extractCompilerFlagsFromInterface(
       shouldModify = true;
     }
 
-    // Diagnose if the version in the target triple parsed from the
-    // swiftinterface is invalid for the OS.
-    const llvm::VersionTuple originalVer = triple.getOSVersion();
-    bool isValidVersion =
-        llvm::Triple::isValidVersionForOS(triple.getOS(), originalVer);
-    if (!isValidVersion) {
+    // Canonicalize the version in the target triple parsed from the
+    // swiftinterface.
+    auto canonicalTriple = getCanonicalTriple(triple);
+    if (!canonicalTriple.has_value()) {
       if (Diag) {
+        const llvm::VersionTuple OSVersion = triple.getOSVersion();
+        const bool isOSVersionInValidRange =
+            llvm::Triple::isValidVersionForOS(triple.getOS(), OSVersion);
+        const llvm::VersionTuple canonicalVersion =
+            llvm::Triple::getCanonicalVersionForOS(
+                triple.getOS(), triple.getOSVersion(), isOSVersionInValidRange);
         Diag->diagnose(SourceLoc(),
-                       diag::target_os_version_from_textual_interface_invalid,
-                       triple.str(), interfacePath);
+                       diag::map_os_version_from_textual_interface_failed,
+                       OSVersion.getAsString(), canonicalVersion.getAsString(),
+                       interfacePath);
       }
       break;
     }
-
-    // Canonicalize the version in the target triple parsed from the
-    // swiftinterface.
-    llvm::VersionTuple newVer = llvm::Triple::getCanonicalVersionForOS(
-        triple.getOS(), originalVer, isValidVersion);
-    if (originalVer != newVer) {
-      std::string originalOSName = triple.getOSName().str();
-      std::string originalVerStr = originalVer.getAsString();
-      std::string newVerStr = newVer.getAsString();
-      const int OSNameWithoutVersionLength =
-          originalOSName.size() - originalVerStr.size();
-      if (!StringRef(originalOSName).ends_with(originalVerStr) ||
-          (OSNameWithoutVersionLength <= 0)) {
-        if (Diag) {
-          Diag->diagnose(SourceLoc(),
-                         diag::map_os_version_from_textual_interface_failed,
-                         originalVerStr, newVerStr, interfacePath);
-        }
-        break;
-      }
-      llvm::SmallString<64> buffer(
-          originalOSName.substr(0, OSNameWithoutVersionLength));
-      buffer.append(newVerStr);
-      triple.setOSName(buffer.str());
+    // Update the triple to use if it differs.
+    if (!areTriplesEqual(triple, *canonicalTriple)) {
+      triple = *canonicalTriple;
       shouldModify = true;
     }
     if (shouldModify)

test/ModuleInterface/canonicalized-os-version.swift

@@ -0,0 +1,51 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/module-cache)
+// RUN: split-file %s %t
+
+// First test the swift interface with a invalid os version behaves fine.
+// RUN: %target-swift-typecheck-module-from-interface(%t/Modules/Simple.swiftmodule/arm64-apple-macos.swiftinterface) -module-name Simple 
+
+// Next build transitive dependencies in zippered mode.
+// RUN: %target-swift-frontend -module-name input %t/input.swift -target arm64-apple-macosx50.1 -target-variant arm64-apple-ios50.1-macabi -I%t/Modules -scan-dependencies -module-cache-path %t/module-cache-path -o %t/deps.json 2>&1 | Filecheck  --allow-empty --implicit-check-not warning: --implicit-check-not error: %s
+// RUN: %validate-json %t/deps.json | %FileCheck %s --check-prefix=DEPS
+
+DEPS:       "arm64-apple-macos26.4"
+DEPS-NOT:   "arm64-apple-macos16.4"
+DEPS-NOT:   "arm64-apple-ios22.0-macabi"
+
+//--- Modules/Simple.swiftmodule/arm64-apple-macos.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -target arm64-apple-macos16.4 
+public struct S {
+}
+
+//--- Modules/Simple.swiftmodule/arm64-apple-ios-macabi.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -target arm64-apple-ios22.0-macabi
+public struct S {
+}
+
+//--- Modules/module.modulemap
+module ClangDep {
+  header "ClangDep.h"
+  export *
+}
+
+//--- Modules/ClangDep.h
+typedef int my_int;
+
+
+//--- Modules/Interopt.swiftmodule/arm64-apple-macos.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -target arm64-apple-macos16.4  
+import Simple
+import ClangDep
+
+//--- Modules/Interopt.swiftmodule/arm64-apple-ios-macabi.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -target arm64-apple-ios22.0-macabi  
+import Simple
+import ClangDep
+
+//--- input.swift
+import Interopt