diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h index dc0884c4c660e..df9130b971694 100644 --- a/include/swift/AST/DiagnosticEngine.h +++ b/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 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; } diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 4944736c3962a..144a3376bedfa 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -873,18 +873,54 @@ ERROR(need_hermetic_seal_to_import_module,none, (Identifier)) ERROR(modularization_issue_decl_moved,Fatal, - "reference to %select{top-level|type}0 %1 broken by a context change; " + "reference to %select{top-level declaration|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; " + "reference to %select{top-level declaration|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; " + "reference to %select{top-level declaration|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_note_expected,none, + "the %select{declaration|type}0 was expected to be found in module %1 at '%2'", + (bool, const ModuleDecl*, StringRef)) +NOTE(modularization_issue_note_expected_underlying,none, + "or expected to be found in the underlying module '%0' defined at '%1'", + (StringRef, StringRef)) +NOTE(modularization_issue_note_found,none, + "the %select{declaration|type}0 was actually found in module %1 at '%2'", + (bool, const ModuleDecl*, StringRef)) + +NOTE(modularization_issue_stale_module,none, + "the module %0 has enabled library-evolution; " + "the following file may need to be deleted if the SDK was modified: '%1'", + (const ModuleDecl *, StringRef)) +NOTE(modularization_issue_swift_version,none, + "the module %0 was built with a Swift language version set to %1 " + "while the current invocation uses %2; " + "APINotes may change how clang declarations are imported", + (const ModuleDecl *, llvm::VersionTuple, llvm::VersionTuple)) +NOTE(modularization_issue_audit_headers,none, + "declarations in the %select{underlying|}0 clang module %1 " + "may be hidden by clang preprocessor macros", + (bool, const ModuleDecl*)) +NOTE(modularization_issue_layering_expected_local,none, + "the distributed module %0 refers to the local module %1; " + "this may be caused by header maps or search paths", + (const ModuleDecl*, const ModuleDecl*)) +NOTE(modularization_issue_layering_found_local,none, + "the reference may break layering; the candidate was found in " + "the local module %1 for a reference from the distributed module %0", + (const ModuleDecl*, const ModuleDecl*)) +NOTE(modularization_issue_related_modules,none, + "the %select{declaration|type}0 %1 moved between related modules; " + "clang preprocessor macros may affect headers shared between these modules", + (bool, DeclName)) NOTE(modularization_issue_side_effect_extension_error,none, "could not deserialize extension", @@ -893,9 +929,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", - ()) +NOTE(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" diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index e74bf4846f906..f2d29f6fa993d 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -40,6 +40,7 @@ #include "swift/ClangImporter/SwiftAbstractBasicReader.h" #include "swift/Serialization/SerializedModuleLoader.h" #include "clang/AST/DeclTemplate.h" +#include "clang/Basic/SourceManager.h" #include "llvm/ADT/Statistic.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/Debug.h" @@ -185,10 +186,25 @@ SourceLoc ModuleFile::getSourceLoc() const { auto filename = getModuleFilename(); auto bufferID = SourceMgr.getIDForBufferIdentifier(filename); if (!bufferID) - bufferID = SourceMgr.addMemBufferCopy(StringRef(), filename); + bufferID = SourceMgr.addMemBufferCopy("", filename); return SourceMgr.getLocForBufferStart(*bufferID); } +SourceLoc ModularizationError::getSourceLoc() const { + auto &SourceMgr = referenceModule->getContext().Diags.SourceMgr; + auto filename = referenceModule->getModuleFilename(); + + // Synthesize some context. We don't have an actual decl here + // so try to print a simple representation of the reference. + std::string S; + llvm::raw_string_ostream OS(S); + OS << expectedModule->getName() << "." << name; + + // If we enable these remarks by default we may want to reuse these buffers. + auto bufferID = SourceMgr.addMemBufferCopy(S, filename); + return SourceMgr.getLocForBufferStart(bufferID); +} + void ModularizationError::diagnose(const ModuleFile *MF, DiagnosticBehavior limit) const { @@ -197,18 +213,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."); }; @@ -220,6 +239,103 @@ ModularizationError::diagnose(const ModuleFile *MF, // We could pass along the `path` information through notes. // However, for a top-level decl a path would just duplicate the // expected module name and the decl name from the diagnostic. + + // Show context with relevant file paths. + ctx.Diags.diagnose(SourceLoc(), + diag::modularization_issue_note_expected, + declIsType, expectedModule, + expectedModule->getModuleSourceFilename()); + + const clang::Module *expectedUnderlying = + expectedModule->findUnderlyingClangModule(); + if (!expectedModule->isNonSwiftModule() && + expectedUnderlying) { + auto CML = ctx.getClangModuleLoader(); + auto &CSM = CML->getClangASTContext().getSourceManager(); + StringRef filename = CSM.getFilename(expectedUnderlying->DefinitionLoc); + ctx.Diags.diagnose(SourceLoc(), + diag::modularization_issue_note_expected_underlying, + expectedUnderlying->Name, + filename); + } + + if (foundModule) + ctx.Diags.diagnose(SourceLoc(), + diag::modularization_issue_note_found, + declIsType, foundModule, + foundModule->getModuleSourceFilename()); + + // A Swift language version mismatch could lead to a different set of rules + // from APINotes files being applied when building the module vs when reading + // from it. + version::Version + moduleLangVersion = referenceModule->getCompatibilityVersion(), + clientLangVersion = MF->getContext().LangOpts.EffectiveLanguageVersion; + ModuleDecl *referenceModuleDecl = referenceModule->getAssociatedModule(); + if (clientLangVersion != moduleLangVersion) { + ctx.Diags.diagnose(SourceLoc(), + diag::modularization_issue_swift_version, + referenceModuleDecl, moduleLangVersion, + clientLangVersion); + } + + // If the error is in a resilient swiftmodule adjacent to a swiftinterface, + // deleting the module to rebuild from the swiftinterface may fix the issue. + // Limit this suggestion to distributed Swift modules to not hint at + // deleting local caches and such. + bool referenceModuleIsDistributed = referenceModuleDecl && + referenceModuleDecl->isNonUserModule(); + if (referenceModule->getResilienceStrategy() == + ResilienceStrategy::Resilient && + referenceModuleIsDistributed) { + ctx.Diags.diagnose(SourceLoc(), + diag::modularization_issue_stale_module, + referenceModuleDecl, + referenceModule->getModuleFilename()); + } + + // If the missing decl was expected to be in a clang module, + // it may be hidden by some clang defined passed via `-Xcc` affecting how + // headers are seen. + if (expectedUnderlying) { + ctx.Diags.diagnose(SourceLoc(), + diag::modularization_issue_audit_headers, + expectedModule->isNonSwiftModule(), expectedModule); + } + + // If the reference goes from a distributed module to a local module, + // the compiler may have picked up an undesired module. We usually expect + // distributed modules to only reference other distributed modules. + // Local modules can reference both local modules and distributed modules. + if (referenceModuleIsDistributed) { + if (!expectedModule->isNonUserModule()) { + ctx.Diags.diagnose(SourceLoc(), + diag::modularization_issue_layering_expected_local, + referenceModuleDecl, expectedModule); + } else if (foundModule && !foundModule->isNonUserModule()) { + ctx.Diags.diagnose(SourceLoc(), + diag::modularization_issue_layering_found_local, + referenceModuleDecl, foundModule); + } + } + + // If a type moved between MyModule and MyModule_Private, it can be caused + // by the use of `-Xcc -D` to change the API of the modules, leading to + // decls moving between both modules. + if (errorKind == Kind::DeclMoved || + errorKind == Kind::DeclKindChanged) { + StringRef foundModuleName = foundModule->getName().str(); + StringRef expectedModuleName = expectedModule->getName().str(); + if (foundModuleName != expectedModuleName && + (foundModuleName.startswith(expectedModuleName) || + expectedModuleName.startswith(foundModuleName)) && + (expectedUnderlying || + expectedModule->findUnderlyingClangModule())) { + ctx.Diags.diagnose(SourceLoc(), + diag::modularization_issue_related_modules, + declIsType, name); + } + } } void TypeError::diagnose(const ModuleFile *MF) const { @@ -1957,7 +2073,7 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) { SmallVector strScratch; auto errorKind = ModularizationError::Kind::DeclNotFound; - Identifier foundIn; + ModuleDecl *foundIn = nullptr; bool isType = false; if (recordID == XREF_TYPE_PATH_PIECE || @@ -2011,7 +2127,7 @@ 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 @@ -2019,20 +2135,18 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) { // 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(declName, isType, errorKind, - expectedIn, - referencedFrom, + baseModule, + this, foundIn, pathTrace); @@ -2043,13 +2157,13 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) { 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); + // Print the error as a warning and notify of the recovery attempt. llvm::handleAllErrors(std::move(error), [&](const ModularizationError &modularError) { - modularError.diagnose(this, DiagnosticBehavior::Note); + modularError.diagnose(this, DiagnosticBehavior::Warning); }); + getContext().Diags.diagnose(SourceLoc(), + diag::modularization_issue_worked_around); } else { return std::move(error); } diff --git a/lib/Serialization/DeserializationErrors.h b/lib/Serialization/DeserializationErrors.h index 57234737e3c23..6c4cb95efb131 100644 --- a/lib/Serialization/DeserializationErrors.h +++ b/lib/Serialization/DeserializationErrors.h @@ -355,46 +355,55 @@ class ModularizationError : public llvm::ErrorInfo { 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"; path.print(OS); } + SourceLoc getSourceLoc() const; + std::error_code convertToErrorCode() const override { return llvm::inconvertibleErrorCode(); } diff --git a/test/Serialization/Recovery/module-recovery-remarks.swift b/test/Serialization/Recovery/module-recovery-remarks.swift index d5b05300e3434..e27b183de9baf 100644 --- a/test/Serialization/Recovery/module-recovery-remarks.swift +++ b/test/Serialization/Recovery/module-recovery-remarks.swift @@ -15,6 +15,43 @@ /// Main error downgraded to a remark. // CHECK-MOVED: LibWithXRef.swiftmodule:1:1: remark: reference to type 'MyType' broken by a context change; 'MyType' was expected to be in 'A', but now a candidate is found only in 'A_related' + +/// Contextual notes about the modules involved. +// CHECK-MOVED: :0: note: the type was expected to be found in module 'A' at ' +// CHECK-MOVED-SAME: A.swiftmodule' +// CHECK-MOVED: :0: note: or expected to be found in the underlying module 'A' defined at ' +// CHECK-MOVED-SAME: module.modulemap' +// CHECK-MOVED: :0: note: the type was actually found in module 'A_related' at ' +// CHECK-MOVED-SAME: A_related.swiftmodule' + +/// More notes depending on the context +// CHECK-MOVED: :0: note: the module 'LibWithXRef' was built with a Swift language version set to 5 +// CHECK-MOVED-SAME: while the current invocation uses 4 + +// CHECK-MOVED: :0: note: the module 'LibWithXRef' has enabled library-evolution; the following file may need to be deleted if the SDK was modified: ' +// CHECK-MOVED-SAME: LibWithXRef.swiftmodule' +// CHECK-MOVED: :0: note: declarations in the underlying clang module 'A' may be hidden by clang preprocessor macros +// CHECK-MOVED: :0: note: the distributed module 'LibWithXRef' refers to the local module 'A'; this may be caused by header maps or search paths +// CHECK-MOVED: :0: note: the type 'MyType' moved between related modules; clang preprocessor macros may affect headers shared between these modules +// CHECK-MOVED: LibWithXRef.swiftmodule:1:1: note: could not deserialize type for 'foo()' +// CHECK-MOVED: error: cannot find 'foo' in scope + +/// Move A to the SDK, triggering a different note about layering. +// RUN: mv %t/A.swiftmodule %t/sdk/A.swiftmodule +// RUN: not %target-swift-frontend -c -O %t/Client.swift -I %t -I %t/sdk -Rmodule-recovery -sdk %t/sdk 2>&1 \ +// RUN: | %FileCheck --check-prefixes CHECK-LAYERING-FOUND %s +// CHECK-LAYERING-FOUND: :0: note: the reference may break layering; the candidate was found in the local module 'A_related' for a reference from the distributed module 'LibWithXRef' +// CHECK-LAYERING-FOUND: error: cannot find 'foo' in scope + +/// Delete A, keep only the underlying clangmodule for notes about clang modules. +// RUN: rm %t/sdk/A.swiftmodule +// RUN: %target-swift-frontend %t/Empty.swift -emit-module-path %t/A_related.swiftmodule -module-name A_related +// RUN: not %target-swift-frontend -c -O %t/Client.swift -I %t -I %t/sdk -Rmodule-recovery -sdk %t/sdk 2>&1 \ +// RUN: | %FileCheck --check-prefixes CHECK-CLANG %s +// CHECK-CLANG: :0: note: declarations in the clang module 'A' may be hidden by clang preprocessor macros +// CHECK-CLANG: error: cannot find 'foo' in scope + + //--- module.modulemap module A { header "A.h" diff --git a/test/Serialization/modularization-error.swift b/test/Serialization/modularization-error.swift index 478b654a79469..4f58b44788c68 100644 --- a/test/Serialization/modularization-error.swift +++ b/test/Serialization/modularization-error.swift @@ -18,8 +18,14 @@ // 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: LibWithXRef.swiftmodule:1:1: warning: 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-NEXT: A.MyType +// CHECK-WORKAROUND-NEXT: ^ +// CHECK-WORKAROUND: :0: note: the type was expected to be found in module 'A' at ' +// CHECK-WORKAROUND-SAME: A.swiftmodule' +// CHECK-WORKAROUND: :0: note: the type was actually found in module 'B' at ' +// CHECK-WORKAROUND-SAME: B.swiftmodule' +// CHECK-WORKAROUND: :0: note: attempting forced recovery enabled by -experimental-force-workaround-broken-modules // CHECK-WORKAROUND: func foo() -> some Proto /// Change MyType into a function. @@ -34,7 +40,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