[Serialization] ModularizationError keeps full objects

Preserve more information about the context of modularization errors by
replacing the module names with pointers to ModuleDecl and ModuleFile
objects.

Author: xymus

include/swift/AST/DiagnosticEngine.h

@@ -143,7 +143,7 @@ namespace swift {
       StringRef StringVal;
       DeclNameRef IdentifierVal;
       ObjCSelector ObjCSelectorVal;
-      ValueDecl *TheValueDecl;
+      const ValueDecl *TheValueDecl;
       Type TypeVal;
       TypeRepr *TyR;
       FullyQualified<Type> FullyQualifiedTypeVal;
@@ -194,7 +194,7 @@ namespace swift {
       : Kind(DiagnosticArgumentKind::ObjCSelector), ObjCSelectorVal(S) {
     }
 
-    DiagnosticArgument(ValueDecl *VD)
+    DiagnosticArgument(const ValueDecl *VD)
       : Kind(DiagnosticArgumentKind::ValueDecl), TheValueDecl(VD) {
     }
 
@@ -305,7 +305,7 @@ namespace swift {
       return ObjCSelectorVal;
     }
 
-    ValueDecl *getAsValueDecl() const {
+    const ValueDecl *getAsValueDecl() const {
       assert(Kind == DiagnosticArgumentKind::ValueDecl);
       return TheValueDecl;
     }

include/swift/AST/DiagnosticsSema.def

@@ -875,16 +875,16 @@ ERROR(need_hermetic_seal_to_import_module,none,
 ERROR(modularization_issue_decl_moved,Fatal,
       "reference to %select{top-level|type}0 %1 broken by a context change; "
       "%1 was expected to be in %2, but now a candidate is found only in %3",
-      (bool, DeclName, Identifier, Identifier))
+      (bool, DeclName, const ModuleDecl*, const ModuleDecl*))
 ERROR(modularization_issue_decl_type_changed,Fatal,
       "reference to %select{top-level|type}0 %1 broken by a context change; "
       "the declaration kind of %1 %select{from %2|}5 changed since building '%3'"
-      "%select{|, it was in %2 and is now found in %4}5",
-      (bool, DeclName, Identifier, StringRef, Identifier, bool))
+      "%select{|, it was in %2 and is now a candidate is found only in %4}5",
+      (bool, DeclName, const ModuleDecl*, StringRef, const ModuleDecl*, bool))
 ERROR(modularization_issue_decl_not_found,Fatal,
       "reference to %select{top-level|type}0 %1 broken by a context change; "
       "%1 is not found, it was expected to be in %2",
-      (bool, DeclName, Identifier))
+      (bool, DeclName, const ModuleDecl*))
 
 NOTE(modularization_issue_side_effect_extension_error,none,
      "could not deserialize extension",

lib/Serialization/Deserialization.cpp

@@ -197,18 +197,21 @@ ModularizationError::diagnose(const ModuleFile *MF,
   auto diagnoseError = [&](Kind errorKind) {
     switch (errorKind) {
     case Kind::DeclMoved:
-      return ctx.Diags.diagnose(MF->getSourceLoc(), diag::modularization_issue_decl_moved,
-                                declIsType, name, expectedModuleName,
-                                foundModuleName);
+      return ctx.Diags.diagnose(getSourceLoc(),
+                                diag::modularization_issue_decl_moved,
+                                declIsType, name, expectedModule,
+                                foundModule);
     case Kind::DeclKindChanged:
       return
-        ctx.Diags.diagnose(MF->getSourceLoc(), diag::modularization_issue_decl_type_changed,
-                           declIsType, name, expectedModuleName,
-                           referencedFromModuleName, foundModuleName,
-                           foundModuleName != expectedModuleName);
+        ctx.Diags.diagnose(getSourceLoc(),
+                           diag::modularization_issue_decl_type_changed,
+                           declIsType, name, expectedModule,
+                           referenceModule->getName(), foundModule,
+                           foundModule != expectedModule);
     case Kind::DeclNotFound:
-      return ctx.Diags.diagnose(MF->getSourceLoc(), diag::modularization_issue_decl_not_found,
-                                declIsType, name, expectedModuleName);
+      return ctx.Diags.diagnose(getSourceLoc(),
+                                diag::modularization_issue_decl_not_found,
+                                declIsType, name, expectedModule);
     }
     llvm_unreachable("Unhandled ModularizationError::Kind in switch.");
   };
@@ -1957,7 +1960,7 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
     SmallVector<char, 64> strScratch;
 
     auto errorKind = ModularizationError::Kind::DeclNotFound;
-    Identifier foundIn;
+    ModuleDecl *foundIn = nullptr;
     bool isType = false;
 
     if (recordID == XREF_TYPE_PATH_PIECE ||
@@ -2011,28 +2014,26 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
           // one because otherwise it would have succeeded on the first search.
           // This is usually caused by the use of poorly modularized headers.
           errorKind = ModularizationError::Kind::DeclMoved;
-          foundIn = otherModule->getName();
+          foundIn = otherModule;
           break;
         } else if (hadAMatchBeforeFiltering) {
           // Found a match that was filtered out. This may be from the same
           // expected module if there's a type difference. This can be caused
           // by the use of different Swift language versions between a library
           // with serialized SIL and a client.
           errorKind = ModularizationError::Kind::DeclKindChanged;
-          foundIn = otherModule->getName();
+          foundIn = otherModule;
           break;
         }
       }
     }
 
     auto declName = getXRefDeclNameForError();
-    auto expectedIn = baseModule->getName();
-    auto referencedFrom = getName();
     auto error = llvm::make_error<ModularizationError>(declName,
                                                        isType,
                                                        errorKind,
-                                                       expectedIn,
-                                                       referencedFrom,
+                                                       baseModule,
+                                                       this,
                                                        foundIn,
                                                        pathTrace);
 

lib/Serialization/DeserializationErrors.h

@@ -355,40 +355,47 @@ class ModularizationError : public llvm::ErrorInfo<ModularizationError> {
   DeclName name;
   bool declIsType;
   Kind errorKind;
-  Identifier expectedModuleName;
-  StringRef referencedFromModuleName;
-  Identifier foundModuleName;
+
+  /// Module targeted by the reference that should define the decl.
+  const ModuleDecl *expectedModule;
+
+  /// Binary module file with the outgoing reference.
+  const ModuleFile *referenceModule;
+
+  /// Module where the compiler did find the decl, if any.
+  const ModuleDecl *foundModule;
+
   XRefTracePath path;
 
 public:
   explicit ModularizationError(DeclName name, bool declIsType, Kind errorKind,
-                               Identifier expectedModuleName,
-                               StringRef referencedFromModuleName,
-                               Identifier foundModuleName,
+                               const ModuleDecl *expectedModule,
+                               const ModuleFile *referenceModule,
+                               const ModuleDecl *foundModule,
                                XRefTracePath path):
     name(name), declIsType(declIsType), errorKind(errorKind),
-    expectedModuleName(expectedModuleName),
-    referencedFromModuleName(referencedFromModuleName),
-    foundModuleName(foundModuleName), path(path) {}
+    expectedModule(expectedModule),
+    referenceModule(referenceModule),
+    foundModule(foundModule), path(path) {}
 
   void diagnose(const ModuleFile *MF,
                 DiagnosticBehavior limit = DiagnosticBehavior::Fatal) const;
 
   void log(raw_ostream &OS) const override {
     OS << "modularization issue on '" << name << "', reference from '";
-    OS << referencedFromModuleName << "' not resolvable: ";
+    OS << referenceModule->getName() << "' not resolvable: ";
     switch (errorKind) {
       case Kind::DeclMoved:
-        OS << "expected in '" << expectedModuleName << "' but found in '";
-        OS << foundModuleName << "'";
+        OS << "expected in '" << expectedModule->getName() << "' but found in '";
+        OS << foundModule->getName() << "'";
         break;
       case Kind::DeclKindChanged:
         OS << "decl details changed between what was imported from '";
-        OS << expectedModuleName << "' and what is now imported from '";
-        OS << foundModuleName << "'";
+        OS << expectedModule->getName() << "' and what is now imported from '";
+        OS << foundModule->getName() << "'";
         break;
       case Kind::DeclNotFound:
-        OS << "not found, expected in '" << expectedModuleName << "'";
+        OS << "not found, expected in '" << expectedModule->getName() << "'";
         break;
     }
     OS << "\n";

test/Serialization/modularization-error.swift

@@ -34,7 +34,7 @@
 // RUN: %target-swift-frontend %t/LibTypeChanged.swift -emit-module-path %t/B.swiftmodule -module-name B
 // RUN: not %target-swift-frontend -emit-sil %t/LibWithXRef.swiftmodule -module-name LibWithXRef -I %t 2>&1 \
 // RUN:   | %FileCheck --check-prefixes CHECK,CHECK-KIND-CHANGED-AND-MOVED %s
-// CHECK-KIND-CHANGED-AND-MOVED: LibWithXRef.swiftmodule:1:1: error: reference to type 'MyType' broken by a context change; the declaration kind of 'MyType' changed since building 'LibWithXRef', it was in 'A' and is now found in 'B'
+// CHECK-KIND-CHANGED-AND-MOVED: LibWithXRef.swiftmodule:1:1: error: reference to type 'MyType' broken by a context change; the declaration kind of 'MyType' changed since building 'LibWithXRef', it was in 'A' and is now a candidate is found only in 'B'
 
 /// Remove MyType from all imported modules.
 // RUN: %target-swift-frontend %t/Empty.swift -emit-module-path %t/A.swiftmodule -module-name A