diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 7ae14d40b7518..b4a28a5167fe1 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -2427,12 +2427,17 @@ NOTE(construct_raw_representable_from_unwrapped_value,none, "construct %0 from unwrapped %1 value", (Type, Type)) ERROR(decl_from_implementation_only_module,none, - "cannot use %0 %1 here; %2 has been imported as implementation-only", - (DescriptiveDeclKind, DeclName, Identifier)) + "cannot use %0 %1 %select{here|" + "in an extension with public or '@usableFromInline' members|" + "in an extension with conditional conformances}2; %3 has been imported " + "as implementation-only", + (DescriptiveDeclKind, DeclName, unsigned, Identifier)) ERROR(conformance_from_implementation_only_module,none, - "cannot use conformance of %0 to %1 here; %2 has been imported as " - "implementation-only", - (Type, DeclName, Identifier)) + "cannot use conformance of %0 to %1 %select{here|" + "in an extension with public or '@usableFromInline' members|" + "in an extension with conditional conformances}2; %3 has been imported " + "as implementation-only", + (Type, DeclName, unsigned, Identifier)) ERROR(assoc_conformance_from_implementation_only_module,none, "cannot use conformance of %0 to %1 in associated type %3 (inferred as " "%4); %2 has been imported as implementation-only", diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp index 2c7a97e2d2e00..5a8bd6961579b 100644 --- a/lib/Sema/ResilienceDiagnostics.cpp +++ b/lib/Sema/ResilienceDiagnostics.cpp @@ -249,7 +249,7 @@ diagnoseGenericArgumentsExportability(SourceLoc loc, ASTContext &ctx = M->getASTContext(); ctx.Diags.diagnose(loc, diag::conformance_from_implementation_only_module, rootConf->getType(), - rootConf->getProtocol()->getFullName(), M->getName()); + rootConf->getProtocol()->getFullName(), 0, M->getName()); hadAnyIssues = true; } return hadAnyIssues; diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index 3cafa4eb026c6..f956781c7c40d 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -568,7 +568,7 @@ class AccessControlChecker : public AccessControlCheckerBase, highlightOffendingType(TC, diag, complainRepr); }); } - + void visitOpaqueTypeDecl(OpaqueTypeDecl *OTD) { // TODO(opaque): The constraint class/protocols on the opaque interface, as // well as the naming decl for the opaque type, need to be accessible. @@ -1559,18 +1559,30 @@ class ExportabilityChecker : public DeclVisitor { }); } + // This enum must be kept in sync with + // diag::decl_from_implementation_only_module and + // diag::conformance_from_implementation_only_module. + enum class Reason : unsigned { + General, + ExtensionWithPublicMembers, + ExtensionWithConditionalConformances + }; + class DiagnoseGenerically { TypeChecker &TC; const Decl *D; + Reason reason; public: - DiagnoseGenerically(TypeChecker &TC, const Decl *D) : TC(TC), D(D) {} + DiagnoseGenerically(TypeChecker &TC, const Decl *D, Reason reason) + : TC(TC), D(D), reason(reason) {} void operator()(const TypeDecl *offendingType, const TypeRepr *complainRepr) { ModuleDecl *M = offendingType->getModuleContext(); auto diag = TC.diagnose(D, diag::decl_from_implementation_only_module, offendingType->getDescriptiveKind(), - offendingType->getFullName(), M->getName()); + offendingType->getFullName(), + static_cast(reason), M->getName()); highlightOffendingType(TC, diag, complainRepr); } @@ -1579,7 +1591,7 @@ class ExportabilityChecker : public DeclVisitor { TC.diagnose(D, diag::conformance_from_implementation_only_module, offendingConformance->getType(), offendingConformance->getProtocol()->getFullName(), - M->getName()); + static_cast(reason), M->getName()); } }; @@ -1592,14 +1604,20 @@ class ExportabilityChecker : public DeclVisitor { CheckExportabilityConformanceCallback>::value, "DiagnoseGenerically has wrong call signature for conformance diags"); - DiagnoseGenerically getDiagnoseCallback(const Decl *D) { - return DiagnoseGenerically(TC, D); + DiagnoseGenerically getDiagnoseCallback(const Decl *D, + Reason reason = Reason::General) { + return DiagnoseGenerically(TC, D, reason); } public: explicit ExportabilityChecker(TypeChecker &TC) : TC(TC) {} static bool shouldSkipChecking(const ValueDecl *VD) { + // Accessors are handled as part of their Var or Subscript, and we don't + // want to redo extension signature checking for them. + if (isa(VD)) + return true; + // Is this part of the module's API or ABI? AccessScope accessScope = VD->getFormalAccessScope(nullptr, @@ -1633,12 +1651,6 @@ class ExportabilityChecker : public DeclVisitor { return; DeclVisitor::visit(D); - - if (auto *extension = dyn_cast(D->getDeclContext())) { - checkType(extension->getExtendedTypeLoc(), extension, - getDiagnoseCallback(extension), getDiagnoseCallback(extension)); - checkConstrainedExtensionRequirements(extension); - } } // Force all kinds to be handled at a lower level. @@ -1678,12 +1690,12 @@ class ExportabilityChecker : public DeclVisitor { void checkNamedPattern(const NamedPattern *NP, const llvm::DenseSet &seenVars) { const VarDecl *theVar = NP->getDecl(); - if (shouldSkipChecking(theVar)) - return; // Only check individual variables if we didn't check an enclosing // TypedPattern. if (seenVars.count(theVar) || theVar->isInvalid()) return; + if (shouldSkipChecking(theVar)) + return; checkType(theVar->getInterfaceType(), /*typeRepr*/nullptr, theVar, getDiagnoseCallback(theVar), getDiagnoseCallback(theVar)); @@ -1809,12 +1821,13 @@ class ExportabilityChecker : public DeclVisitor { getDiagnoseCallback(EED)); } - void checkConstrainedExtensionRequirements(ExtensionDecl *ED) { + void checkConstrainedExtensionRequirements(ExtensionDecl *ED, + Reason reason) { if (!ED->getTrailingWhereClause()) return; forAllRequirementTypes(ED, [&](Type type, TypeRepr *typeRepr) { - checkType(type, typeRepr, ED, getDiagnoseCallback(ED), - getDiagnoseCallback(ED)); + checkType(type, typeRepr, ED, getDiagnoseCallback(ED, reason), + getDiagnoseCallback(ED, reason)); }); } @@ -1830,8 +1843,26 @@ class ExportabilityChecker : public DeclVisitor { getDiagnoseCallback(ED)); }); - if (!ED->getInherited().empty()) - checkConstrainedExtensionRequirements(ED); + bool hasPublicMembers = llvm::any_of(ED->getMembers(), + [](const Decl *member) -> bool { + auto *valueMember = dyn_cast(member); + if (!valueMember) + return false; + return !shouldSkipChecking(valueMember); + }); + + if (hasPublicMembers) { + checkType(ED->getExtendedTypeLoc(), ED, + getDiagnoseCallback(ED, Reason::ExtensionWithPublicMembers), + getDiagnoseCallback(ED, Reason::ExtensionWithPublicMembers)); + } + + if (hasPublicMembers || !ED->getInherited().empty()) { + Reason reason = + hasPublicMembers ? Reason::ExtensionWithPublicMembers + : Reason::ExtensionWithConditionalConformances; + checkConstrainedExtensionRequirements(ED, reason); + } } void checkPrecedenceGroup(const PrecedenceGroupDecl *PGD, @@ -1844,6 +1875,7 @@ class ExportabilityChecker : public DeclVisitor { auto diag = TC.diagnose(diagLoc, diag::decl_from_implementation_only_module, PGD->getDescriptiveKind(), PGD->getName(), + static_cast(Reason::General), M->getName()); if (refRange.isValid()) diag.highlight(refRange); diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index b2fa29bbe22aa..3359874bfa65e 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -3684,7 +3684,7 @@ void ConformanceChecker::ensureRequirementsAreSatisfied( conformanceBeingChecked->getLoc(), diag::conformance_from_implementation_only_module, rootConformance->getType(), - rootConformance->getProtocol()->getName(), M->getName()); + rootConformance->getProtocol()->getName(), 0, M->getName()); } else { ctx.Diags.diagnose( conformanceBeingChecked->getLoc(), diff --git a/test/Sema/implementation-only-import-in-decls.swift b/test/Sema/implementation-only-import-in-decls.swift index 958ce361bd199..e07ed04d68ee6 100644 --- a/test/Sema/implementation-only-import-in-decls.swift +++ b/test/Sema/implementation-only-import-in-decls.swift @@ -86,19 +86,27 @@ public var (testBadTypeTuplePartlyInferred3, testBadTypeTuplePartlyInferred4): ( public var testMultipleBindings1: Int? = nil, testMultipleBindings2: BadStruct? = nil // expected-error {{cannot use struct 'BadStruct' here; 'BADLibrary' has been imported as implementation-only}} public var testMultipleBindings3: BadStruct? = nil, testMultipleBindings4: Int? = nil // expected-error {{cannot use struct 'BadStruct' here; 'BADLibrary' has been imported as implementation-only}} -extension BadStruct { // expected-error {{cannot use struct 'BadStruct' here; 'BADLibrary' has been imported as implementation-only}} - public func testExtensionOfBadType() {} // FIXME: Should complain here instead of at the extension decl. +extension BadStruct { // expected-error {{cannot use struct 'BadStruct' in an extension with public or '@usableFromInline' members; 'BADLibrary' has been imported as implementation-only}} + public func testExtensionOfBadType() {} + public var testExtensionVarBad: Int { 0 } + public subscript(bad _: Int) -> Int { 0 } } extension BadStruct { func testExtensionOfOkayType() {} + var testExtensionVarOkay: Int { 0 } + subscript(okay _: Int) -> Int { 0 } } -extension Array where Element == BadStruct { // expected-error {{cannot use struct 'BadStruct' here; 'BADLibrary' has been imported as implementation-only}} - public func testExtensionWithBadRequirement() {} // FIXME: Should complain here instead of at the extension decl. +extension Array where Element == BadStruct { // expected-error {{cannot use struct 'BadStruct' in an extension with public or '@usableFromInline' members; 'BADLibrary' has been imported as implementation-only}} + public func testExtensionWithBadRequirement() {} + public var testExtensionVarBad: Int { 0 } + public subscript(bad _: Int) -> Int { 0 } } extension Array where Element == BadStruct { func testExtensionWithOkayRequirement() {} // okay + var testExtensionVarOkay: Int { 0 } // okay + subscript(okay _: Int) -> Int { 0 } // okay } extension Int: BadProto {} // expected-error {{cannot use protocol 'BadProto' here; 'BADLibrary' has been imported as implementation-only}} @@ -106,7 +114,7 @@ struct TestExtensionConformanceOkay {} extension TestExtensionConformanceOkay: BadProto {} // okay public protocol TestConstrainedExtensionProto {} -extension Array: TestConstrainedExtensionProto where Element == BadStruct { // expected-error {{cannot use struct 'BadStruct' here; 'BADLibrary' has been imported as implementation-only}} +extension Array: TestConstrainedExtensionProto where Element == BadStruct { // expected-error {{cannot use struct 'BadStruct' in an extension with conditional conformances; 'BADLibrary' has been imported as implementation-only}} } @@ -165,7 +173,7 @@ public class SubclassOfNormalClass: NormalClass {} public func testInheritedConformance(_: NormalProtoAssocHolder) {} // expected-error {{cannot use conformance of 'NormalClass' to 'NormalProto' here; 'BADLibrary' has been imported as implementation-only}} public func testSpecializedConformance(_: NormalProtoAssocHolder>) {} // expected-error {{cannot use conformance of 'GenericStruct' to 'NormalProto' here; 'BADLibrary' has been imported as implementation-only}} -extension Array where Element == NormalProtoAssocHolder { // expected-error {{cannot use conformance of 'NormalStruct' to 'NormalProto' here; 'BADLibrary' has been imported as implementation-only}} +extension Array where Element == NormalProtoAssocHolder { // expected-error {{cannot use conformance of 'NormalStruct' to 'NormalProto' in an extension with public or '@usableFromInline' members; 'BADLibrary' has been imported as implementation-only}} public func testConstrainedExtensionUsingBadConformance() {} }