Merge pull request #66186 from xymus/force-workaround

[Serialization] Add flag to force unsafe recovery from some xref failures

Author: xymus

include/swift/AST/DiagnosticsSema.def

@@ -893,6 +893,10 @@ NOTE(modularization_issue_side_effect_type_error,none,
      "could not deserialize type for %0",
      (DeclName))
 
+WARNING(modularization_issue_worked_around,none,
+        "attempting forced recovery enabled by -experimental-force-workaround-broken-modules",
+        ())
+
 ERROR(reserved_member_name,none,
       "type member must not be named %0, since it would conflict with the"
       " 'foo.%1' expression", (DeclName, StringRef))

include/swift/Basic/LangOptions.h

@@ -399,6 +399,11 @@ namespace swift {
     bool EnableDeserializationSafety =
       ::getenv("SWIFT_ENABLE_DESERIALIZATION_SAFETY");
 
+    /// Attempt to recover for imported modules with broken modularization
+    /// in an unsafe way. Currently applies only to xrefs where the target
+    /// decl moved to a different module that is already loaded.
+    bool ForceWorkaroundBrokenModules = false;
+
     /// Whether to enable the new operator decl and precedencegroup lookup
     /// behavior. This is a staging flag, and will be removed in the future.
     bool EnableNewOperatorLookup = false;

include/swift/Option/FrontendOptions.td

@@ -588,6 +588,10 @@ def disable_deserialization_safety :
   Flag<["-"], "disable-deserialization-safety">,
   HelpText<"Don't avoid reading potentially unsafe decls in swiftmodules">;
 
+def force_workaround_broken_modules :
+  Flag<["-"], "experimental-force-workaround-broken-modules">,
+  HelpText<"Attempt unsafe recovery for imported modules with broken modularization">;
+
 def enable_experimental_string_processing :
   Flag<["-"], "enable-experimental-string-processing">,
   HelpText<"Enable experimental string processing">;

lib/Frontend/CompilerInvocation.cpp

@@ -600,6 +600,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
     Opts.EnableAccessControl
       = A->getOption().matches(OPT_enable_access_control);
   }
+  Opts.ForceWorkaroundBrokenModules
+    |= Args.hasArg(OPT_force_workaround_broken_modules);
 
   // Whether '/.../' regex literals are enabled. This implies experimental
   // string processing.

lib/Serialization/Deserialization.cpp

@@ -1885,8 +1885,10 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
   }
 
   auto getXRefDeclNameForError = [&]() -> DeclName {
+    BCOffsetRAII restoreOffset(DeclTypeCursor);
     DeclName result = pathTrace.getLastName();
-    while (--pathLen) {
+    uint32_t namePathLen = pathLen;
+    while (--namePathLen) {
       llvm::BitstreamEntry entry =
           fatalIfUnexpected(DeclTypeCursor.advance(AF_DontPopBlockAtEnd));
       if (entry.Kind != llvm::BitstreamEntry::Record)
@@ -1951,8 +1953,7 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
     // up to the caller to recover if possible.
 
     // Look for types and value decls in other modules. This extra information
-    // is mostly for compiler engineers to understand a likely solution at a
-    // quick glance.
+    // will be used for diagnostics by the caller logic.
     SmallVector<char, 64> strScratch;
 
     auto errorKind = ModularizationError::Kind::DeclNotFound;
@@ -2027,13 +2028,31 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
     auto declName = getXRefDeclNameForError();
     auto expectedIn = baseModule->getName();
     auto referencedFrom = getName();
-    return llvm::make_error<ModularizationError>(declName,
-                                                 isType,
-                                                 errorKind,
-                                                 expectedIn,
-                                                 referencedFrom,
-                                                 foundIn,
-                                                 pathTrace);
+    auto error = llvm::make_error<ModularizationError>(declName,
+                                                       isType,
+                                                       errorKind,
+                                                       expectedIn,
+                                                       referencedFrom,
+                                                       foundIn,
+                                                       pathTrace);
+
+    // If we want to workaround broken modularization, we can keep going if
+    // we found a matching top-level decl in a different module. This is
+    // obviously dangerous as it could just be some other decl that happens to
+    // match.
+    if (getContext().LangOpts.ForceWorkaroundBrokenModules &&
+        errorKind == ModularizationError::Kind::DeclMoved &&
+        !values.empty()) {
+      // Print the error as a remark and notify of the recovery attempt.
+      getContext().Diags.diagnose(getSourceLoc(),
+                                  diag::modularization_issue_worked_around);
+      llvm::handleAllErrors(std::move(error),
+        [&](const ModularizationError &modularError) {
+          modularError.diagnose(this, DiagnosticBehavior::Note);
+        });
+    } else {
+      return std::move(error);
+    }
   }
 
   // Filters for values discovered in the remaining path pieces.

test/Serialization/modularization-error.swift

@@ -14,6 +14,14 @@
 // RUN:   | %FileCheck --check-prefixes CHECK,CHECK-MOVED %s
 // CHECK-MOVED: LibWithXRef.swiftmodule:1:1: error: reference to type 'MyType' broken by a context change; 'MyType' was expected to be in 'A', but now a candidate is found only in 'B'
 
+/// Force working around the broken modularization to get a result and no errors.
+// RUN: %target-swift-frontend -emit-sil %t/LibWithXRef.swiftmodule -module-name LibWithXRef -I %t \
+// RUN:   -experimental-force-workaround-broken-modules 2>&1 \
+// RUN:   | %FileCheck --check-prefixes CHECK-WORKAROUND %s
+// CHECK-WORKAROUND: warning: attempting forced recovery enabled by -experimental-force-workaround-broken-modules
+// CHECK-WORKAROUND: note: reference to type 'MyType' broken by a context change; 'MyType' was expected to be in 'A', but now a candidate is found only in 'B'
+// CHECK-WORKAROUND: func foo() -> some Proto
+
 /// Change MyType into a function.
 // RUN: %target-swift-frontend %t/LibTypeChanged.swift -emit-module-path %t/A.swiftmodule -module-name A
 // RUN: %target-swift-frontend %t/Empty.swift -emit-module-path %t/B.swiftmodule -module-name B