From 2f03c952c340d386a76d1fab19f96102d00da25a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferri=C3=A8re?= Date: Wed, 17 May 2023 18:18:25 -0700 Subject: [PATCH 1/5] [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. --- include/swift/AST/DiagnosticEngine.h | 6 +-- include/swift/AST/DiagnosticsSema.def | 8 ++-- lib/Serialization/Deserialization.cpp | 33 +++++++++-------- lib/Serialization/DeserializationErrors.h | 37 +++++++++++-------- test/Serialization/modularization-error.swift | 2 +- 5 files changed, 47 insertions(+), 39 deletions(-) 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..10cb9e2f71e18 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/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", diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index e74bf4846f906..29359a7f00523 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/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 strScratch; auto errorKind = ModularizationError::Kind::DeclNotFound; - Identifier foundIn; + ModuleDecl *foundIn = nullptr; bool isType = false; if (recordID == XREF_TYPE_PATH_PIECE || @@ -2011,7 +2014,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 +2022,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); diff --git a/lib/Serialization/DeserializationErrors.h b/lib/Serialization/DeserializationErrors.h index 57234737e3c23..ba1bec221f551 100644 --- a/lib/Serialization/DeserializationErrors.h +++ b/lib/Serialization/DeserializationErrors.h @@ -355,40 +355,47 @@ 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"; diff --git a/test/Serialization/modularization-error.swift b/test/Serialization/modularization-error.swift index 478b654a79469..a97eec1ea67a5 100644 --- a/test/Serialization/modularization-error.swift +++ b/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 From 1cd4f4ac67d0eb5f9a6efd9e1eceac48d872ebf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferri=C3=A8re?= Date: Mon, 22 May 2023 09:30:20 -0700 Subject: [PATCH 2/5] [Serialization] Intro ModularizationError::getSourceLoc This service provides a path to the loaded swiftmodule file and synthesizes pseudo-Swift code representing the reference. --- lib/Serialization/Deserialization.cpp | 15 +++++++++++++++ lib/Serialization/DeserializationErrors.h | 2 ++ 2 files changed, 17 insertions(+) diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 29359a7f00523..5e1b4164f3a79 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -189,6 +189,21 @@ SourceLoc ModuleFile::getSourceLoc() const { 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 { diff --git a/lib/Serialization/DeserializationErrors.h b/lib/Serialization/DeserializationErrors.h index ba1bec221f551..6c4cb95efb131 100644 --- a/lib/Serialization/DeserializationErrors.h +++ b/lib/Serialization/DeserializationErrors.h @@ -402,6 +402,8 @@ class ModularizationError : public llvm::ErrorInfo { path.print(OS); } + SourceLoc getSourceLoc() const; + std::error_code convertToErrorCode() const override { return llvm::inconvertibleErrorCode(); } From b1e0b89cf772440ef3a86017f10892fc32aae890 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferri=C3=A8re?= Date: Tue, 30 May 2023 15:28:54 -0700 Subject: [PATCH 3/5] [Serialization] Invert order of diagnostics in forced recovery mode --- include/swift/AST/DiagnosticsSema.def | 7 ++++--- lib/Serialization/Deserialization.cpp | 8 ++++---- test/Serialization/modularization-error.swift | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 10cb9e2f71e18..8d4aa7aa284e0 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -893,9 +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", - ()) +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 5e1b4164f3a79..c4cdc1499db9a 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -2059,13 +2059,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(getSourceLoc(), + diag::modularization_issue_worked_around); } else { return std::move(error); } diff --git a/test/Serialization/modularization-error.swift b/test/Serialization/modularization-error.swift index a97eec1ea67a5..0fb8d4d1badd8 100644 --- a/test/Serialization/modularization-error.swift +++ b/test/Serialization/modularization-error.swift @@ -18,8 +18,8 @@ // 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: LibWithXRef.swiftmodule:1:1: note: attempting forced recovery enabled by -experimental-force-workaround-broken-modules // CHECK-WORKAROUND: func foo() -> some Proto /// Change MyType into a function. From 98040755498376f358bde252ae5b3706002ec09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferri=C3=A8re?= Date: Thu, 25 May 2023 10:43:59 -0700 Subject: [PATCH 4/5] [Serialization] Show contextual notes on deserialization errors This PR makes diagnostics on deserialization errors caused by project configuration more helpful by providing contextual information on the issue: - Path to the modules involved (up to 4 modules): the loaded swiftmodule with the broken outgoing reference, the path to the module where the decl was expected, the path to the underlying clang module, and the path to the module where the decl was found. This information should prevent us from having to ask for a different log with `-Rmodule-loading`. - Hint to delete the swiftmodule files when the module is library-evolution enabled. - Hint that clang settings affect clang modules involved in this scenario. - Pointing out when a decl moved between two modules with a similar name (where one name is a prefix of the other). This is a common issue when headers are shared between a clang framework's public and private modules. - Pointing out layering issues when an SDK module imports a local module. - Pointing out Swift language version mismatch which may lead to the compiler viewing the same clang decls differently when they are modified by APINotes files. --- include/swift/AST/DiagnosticsSema.def | 42 +++++++- lib/Serialization/Deserialization.cpp | 102 +++++++++++++++++- .../Recovery/module-recovery-remarks.swift | 37 +++++++ test/Serialization/modularization-error.swift | 8 +- 4 files changed, 183 insertions(+), 6 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 8d4aa7aa284e0..1a9cbfca176c6 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -873,19 +873,55 @@ 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, 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 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, 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 project settings", + (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 clang project settings", + (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 project settings may affect headers shared between these modules", + (bool, DeclName)) + NOTE(modularization_issue_side_effect_extension_error,none, "could not deserialize extension", ()) diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index c4cdc1499db9a..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,7 +186,7 @@ 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); } @@ -238,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 { @@ -2064,7 +2162,7 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) { [&](const ModularizationError &modularError) { modularError.diagnose(this, DiagnosticBehavior::Warning); }); - getContext().Diags.diagnose(getSourceLoc(), + getContext().Diags.diagnose(SourceLoc(), diag::modularization_issue_worked_around); } else { return std::move(error); diff --git a/test/Serialization/Recovery/module-recovery-remarks.swift b/test/Serialization/Recovery/module-recovery-remarks.swift index d5b05300e3434..8ee25d240abdc 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 project settings +// CHECK-MOVED: :0: note: the distributed module 'LibWithXRef' refers to the local module 'A'; this may be caused by clang project settings +// CHECK-MOVED: :0: note: the type 'MyType' moved between related modules; clang project settings 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 project settings +// 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 0fb8d4d1badd8..4f58b44788c68 100644 --- a/test/Serialization/modularization-error.swift +++ b/test/Serialization/modularization-error.swift @@ -19,7 +19,13 @@ // RUN: -experimental-force-workaround-broken-modules 2>&1 \ // RUN: | %FileCheck --check-prefixes CHECK-WORKAROUND %s // 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: LibWithXRef.swiftmodule:1:1: note: attempting forced recovery enabled by -experimental-force-workaround-broken-modules +// 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. From af88a6588ae62c6ba551f5c6a090b40ce23d1a64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferri=C3=A8re?= Date: Thu, 8 Jun 2023 12:35:01 -0700 Subject: [PATCH 5/5] [Serialization] Be more specific irt which clang settings may cause a failure --- include/swift/AST/DiagnosticsSema.def | 6 +++--- test/Serialization/Recovery/module-recovery-remarks.swift | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 1a9cbfca176c6..144a3376bedfa 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -907,11 +907,11 @@ NOTE(modularization_issue_swift_version,none, (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 project settings", + "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 clang project settings", + "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 " @@ -919,7 +919,7 @@ NOTE(modularization_issue_layering_found_local,none, (const ModuleDecl*, const ModuleDecl*)) NOTE(modularization_issue_related_modules,none, "the %select{declaration|type}0 %1 moved between related modules; " - "clang project settings may affect headers shared between these modules", + "clang preprocessor macros may affect headers shared between these modules", (bool, DeclName)) NOTE(modularization_issue_side_effect_extension_error,none, diff --git a/test/Serialization/Recovery/module-recovery-remarks.swift b/test/Serialization/Recovery/module-recovery-remarks.swift index 8ee25d240abdc..e27b183de9baf 100644 --- a/test/Serialization/Recovery/module-recovery-remarks.swift +++ b/test/Serialization/Recovery/module-recovery-remarks.swift @@ -30,9 +30,9 @@ // 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 project settings -// CHECK-MOVED: :0: note: the distributed module 'LibWithXRef' refers to the local module 'A'; this may be caused by clang project settings -// CHECK-MOVED: :0: note: the type 'MyType' moved between related modules; clang project settings may affect headers shared between these modules +// 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 @@ -48,7 +48,7 @@ // 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 project settings +// 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