From 0c51dda34b3b24635c921459bec40175b4205040 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 15 Oct 2020 21:38:24 -0400 Subject: [PATCH 1/9] Sema: Pull @_implementationOnly override checking out of ExportabilityChecker --- lib/Sema/TypeCheckAccess.cpp | 37 ---------------------- lib/Sema/TypeCheckDecl.h | 1 + lib/Sema/TypeCheckDeclOverride.cpp | 50 ++++++++++++++++++++++++++++++ lib/Sema/TypeCheckDeclPrimary.cpp | 8 +++++ 4 files changed, 59 insertions(+), 37 deletions(-) diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index d1d1cdf79227b..163b7862664ac 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -1737,40 +1737,6 @@ class ExportabilityChecker : public DeclVisitor { return true; } - void checkOverride(const ValueDecl *VD) { - const ValueDecl *overridden = VD->getOverriddenDecl(); - if (!overridden) - return; - - auto *SF = VD->getDeclContext()->getParentSourceFile(); - assert(SF && "checking a non-source declaration?"); - - ModuleDecl *M = overridden->getModuleContext(); - if (SF->isImportedImplementationOnly(M)) { - VD->diagnose(diag::implementation_only_override_import_without_attr, - overridden->getDescriptiveKind()) - .fixItInsert(VD->getAttributeInsertionLoc(false), - "@_implementationOnly "); - overridden->diagnose(diag::overridden_here); - return; - } - - if (overridden->getAttrs().hasAttribute()) { - VD->diagnose(diag::implementation_only_override_without_attr, - overridden->getDescriptiveKind()) - .fixItInsert(VD->getAttributeInsertionLoc(false), - "@_implementationOnly "); - overridden->diagnose(diag::overridden_here); - return; - } - - // FIXME: Check storage decls where the setter is in a separate module from - // the getter, which is a thing Objective-C can do. The ClangImporter - // doesn't make this easy, though, because it just gives the setter the same - // DeclContext as the property or subscript, which means we've lost the - // information about whether its module was implementation-only imported. - } - void visit(Decl *D) { if (D->isInvalid() || D->isImplicit()) return; @@ -1778,7 +1744,6 @@ class ExportabilityChecker : public DeclVisitor { if (auto *VD = dyn_cast(D)) { if (shouldSkipChecking(VD)) return; - checkOverride(VD); } // Note: references to @_spi and @_implementationOnly declarations from @@ -1828,8 +1793,6 @@ class ExportabilityChecker : public DeclVisitor { if (shouldSkipChecking(theVar)) return; - checkOverride(theVar); - // Only check the type of individual variables if we didn't check an // enclosing TypedPattern. if (seenVars.count(theVar) || theVar->isInvalid()) diff --git a/lib/Sema/TypeCheckDecl.h b/lib/Sema/TypeCheckDecl.h index 5f3a01c8348fb..5bb835b9e3585 100644 --- a/lib/Sema/TypeCheckDecl.h +++ b/lib/Sema/TypeCheckDecl.h @@ -33,6 +33,7 @@ const ConstructorDecl *findNonImplicitRequiredInit(const ConstructorDecl *CD); // Implemented in TypeCheckDeclOverride.cpp bool checkOverrides(ValueDecl *decl); +void checkImplementationOnlyOverride(const ValueDecl *VD); // Implemented in TypeCheckStorage.cpp void setBoundVarsTypeError(Pattern *pattern, ASTContext &ctx); diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index 11c7985877a54..5c330a6ead843 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -2156,3 +2156,53 @@ bool IsABICompatibleOverrideRequest::evaluate(Evaluator &evaluator, return derivedInterfaceTy->matches(overrideInterfaceTy, TypeMatchFlags::AllowABICompatible); } + +void swift::checkImplementationOnlyOverride(const ValueDecl *VD) { + if (VD->isImplicit()) + return; + + if (VD->getAttrs().hasAttribute()) + return; + + if (isa(VD)) + return; + + // Is this part of the module's API or ABI? + AccessScope accessScope = + VD->getFormalAccessScope(nullptr, + /*treatUsableFromInlineAsPublic*/true); + if (!accessScope.isPublic()) + return; + + const ValueDecl *overridden = VD->getOverriddenDecl(); + if (!overridden) + return; + + auto *SF = VD->getDeclContext()->getParentSourceFile(); + assert(SF && "checking a non-source declaration?"); + + ModuleDecl *M = overridden->getModuleContext(); + if (SF->isImportedImplementationOnly(M)) { + VD->diagnose(diag::implementation_only_override_import_without_attr, + overridden->getDescriptiveKind()) + .fixItInsert(VD->getAttributeInsertionLoc(false), + "@_implementationOnly "); + overridden->diagnose(diag::overridden_here); + return; + } + + if (overridden->getAttrs().hasAttribute()) { + VD->diagnose(diag::implementation_only_override_without_attr, + overridden->getDescriptiveKind()) + .fixItInsert(VD->getAttributeInsertionLoc(false), + "@_implementationOnly "); + overridden->diagnose(diag::overridden_here); + return; + } + + // FIXME: Check storage decls where the setter is in a separate module from + // the getter, which is a thing Objective-C can do. The ClangImporter + // doesn't make this easy, though, because it just gives the setter the same + // DeclContext as the property or subscript, which means we've lost the + // information about whether its module was implementation-only imported. +} diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index 6a5da452ac077..fa1163a9036e9 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -1551,6 +1551,8 @@ class DeclChecker : public DeclVisitor { } } + checkImplementationOnlyOverride(VD); + if (VD->getDeclContext()->getSelfClassDecl()) { if (VD->getValueInterfaceType()->hasDynamicSelfType()) { if (VD->hasStorage()) @@ -1772,6 +1774,8 @@ class DeclChecker : public DeclVisitor { } } + checkImplementationOnlyOverride(SD); + // Compute these requests in case they emit diagnostics. (void) SD->isGetterMutating(); (void) SD->isSetterMutating(); @@ -2364,6 +2368,8 @@ class DeclChecker : public DeclVisitor { } } + checkImplementationOnlyOverride(FD); + if (requiresDefinition(FD) && !FD->hasBody()) { // Complain if we should have a body. FD->diagnose(diag::func_decl_without_brace); @@ -2672,6 +2678,8 @@ class DeclChecker : public DeclVisitor { } } + checkImplementationOnlyOverride(CD); + // If this initializer overrides a 'required' initializer, it must itself // be marked 'required'. if (!CD->getAttrs().hasAttribute()) { From 0ea136738473696004aa7b9b14a449f4ba3bea26 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 15 Oct 2020 18:33:07 -0400 Subject: [PATCH 2/9] Sema: Refactoring in preparation for conformance availability checking Today, we check conformance exportability in two places: 1) For inlinable function bodies: in availability checking 2) For types in inlinable declaration signatures: in availability checking 3) For types in non-inlinable declaration signatures: in ExportabilityChecker I want to merge 2) and 3) together, and also generalize the conformance exportability check to check conformance availability as well. This is a preliminary refactoring towards achieving this goal. --- lib/Sema/ResilienceDiagnostics.cpp | 133 +++++++++++-------------- lib/Sema/TypeCheckAccess.cpp | 33 ++++--- lib/Sema/TypeCheckAccess.h | 14 +++ lib/Sema/TypeCheckAvailability.cpp | 154 ++++++++++++++++++++++------- lib/Sema/TypeCheckAvailability.h | 37 ++++++- lib/Sema/TypeChecker.h | 21 ++-- 6 files changed, 258 insertions(+), 134 deletions(-) diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp index 3a26433ce59c3..14b9ba6c23098 100644 --- a/lib/Sema/ResilienceDiagnostics.cpp +++ b/lib/Sema/ResilienceDiagnostics.cpp @@ -27,13 +27,6 @@ using namespace swift; -/// A uniquely-typed boolean to reduce the chances of accidentally inverting -/// a check. -enum class DowngradeToWarning: bool { - No, - Yes -}; - bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc, ConcreteDeclRef declRef, const DeclContext *DC, @@ -55,16 +48,17 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc, // Skip this check for accessors because the associated property or subscript // will also be checked, and will provide a better error message. if (!isa(D)) - if (diagnoseDeclRefExportability(loc, declRef, DC, Kind)) + if (diagnoseDeclRefExportability(loc, declRef, DC, + None, Kind)) return true; return false; } bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc, - const ValueDecl *D, - const DeclContext *DC, - FragileFunctionKind Kind) { + const ValueDecl *D, + const DeclContext *DC, + FragileFunctionKind Kind) { assert(Kind.kind != FragileFunctionKind::None); // Local declarations are OK. @@ -150,84 +144,79 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc, static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D, const SourceFile &userSF, const DeclContext *userDC, + Optional reason, FragileFunctionKind fragileKind) { - assert(fragileKind.kind != FragileFunctionKind::None); + if (fragileKind.kind == FragileFunctionKind::None && !reason.hasValue()) + return false; auto definingModule = D->getModuleContext(); + auto downgradeToWarning = DowngradeToWarning::No; auto originKind = getDisallowedOriginKind( - D, userSF, userDC->getInnermostDeclarationDeclContext()); + D, userSF, userDC->getInnermostDeclarationDeclContext(), + downgradeToWarning); if (originKind == DisallowedOriginKind::None) return false; - // TODO: different diagnostics ASTContext &ctx = definingModule->getASTContext(); - ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_from_hidden_module, - D->getDescriptiveKind(), D->getName(), - static_cast(fragileKind.kind), - definingModule->getName(), - static_cast(originKind)); - return true; -} -static bool -diagnoseGenericArgumentsExportability(SourceLoc loc, - SubstitutionMap subs, - const SourceFile &userSF, - const DeclContext *userDC) { - bool hadAnyIssues = false; - for (ProtocolConformanceRef conformance : subs.getConformances()) { - if (!conformance.isConcrete()) - continue; - const ProtocolConformance *concreteConf = conformance.getConcrete(); - - SubstitutionMap subConformanceSubs = - concreteConf->getSubstitutions(userSF.getParentModule()); - diagnoseGenericArgumentsExportability(loc, subConformanceSubs, userSF, userDC); - - const RootProtocolConformance *rootConf = - concreteConf->getRootConformance(); - ModuleDecl *M = rootConf->getDeclContext()->getParentModule(); - - auto originKind = getDisallowedOriginKind( - rootConf->getDeclContext()->getAsDecl(), - userSF, userDC->getInnermostDeclarationDeclContext()); - if (originKind == DisallowedOriginKind::None) - continue; - - ASTContext &ctx = M->getASTContext(); - ctx.Diags.diagnose(loc, diag::conformance_from_implementation_only_module, - rootConf->getType(), - rootConf->getProtocol()->getName(), 0, M->getName(), + if (fragileKind.kind == FragileFunctionKind::None) { + auto errorOrWarning = downgradeToWarning == DowngradeToWarning::Yes? + diag::decl_from_hidden_module_warn: + diag::decl_from_hidden_module; + ctx.Diags.diagnose(loc, errorOrWarning, + D->getDescriptiveKind(), + D->getName(), + static_cast(*reason), + definingModule->getName(), + static_cast(originKind)); + + D->diagnose(diag::kind_declared_here, DescriptiveDeclKind::Type); + } else { + ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_from_hidden_module, + D->getDescriptiveKind(), D->getName(), + static_cast(fragileKind.kind), + definingModule->getName(), static_cast(originKind)); - hadAnyIssues = true; } - return hadAnyIssues; + return true; } -void TypeChecker::diagnoseGenericTypeExportability(SourceLoc Loc, Type T, - const DeclContext *DC) { - const SourceFile *SF = DC->getParentSourceFile(); - if (!SF) - return; - - // FIXME: It would be nice to highlight just the part of the type that's - // problematic, but unfortunately the TypeRepr doesn't have the - // information we need and the Type doesn't easily map back to it. - if (auto *BGT = dyn_cast(T.getPointer())) { - ModuleDecl *useModule = SF->getParentModule(); - auto subs = T->getContextSubstitutionMap(useModule, BGT->getDecl()); - (void)diagnoseGenericArgumentsExportability(Loc, subs, *SF, DC); - } else if (auto *TAT = dyn_cast(T.getPointer())) { - auto subs = TAT->getSubstitutionMap(); - (void)diagnoseGenericArgumentsExportability(Loc, subs, *SF, DC); - } +bool +TypeChecker::diagnoseConformanceExportability(SourceLoc loc, + const RootProtocolConformance *rootConf, + const SourceFile &userSF, + const DeclContext *userDC, + Optional reason, + FragileFunctionKind fragileKind) { + if (fragileKind.kind == FragileFunctionKind::None && !reason.hasValue()) + return false; + + auto originKind = getDisallowedOriginKind( + rootConf->getDeclContext()->getAsDecl(), + userSF, userDC->getInnermostDeclarationDeclContext()); + if (originKind == DisallowedOriginKind::None) + return false; + + if (!reason.hasValue()) + reason = ExportabilityReason::General; + + ModuleDecl *M = rootConf->getDeclContext()->getParentModule(); + ASTContext &ctx = M->getASTContext(); + ctx.Diags.diagnose(loc, diag::conformance_from_implementation_only_module, + rootConf->getType(), + rootConf->getProtocol()->getName(), + static_cast(*reason), + M->getName(), + static_cast(originKind)); + return true; } bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc, ConcreteDeclRef declRef, const DeclContext *DC, + Optional reason, FragileFunctionKind fragileKind) { // We're only interested in diagnosing uses from source files. auto userSF = DC->getParentSourceFile(); @@ -235,11 +224,7 @@ TypeChecker::diagnoseDeclRefExportability(SourceLoc loc, return false; const ValueDecl *D = declRef.getDecl(); - if (diagnoseDeclExportability(loc, D, *userSF, DC, fragileKind)) - return true; - if (diagnoseGenericArgumentsExportability(loc, declRef.getSubstitutions(), - *userSF, DC)) { + if (diagnoseDeclExportability(loc, D, *userSF, DC, reason, fragileKind)) return true; - } return false; } diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index 163b7862664ac..99a65433b0643 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -35,15 +35,6 @@ using namespace swift; namespace { -/// A uniquely-typed boolean to reduce the chances of accidentally inverting -/// a check. -/// -/// \see checkTypeAccess -enum class DowngradeToWarning: bool { - No, - Yes -}; - /// Calls \p callback for each type in each requirement provided by /// \p source. static void forAllRequirementTypes( @@ -1464,15 +1455,17 @@ class UsableFromInlineChecker : public AccessControlCheckerBase, } }; +} // end anonymous namespace + /// Returns the kind of origin, implementation-only import or SPI declaration, /// that restricts exporting \p decl from the given file and context. /// /// Local variant to swift::getDisallowedOriginKind for downgrade to warnings. DisallowedOriginKind -getDisallowedOriginKind(const Decl *decl, - const SourceFile &userSF, - const Decl *userContext, - DowngradeToWarning &downgradeToWarning) { +swift::getDisallowedOriginKind(const Decl *decl, + const SourceFile &userSF, + const Decl *userContext, + DowngradeToWarning &downgradeToWarning) { downgradeToWarning = DowngradeToWarning::No; ModuleDecl *M = decl->getModuleContext(); if (userSF.isImportedImplementationOnly(M)) { @@ -1493,6 +1486,8 @@ getDisallowedOriginKind(const Decl *decl, return DisallowedOriginKind::None; }; +namespace { + // Diagnose public APIs exposing types that are either imported as // implementation-only or declared as SPI. class ExportabilityChecker : public DeclVisitor { @@ -2015,6 +2010,8 @@ class ExportabilityChecker : public DeclVisitor { /// Diagnose declarations whose signatures refer to unavailable types. class DeclAvailabilityChecker : public DeclVisitor { DeclContext *DC; + Optional ExportReason; + FragileFunctionKind FragileKind; void checkType(Type type, const TypeRepr *typeRepr, const Decl *context, bool allowUnavailableProtocol=false) { @@ -2036,12 +2033,14 @@ class DeclAvailabilityChecker : public DeclVisitor { if (allowUnavailableProtocol) flags |= DeclAvailabilityFlag::AllowPotentiallyUnavailableProtocol; - diagnoseTypeReprAvailability(typeRepr, DC, flags); + diagnoseTypeReprAvailability(typeRepr, DC, ExportReason, FragileKind, + flags); } // Check the type for references to unavailable conformances. if (type) - diagnoseTypeAvailability(type, context->getLoc(), DC); + diagnoseTypeAvailability(type, context->getLoc(), DC, + ExportReason, FragileKind); } void checkGenericParams(const GenericContext *ownerCtx, @@ -2070,7 +2069,9 @@ class DeclAvailabilityChecker : public DeclVisitor { public: explicit DeclAvailabilityChecker(Decl *D) - : DC(D->getInnermostDeclContext()) {} + : DC(D->getInnermostDeclContext()) { + FragileKind = DC->getFragileFunctionKind(); + } void visit(Decl *D) { if (D->isImplicit()) diff --git a/lib/Sema/TypeCheckAccess.h b/lib/Sema/TypeCheckAccess.h index d29523ef39a8d..764603a85ac09 100644 --- a/lib/Sema/TypeCheckAccess.h +++ b/lib/Sema/TypeCheckAccess.h @@ -43,12 +43,26 @@ enum class DisallowedOriginKind : uint8_t { None }; +/// A uniquely-typed boolean to reduce the chances of accidentally inverting +/// a check. +/// +/// \see checkTypeAccess +enum class DowngradeToWarning: bool { + No, + Yes +}; + /// Returns the kind of origin, implementation-only import or SPI declaration, /// that restricts exporting \p decl from the given file and context. DisallowedOriginKind getDisallowedOriginKind(const Decl *decl, const SourceFile &userSF, const Decl *userContext); +DisallowedOriginKind getDisallowedOriginKind(const Decl *decl, + const SourceFile &userSF, + const Decl *userContext, + DowngradeToWarning &downgradeToWarning); + } // end namespace swift #endif diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 0b6386d97128d..92ca195d08d9a 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -22,6 +22,7 @@ #include "swift/AST/Initializer.h" #include "swift/AST/NameLookup.h" #include "swift/AST/Pattern.h" +#include "swift/AST/ProtocolConformance.h" #include "swift/AST/SourceFile.h" #include "swift/AST/TypeDeclFinder.h" #include "swift/AST/TypeRefinementContext.h" @@ -2344,6 +2345,7 @@ class AvailabilityWalker : public ASTWalker { DeclContext *DC; MemberAccessContext AccessContext = MemberAccessContext::Getter; SmallVector ExprStack; + Optional ExportReason; FragileFunctionKind FragileKind; /// Returns true if DC is an \c init(rawValue:) declaration and it is marked @@ -2360,9 +2362,11 @@ class AvailabilityWalker : public ASTWalker { } public: - AvailabilityWalker(DeclContext *DC) : Context(DC->getASTContext()), DC(DC) { - FragileKind = DC->getFragileFunctionKind(); - } + AvailabilityWalker(DeclContext *DC, + Optional ExportReason, + FragileFunctionKind FragileKind) + : Context(DC->getASTContext()), DC(DC), + ExportReason(ExportReason), FragileKind(FragileKind) {} bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override { return false; @@ -2430,16 +2434,20 @@ class AvailabilityWalker : public ASTWalker { if (auto T = dyn_cast(E)) { if (!T->isImplicit()) if (auto type = T->getType()) - diagnoseTypeAvailability(type, E->getLoc(), DC); + diagnoseTypeAvailability(type, E->getLoc(), DC, + ExportReason, FragileKind); } if (auto CE = dyn_cast(E)) { for (auto *param : *CE->getParameters()) { - diagnoseTypeAvailability(param->getInterfaceType(), E->getLoc(), DC); + diagnoseTypeAvailability(param->getInterfaceType(), E->getLoc(), DC, + ExportReason, FragileKind); } - diagnoseTypeAvailability(CE->getResultType(), E->getLoc(), DC); + diagnoseTypeAvailability(CE->getResultType(), E->getLoc(), DC, + ExportReason, FragileKind); } if (auto IE = dyn_cast(E)) { - diagnoseTypeAvailability(IE->getCastType(), E->getLoc(), DC); + diagnoseTypeAvailability(IE->getCastType(), E->getLoc(), DC, + ExportReason, FragileKind); } return visitChildren(); @@ -2453,7 +2461,7 @@ class AvailabilityWalker : public ASTWalker { } bool walkToTypeReprPre(TypeRepr *T) override { - diagnoseTypeReprAvailability(T, DC); + diagnoseTypeReprAvailability(T, DC, ExportReason, FragileKind); return false; } @@ -2660,10 +2668,23 @@ AvailabilityWalker::diagAvailability(ConcreteDeclRef declRef, SourceRange R, return false; } - if (FragileKind.kind != FragileFunctionKind::None) + if (FragileKind.kind != FragileFunctionKind::None) { if (R.isValid()) if (TypeChecker::diagnoseInlinableDeclRef(R.Start, declRef, DC, FragileKind)) return true; + } else if (ExportReason.hasValue()) { + if (R.isValid()) + if (TypeChecker::diagnoseDeclRefExportability(R.Start, declRef, DC, + ExportReason, FragileKind)) + return true; + } + + if (R.isValid()) { + if (diagnoseSubstitutionMapAvailability(R.Start, declRef.getSubstitutions(), + DC, ExportReason, FragileKind)) { + return true; + } + } if (diagnoseExplicitUnavailability(D, R, DC, call, Flags)) return true; @@ -2856,7 +2877,9 @@ AvailabilityWalker::diagnoseMemoryLayoutMigration(const ValueDecl *D, /// Diagnose uses of unavailable declarations. void swift::diagAvailability(const Expr *E, DeclContext *DC) { - AvailabilityWalker walker(DC); + Optional reason = None; + FragileFunctionKind fragileKind = DC->getFragileFunctionKind(); + AvailabilityWalker walker(DC, reason, fragileKind); const_cast(E)->walk(walker); } @@ -2864,9 +2887,13 @@ namespace { class StmtAvailabilityWalker : public ASTWalker { DeclContext *DC; + Optional ExportReason; + FragileFunctionKind FragileKind; public: - explicit StmtAvailabilityWalker(DeclContext *DC) : DC(DC) {} + explicit StmtAvailabilityWalker(DeclContext *DC) : DC(DC) { + FragileKind = DC->getFragileFunctionKind(); + } /// We'll visit the expression from performSyntacticExprDiagnostics(). std::pair walkToExprPre(Expr *E) override { @@ -2874,14 +2901,15 @@ class StmtAvailabilityWalker : public ASTWalker { } bool walkToTypeReprPre(TypeRepr *T) override { - diagnoseTypeReprAvailability(T, DC); + diagnoseTypeReprAvailability(T, DC, ExportReason, FragileKind); return false; } std::pair walkToPatternPre(Pattern *P) override { if (auto *IP = dyn_cast(P)) if (auto T = IP->getCastType()) - diagnoseTypeAvailability(T, P->getLoc(), DC); + diagnoseTypeAvailability(T, P->getLoc(), DC, + ExportReason, FragileKind); return std::make_pair(true, P); } @@ -2902,12 +2930,16 @@ namespace { class TypeReprAvailabilityWalker : public ASTWalker { DeclContext *DC; + Optional reason; + FragileFunctionKind fragileKind; DeclAvailabilityFlags flags; bool checkComponentIdentTypeRepr(ComponentIdentTypeRepr *ITR) { if (auto *typeDecl = ITR->getBoundDecl()) { auto range = ITR->getNameLoc().getSourceRange(); - if (diagnoseDeclAvailability(typeDecl, DC, range, flags)) + if (diagnoseDeclAvailability(typeDecl, range, DC, + reason, fragileKind, + flags)) return true; } @@ -2918,7 +2950,9 @@ class TypeReprAvailabilityWalker : public ASTWalker { genericFlags -= DeclAvailabilityFlag::AllowPotentiallyUnavailableProtocol; for (auto *genericArg : GTR->getGenericArgs()) { - if (diagnoseTypeReprAvailability(genericArg, DC, genericFlags)) + if (diagnoseTypeReprAvailability(genericArg, DC, + reason, fragileKind, + genericFlags)) foundAnyIssues = true; } } @@ -2930,8 +2964,10 @@ class TypeReprAvailabilityWalker : public ASTWalker { bool foundAnyIssues = false; TypeReprAvailabilityWalker(DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind, DeclAvailabilityFlags flags) - : DC(DC), flags(flags) {} + : DC(DC), reason(reason), fragileKind(fragileKind), flags(flags) {} bool walkToTypeReprPre(TypeRepr *T) override { if (auto *ITR = dyn_cast(T)) { @@ -2965,8 +3001,10 @@ class TypeReprAvailabilityWalker : public ASTWalker { } bool swift::diagnoseTypeReprAvailability(const TypeRepr *T, DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind, DeclAvailabilityFlags flags) { - TypeReprAvailabilityWalker walker(DC, flags); + TypeReprAvailabilityWalker walker(DC, reason, fragileKind, flags); const_cast(T)->walk(walker); return walker.foundAnyIssues; } @@ -2974,31 +3012,38 @@ bool swift::diagnoseTypeReprAvailability(const TypeRepr *T, DeclContext *DC, namespace { class ProblematicTypeFinder : public TypeDeclFinder { + SourceLoc Loc; DeclContext *DC; + Optional ExportReason; FragileFunctionKind FragileKind; - SourceLoc Loc; public: - ProblematicTypeFinder(DeclContext *DC, SourceLoc Loc) - : DC(DC), Loc(Loc) { - FragileKind = DC->getFragileFunctionKind(); - } + ProblematicTypeFinder(SourceLoc Loc, DeclContext *DC, + Optional ExportReason, + FragileFunctionKind FragileKind) + : Loc(Loc), DC(DC), + ExportReason(ExportReason), + FragileKind(FragileKind) {} Action visitNominalType(NominalType *ty) override { - if (FragileKind.kind != FragileFunctionKind::None) - TypeChecker::diagnoseGenericTypeExportability(Loc, ty, DC); + /// FIXME return Action::Continue; } Action visitBoundGenericType(BoundGenericType *ty) override { - if (FragileKind.kind != FragileFunctionKind::None) - TypeChecker::diagnoseGenericTypeExportability(Loc, ty, DC); + ModuleDecl *useModule = DC->getParentModule(); + auto subs = ty->getContextSubstitutionMap(useModule, ty->getDecl()); + (void) diagnoseSubstitutionMapAvailability(Loc, subs, DC, + ExportReason, + FragileKind); return Action::Continue; } Action visitTypeAliasType(TypeAliasType *ty) override { - if (FragileKind.kind != FragileFunctionKind::None) - TypeChecker::diagnoseGenericTypeExportability(Loc, ty, DC); + auto subs = ty->getSubstitutionMap(); + (void) diagnoseSubstitutionMapAvailability(Loc, subs, DC, + ExportReason, + FragileKind); return Action::Continue; } @@ -3006,7 +3051,8 @@ class ProblematicTypeFinder : public TypeDeclFinder { // post-visitor so that we diagnose any unexportable component // types first. Action walkToTypePost(Type T) override { - if (FragileKind.kind != FragileFunctionKind::None) { + if (FragileKind.kind != FragileFunctionKind::None || + ExportReason.hasValue()) { if (auto fnType = T->getAs()) { if (auto clangType = fnType->getClangTypeInfo().getType()) { auto &ctx = DC->getASTContext(); @@ -3028,19 +3074,61 @@ class ProblematicTypeFinder : public TypeDeclFinder { } -void swift::diagnoseTypeAvailability(Type T, SourceLoc loc, DeclContext *DC) { - T.walk(ProblematicTypeFinder(DC, loc)); +void swift::diagnoseTypeAvailability(Type T, SourceLoc loc, DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind) { + T.walk(ProblematicTypeFinder(loc, DC, reason, fragileKind)); +} + +bool +swift::diagnoseConformanceAvailability(SourceLoc loc, + ProtocolConformanceRef conformance, + const DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind) { + if (!conformance.isConcrete()) + return false; + const ProtocolConformance *concreteConf = conformance.getConcrete(); + + SubstitutionMap subConformanceSubs = + concreteConf->getSubstitutions(DC->getParentModule()); + diagnoseSubstitutionMapAvailability(loc, subConformanceSubs, DC, + reason, fragileKind); + + const RootProtocolConformance *rootConf = + concreteConf->getRootConformance(); + + return TypeChecker::diagnoseConformanceExportability( + loc, rootConf, *DC->getParentSourceFile(), DC, + reason, fragileKind); +} + +bool +swift::diagnoseSubstitutionMapAvailability(SourceLoc loc, + SubstitutionMap subs, + const DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind) { + bool hadAnyIssues = false; + for (ProtocolConformanceRef conformance : subs.getConformances()) { + if (diagnoseConformanceAvailability(loc, conformance, DC, + reason, fragileKind)) + hadAnyIssues = true; + } + return hadAnyIssues; } /// Run the Availability-diagnostics algorithm otherwise used in an expr /// context, but for non-expr contexts such as TypeDecls referenced from /// TypeReprs. bool swift::diagnoseDeclAvailability(const ValueDecl *Decl, - DeclContext *DC, SourceRange R, + DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind, DeclAvailabilityFlags Flags) { - AvailabilityWalker AW(DC); + AvailabilityWalker AW(DC, reason, fragileKind); return AW.diagAvailability(const_cast(Decl), R, nullptr, Flags); } diff --git a/lib/Sema/TypeCheckAvailability.h b/lib/Sema/TypeCheckAvailability.h index a13151fa93fd0..e19a9fe25b61a 100644 --- a/lib/Sema/TypeCheckAvailability.h +++ b/lib/Sema/TypeCheckAvailability.h @@ -28,7 +28,10 @@ namespace swift { class Expr; class InFlightDiagnostic; class Decl; + struct FragileFunctionKind; + class ProtocolConformanceRef; class Stmt; + class SubstitutionMap; class Type; class TypeRepr; class ValueDecl; @@ -57,6 +60,16 @@ enum class DeclAvailabilityFlag : uint8_t { }; using DeclAvailabilityFlags = OptionSet; +// This enum must be kept in sync with +// diag::decl_from_hidden_module and +// diag::conformance_from_implementation_only_module. +enum class ExportabilityReason : unsigned { + General, + PropertyWrapper, + ExtensionWithPublicMembers, + ExtensionWithConditionalConformances +}; + /// Diagnose uses of unavailable declarations in expressions. void diagAvailability(const Expr *E, DeclContext *DC); @@ -66,17 +79,37 @@ void diagAvailability(const Stmt *S, DeclContext *DC); /// Diagnose uses of unavailable declarations in types. bool diagnoseTypeReprAvailability(const TypeRepr *T, DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind, DeclAvailabilityFlags flags = None); /// Diagnose uses of unavailable conformances in types. -void diagnoseTypeAvailability(Type T, SourceLoc loc, DeclContext *DC); +void diagnoseTypeAvailability(Type T, SourceLoc loc, DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind); + +bool +diagnoseConformanceAvailability(SourceLoc loc, + ProtocolConformanceRef conformance, + const DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind); + +bool +diagnoseSubstitutionMapAvailability(SourceLoc loc, + SubstitutionMap subs, + const DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind); /// Run the Availability-diagnostics algorithm otherwise used in an expr /// context, but for non-expr contexts such as TypeDecls referenced from /// TypeReprs. bool diagnoseDeclAvailability(const ValueDecl *Decl, - DeclContext *DC, SourceRange R, + DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind, DeclAvailabilityFlags Options); void diagnoseUnavailableOverride(ValueDecl *override, diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index e384ae6fc9dae..beb69dda5b82b 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -40,11 +40,13 @@ namespace swift { class GenericSignatureBuilder; class NominalTypeDecl; class NormalProtocolConformance; +class RootProtocolConformance; class TypeResolution; class TypeResolutionOptions; class TypoCorrectionResults; class ExprPattern; enum class TypeResolutionStage : uint8_t; +enum class ExportabilityReason : unsigned; namespace constraints { enum class ConstraintKind : char; @@ -959,17 +961,18 @@ bool diagnoseInlinableDeclRefAccess(SourceLoc loc, const ValueDecl *D, /// reasonably be shared. bool diagnoseDeclRefExportability(SourceLoc loc, ConcreteDeclRef declRef, const DeclContext *DC, + Optional exportability, FragileFunctionKind fragileKind); -/// Given that a type is used from a particular context which -/// exposes it in the interface of the current module, diagnose if its -/// generic arguments require the use of conformances that cannot reasonably -/// be shared. -/// -/// This method \e only checks how generic arguments are used; it is assumed -/// that the declarations involved have already been checked elsewhere. -void diagnoseGenericTypeExportability(SourceLoc loc, Type type, - const DeclContext *DC); +/// Given that a conformance is used from a particular context which +/// exposes it in the interface of the current module, diagnose if the +/// conformance is SPI or visible via an implementation-only import. +bool diagnoseConformanceExportability(SourceLoc loc, + const RootProtocolConformance *rootConf, + const SourceFile &userSF, + const DeclContext *userDC, + Optional reason, + FragileFunctionKind fragileKind); /// \name Availability checking /// From 50c44c202634136a82a3534aad7651f974709d28 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 19 Oct 2020 15:57:03 -0400 Subject: [PATCH 3/9] Sema: Don't desugar the type when diagnosing unexportable Clang function types --- lib/Sema/TypeCheckAvailability.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 92ca195d08d9a..79d4728d1ec95 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -3061,8 +3061,7 @@ class ProblematicTypeFinder : public TypeDeclFinder { // but we need the canonical type to be serializable or else // canonicalization (e.g. in SIL) might break things. if (!loader->isSerializable(clangType, /*check canonical*/ true)) { - ctx.Diags.diagnose(Loc, diag::unexportable_clang_function_type, - fnType); + ctx.Diags.diagnose(Loc, diag::unexportable_clang_function_type, T); } } } From f48aa518842d9fc27b352a227868e98fa5f369a4 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 16 Oct 2020 00:42:32 -0400 Subject: [PATCH 4/9] Sema: Remove ExportabilityChecker Availability checking already knows how to check exportability for types and conformances referenced from inlinable function signatures and bodies. Let's generalize this to work on non-inlinable function signatures as well, allowing us to get rid of the ExportabilityChecker altogether. --- lib/Sema/TypeCheckAccess.cpp | 684 ++++++----------------------- lib/Sema/TypeCheckAvailability.cpp | 75 +++- lib/Sema/TypeCheckAvailability.h | 13 +- test/SPI/local_spi_decls.swift | 7 +- test/Sema/spi-in-decls.swift | 2 +- 5 files changed, 212 insertions(+), 569 deletions(-) diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index 99a65433b0643..d5d662d794de4 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -20,14 +20,11 @@ #include "TypeAccessScopeChecker.h" #include "swift/AST/ASTVisitor.h" #include "swift/AST/ASTWalker.h" -#include "swift/AST/ClangModuleLoader.h" #include "swift/AST/DiagnosticsSema.h" #include "swift/AST/ExistentialLayout.h" #include "swift/AST/Pattern.h" #include "swift/AST/ParameterList.h" -#include "swift/AST/ProtocolConformance.h" #include "swift/AST/TypeCheckRequests.h" -#include "swift/AST/TypeDeclFinder.h" using namespace swift; @@ -1486,139 +1483,93 @@ swift::getDisallowedOriginKind(const Decl *decl, return DisallowedOriginKind::None; }; -namespace { +static bool isExported(const ValueDecl *VD) { + if (VD->getAttrs().hasAttribute()) + return false; -// Diagnose public APIs exposing types that are either imported as -// implementation-only or declared as SPI. -class ExportabilityChecker : public DeclVisitor { - class Diagnoser; + // Is this part of the module's API or ABI? + AccessScope accessScope = + VD->getFormalAccessScope(nullptr, + /*treatUsableFromInlineAsPublic*/true); + if (accessScope.isPublic()) + return true; - void checkTypeImpl( - Type type, const TypeRepr *typeRepr, const SourceFile &SF, - const Decl *context, - const Diagnoser &diagnoser) { - // Don't bother checking errors. - if (type && type->hasError()) - return; + // Is this a stored property in a non-resilient struct or class? + auto *property = dyn_cast(VD); + if (!property || !property->hasStorage() || property->isStatic()) + return false; + auto *parentNominal = dyn_cast(property->getDeclContext()); + if (!parentNominal || parentNominal->isResilient()) + return false; - bool foundAnyIssues = false; - - // Check the TypeRepr first (if present), because that will give us a - // better diagnostic. - if (typeRepr) { - const_cast(typeRepr)->walk(TypeReprIdentFinder( - [&](const ComponentIdentTypeRepr *component) { - TypeDecl *typeDecl = component->getBoundDecl(); - auto downgradeToWarning = DowngradeToWarning::No; - auto originKind = getDisallowedOriginKind(typeDecl, SF, context, downgradeToWarning); - if (originKind != DisallowedOriginKind::None) { - diagnoser.diagnoseType(typeDecl, component, originKind, downgradeToWarning); - foundAnyIssues = true; - } + // Is that struct or class part of the module's API or ABI? + AccessScope parentAccessScope = parentNominal->getFormalAccessScope( + nullptr, /*treatUsableFromInlineAsPublic*/true); + if (parentAccessScope.isPublic()) + return true; + + return false; +} - // We still continue even in the diagnostic case to report multiple - // violations. - return true; - })); +static bool isExported(Decl *D) { + if (auto *VD = dyn_cast(D)) { + return isExported(VD); + } + if (auto *PBD = dyn_cast(D)) { + for (unsigned i = 0, e = PBD->getNumPatternEntries(); i < e; ++i) { + if (auto *VD = PBD->getAnchoringVarDecl(i)) + return isExported(VD); } - // Note that if we have a type, we can't skip checking it even if the - // TypeRepr is okay, because that's how we check what conformances are - // being used. - // - // We still don't want to do this if we found issues with the TypeRepr, - // though, because that would result in some issues being reported twice. - if (foundAnyIssues || !type) - return; + return false; + } + if (auto *ED = dyn_cast(D)) { + if (auto *NTD = ED->getExtendedNominal()) + return isExported(NTD); - class ProblematicTypeFinder : public TypeDeclFinder { - const SourceFile &SF; - const Decl *context; - const Diagnoser &diagnoser; - public: - ProblematicTypeFinder(const SourceFile &SF, const Decl *context, const Diagnoser &diagnoser) - : SF(SF), context(context), diagnoser(diagnoser) {} - - void visitTypeDecl(const TypeDecl *typeDecl) { - auto downgradeToWarning = DowngradeToWarning::No; - auto originKind = getDisallowedOriginKind(typeDecl, SF, context, downgradeToWarning); - if (originKind != DisallowedOriginKind::None) - diagnoser.diagnoseType(typeDecl, /*typeRepr*/nullptr, originKind, downgradeToWarning); - } + return false; + } - void visitSubstitutionMap(SubstitutionMap subs) { - for (ProtocolConformanceRef conformance : subs.getConformances()) { - if (!conformance.isConcrete()) - continue; - const ProtocolConformance *concreteConf = conformance.getConcrete(); - - SubstitutionMap subConformanceSubs = - concreteConf->getSubstitutions(SF.getParentModule()); - visitSubstitutionMap(subConformanceSubs); - - const RootProtocolConformance *rootConf = - concreteConf->getRootConformance(); - auto originKind = getDisallowedOriginKind( - rootConf->getDeclContext()->getAsDecl(), - SF, context); - if (originKind == DisallowedOriginKind::None) - continue; - diagnoser.diagnoseConformance(rootConf, originKind); - } - } + return true; +} - Action visitNominalType(NominalType *ty) override { - visitTypeDecl(ty->getDecl()); - return Action::Continue; - } +namespace { - Action visitBoundGenericType(BoundGenericType *ty) override { - visitTypeDecl(ty->getDecl()); - SubstitutionMap subs = - ty->getContextSubstitutionMap(SF.getParentModule(), ty->getDecl()); - visitSubstitutionMap(subs); - return Action::Continue; - } +/// Diagnose declarations whose signatures refer to unavailable types. +class DeclAvailabilityChecker : public DeclVisitor { + DeclContext *DC; + FragileFunctionKind FragileKind; + bool Exported; - Action visitTypeAliasType(TypeAliasType *ty) override { - visitTypeDecl(ty->getDecl()); - visitSubstitutionMap(ty->getSubstitutionMap()); - return Action::Continue; - } + void checkType(Type type, const TypeRepr *typeRepr, const Decl *context, + ExportabilityReason reason=ExportabilityReason::General, + bool allowUnavailableProtocol=false) { + // Don't bother checking errors. + if (type && type->hasError()) + return; - // We diagnose unserializable Clang function types in the - // post-visitor so that we diagnose any unexportable component - // types first. - Action walkToTypePost(Type T) override { - if (auto fnType = T->getAs()) { - if (auto clangType = fnType->getClangTypeInfo().getType()) { - auto loader = T->getASTContext().getClangModuleLoader(); - // Serialization will serialize the sugared type if it can, - // but we need the canonical type to be serializable or else - // canonicalization (e.g. in SIL) might break things. - if (!loader->isSerializable(clangType, /*check canonical*/ true)) { - diagnoser.diagnoseClangFunctionType(T, clangType); - } - } - } - return TypeDeclFinder::walkToTypePost(T); - } - }; + Optional optReason; + if (Exported) + optReason = reason; - type.walk(ProblematicTypeFinder(SF, context, diagnoser)); - } + DeclAvailabilityFlags flags = None; - void checkType( - Type type, const TypeRepr *typeRepr, const Decl *context, - const Diagnoser &diagnoser) { - auto *SF = context->getDeclContext()->getParentSourceFile(); - assert(SF && "checking a non-source declaration?"); - return checkTypeImpl(type, typeRepr, *SF, context, diagnoser); - } + // We allow a type to conform to a protocol that is less available than + // the type itself. This enables a type to retroactively model or directly + // conform to a protocol only available on newer OSes and yet still be used on + // older OSes. + // + // To support this, inside inheritance clauses we allow references to + // protocols that are unavailable in the current type refinement context. + if (allowUnavailableProtocol) + flags |= DeclAvailabilityFlag::AllowPotentiallyUnavailableProtocol; + + auto loc = context->getLoc(); + if (auto *varDecl = dyn_cast(context)) + loc = varDecl->getNameLoc(); - void checkType( - const TypeLoc &TL, const Decl *context, const Diagnoser &diagnoser) { - checkType(TL.getType(), TL.getTypeRepr(), context, diagnoser); + diagnoseTypeAvailability(typeRepr, type, loc, DC, + optReason, FragileKind, flags); } void checkGenericParams(const GenericContext *ownerCtx, @@ -1631,8 +1582,8 @@ class ExportabilityChecker : public DeclVisitor { if (param->getInherited().empty()) continue; assert(param->getInherited().size() == 1); - checkType(param->getInherited().front(), ownerDecl, - getDiagnoser(ownerDecl)); + auto inherited = param->getInherited().front(); + checkType(inherited.getType(), inherited.getTypeRepr(), ownerDecl); } } @@ -1640,112 +1591,16 @@ class ExportabilityChecker : public DeclVisitor { forAllRequirementTypes(WhereClauseOwner( const_cast(ownerCtx)), [&](Type type, TypeRepr *typeRepr) { - checkType(type, typeRepr, ownerDecl, getDiagnoser(ownerDecl)); + checkType(type, typeRepr, ownerDecl); }); } } - // This enum must be kept in sync with - // diag::decl_from_hidden_module and - // diag::conformance_from_implementation_only_module. - enum class Reason : unsigned { - General, - PropertyWrapper, - ExtensionWithPublicMembers, - ExtensionWithConditionalConformances - }; - - class Diagnoser { - const Decl *D; - Reason reason; - public: - Diagnoser(const Decl *D, Reason reason) : D(D), reason(reason) {} - - void diagnoseType(const TypeDecl *offendingType, - const TypeRepr *complainRepr, - DisallowedOriginKind originKind, - DowngradeToWarning downgradeToWarning) const { - ModuleDecl *M = offendingType->getModuleContext(); - auto errorOrWarning = downgradeToWarning == DowngradeToWarning::Yes? - diag::decl_from_hidden_module_warn: - diag::decl_from_hidden_module; - auto diag = D->diagnose(errorOrWarning, - offendingType->getDescriptiveKind(), - offendingType->getName(), - static_cast(reason), M->getName(), - static_cast(originKind)); - highlightOffendingType(diag, complainRepr); - } - - void diagnoseConformance(const ProtocolConformance *offendingConformance, - DisallowedOriginKind originKind) const { - ModuleDecl *M = offendingConformance->getDeclContext()->getParentModule(); - D->diagnose(diag::conformance_from_implementation_only_module, - offendingConformance->getType(), - offendingConformance->getProtocol()->getName(), - static_cast(reason), M->getName(), - static_cast(originKind)); - } - - void diagnoseClangFunctionType(Type fnType, const clang::Type *type) const { - D->diagnose(diag::unexportable_clang_function_type, fnType); - } - }; - - Diagnoser getDiagnoser(const Decl *D, Reason reason = Reason::General) { - return Diagnoser(D, reason); - } - public: - ExportabilityChecker() {} - - static bool shouldSkipChecking(const ValueDecl *VD) { - if (VD->getAttrs().hasAttribute()) - return true; - - // 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, - /*treatUsableFromInlineAsPublic*/true); - if (accessScope.isPublic()) - return false; - - // Is this a stored property in a non-resilient struct or class? - auto *property = dyn_cast(VD); - if (!property || !property->hasStorage() || property->isStatic()) - return true; - auto *parentNominal = dyn_cast(property->getDeclContext()); - if (!parentNominal || parentNominal->isResilient()) - return true; - - // Is that struct or class part of the module's API or ABI? - AccessScope parentAccessScope = parentNominal->getFormalAccessScope( - nullptr, /*treatUsableFromInlineAsPublic*/true); - if (parentAccessScope.isPublic()) - return false; - - return true; - } - - void visit(Decl *D) { - if (D->isInvalid() || D->isImplicit()) - return; - - if (auto *VD = dyn_cast(D)) { - if (shouldSkipChecking(VD)) - return; - } - - // Note: references to @_spi and @_implementationOnly declarations from - // @inlinable code are diagnosed by DeclAvailabilityChecker below. - auto *DC = D->getInnermostDeclContext(); - if (DC->getFragileFunctionKind().kind == FragileFunctionKind::None) - DeclVisitor::visit(D); + explicit DeclAvailabilityChecker(Decl *D) + : DC(D->getInnermostDeclContext()) { + FragileKind = DC->getFragileFunctionKind(); + Exported = isExported(D); } // Force all kinds to be handled at a lower level. @@ -1785,20 +1640,18 @@ class ExportabilityChecker : public DeclVisitor { void checkNamedPattern(const NamedPattern *NP, const llvm::DenseSet &seenVars) { const VarDecl *theVar = NP->getDecl(); - if (shouldSkipChecking(theVar)) - return; // Only check the type of individual variables if we didn't check an // enclosing TypedPattern. - if (seenVars.count(theVar) || theVar->isInvalid()) + if (seenVars.count(theVar)) return; - checkType(theVar->getInterfaceType(), /*typeRepr*/nullptr, theVar, - getDiagnoser(theVar)); + checkType(theVar->getValueInterfaceType(), /*typeRepr*/nullptr, theVar); } /// \see visitPatternBindingDecl - void checkTypedPattern(const TypedPattern *TP, + void checkTypedPattern(PatternBindingDecl *PBD, + const TypedPattern *TP, llvm::DenseSet &seenVars) { // FIXME: We need to figure out if this is a stored or computed property, // so we pull out some random VarDecl in the pattern. They're all going to @@ -1808,18 +1661,15 @@ class ExportabilityChecker : public DeclVisitor { seenVars.insert(V); anyVar = V; }); - if (!anyVar) - return; - if (shouldSkipChecking(anyVar)) - return; checkType(TP->hasType() ? TP->getType() : Type(), - TP->getTypeRepr(), anyVar, getDiagnoser(anyVar)); + TP->getTypeRepr(), anyVar ? (Decl *)anyVar : (Decl *)PBD); // Check the property wrapper types. - for (auto attr : anyVar->getAttachedPropertyWrappers()) - checkType(attr->getType(), attr->getTypeRepr(), anyVar, - getDiagnoser(anyVar, Reason::PropertyWrapper)); + if (anyVar) + for (auto attr : anyVar->getAttachedPropertyWrappers()) + checkType(attr->getType(), attr->getTypeRepr(), anyVar, + ExportabilityReason::PropertyWrapper); } void visitPatternBindingDecl(PatternBindingDecl *PBD) { @@ -1834,7 +1684,7 @@ class ExportabilityChecker : public DeclVisitor { auto *TP = dyn_cast(P); if (!TP) return; - checkTypedPattern(TP, seenVars); + checkTypedPattern(PBD, TP, seenVars); }); seenVars.clear(); } @@ -1843,22 +1693,22 @@ class ExportabilityChecker : public DeclVisitor { void visitTypeAliasDecl(TypeAliasDecl *TAD) { checkGenericParams(TAD, TAD); checkType(TAD->getUnderlyingType(), - TAD->getUnderlyingTypeRepr(), TAD, getDiagnoser(TAD)); + TAD->getUnderlyingTypeRepr(), TAD); } void visitAssociatedTypeDecl(AssociatedTypeDecl *assocType) { llvm::for_each(assocType->getInherited(), [&](TypeLoc requirement) { - checkType(requirement, assocType, getDiagnoser(assocType)); + checkType(requirement.getType(), requirement.getTypeRepr(), + assocType); }); checkType(assocType->getDefaultDefinitionType(), - assocType->getDefaultDefinitionTypeRepr(), assocType, - getDiagnoser(assocType)); + assocType->getDefaultDefinitionTypeRepr(), assocType); if (assocType->getTrailingWhereClause()) { forAllRequirementTypes(assocType, [&](Type type, TypeRepr *typeRepr) { - checkType(type, typeRepr, assocType, getDiagnoser(assocType)); + checkType(type, typeRepr, assocType); }); } } @@ -1867,20 +1717,24 @@ class ExportabilityChecker : public DeclVisitor { checkGenericParams(nominal, nominal); llvm::for_each(nominal->getInherited(), - [&](TypeLoc nextInherited) { - checkType(nextInherited, nominal, getDiagnoser(nominal)); + [&](TypeLoc inherited) { + checkType(inherited.getType(), inherited.getTypeRepr(), + nominal, ExportabilityReason::General, + /*allowUnavailableProtocol=*/true); }); } void visitProtocolDecl(ProtocolDecl *proto) { llvm::for_each(proto->getInherited(), [&](TypeLoc requirement) { - checkType(requirement, proto, getDiagnoser(proto)); + checkType(requirement.getType(), requirement.getTypeRepr(), proto, + ExportabilityReason::General, + /*allowUnavailableProtocol=*/false); }); if (proto->getTrailingWhereClause()) { forAllRequirementTypes(proto, [&](Type type, TypeRepr *typeRepr) { - checkType(type, typeRepr, proto, getDiagnoser(proto)); + checkType(type, typeRepr, proto); }); } } @@ -1889,76 +1743,82 @@ class ExportabilityChecker : public DeclVisitor { checkGenericParams(SD, SD); for (auto &P : *SD->getIndices()) { - checkType(P->getInterfaceType(), P->getTypeRepr(), SD, - getDiagnoser(SD)); + checkType(P->getInterfaceType(), P->getTypeRepr(), SD); } - checkType(SD->getElementInterfaceType(), SD->getElementTypeRepr(), SD, - getDiagnoser(SD)); + checkType(SD->getElementInterfaceType(), SD->getElementTypeRepr(), SD); } void visitAbstractFunctionDecl(AbstractFunctionDecl *fn) { checkGenericParams(fn, fn); for (auto *P : *fn->getParameters()) - checkType(P->getInterfaceType(), P->getTypeRepr(), fn, - getDiagnoser(fn)); + checkType(P->getInterfaceType(), P->getTypeRepr(), fn); } void visitFuncDecl(FuncDecl *FD) { visitAbstractFunctionDecl(FD); - checkType(FD->getResultInterfaceType(), FD->getResultTypeRepr(), FD, - getDiagnoser(FD)); + checkType(FD->getResultInterfaceType(), FD->getResultTypeRepr(), FD); } void visitEnumElementDecl(EnumElementDecl *EED) { if (!EED->hasAssociatedValues()) return; for (auto &P : *EED->getParameterList()) - checkType(P->getInterfaceType(), P->getTypeRepr(), EED, - getDiagnoser(EED)); + checkType(P->getInterfaceType(), P->getTypeRepr(), EED); } void checkConstrainedExtensionRequirements(ExtensionDecl *ED, - Reason reason) { + bool hasExportedMembers) { if (!ED->getTrailingWhereClause()) return; + + ExportabilityReason reason = + hasExportedMembers ? ExportabilityReason::ExtensionWithPublicMembers + : ExportabilityReason::ExtensionWithConditionalConformances; + forAllRequirementTypes(ED, [&](Type type, TypeRepr *typeRepr) { - checkType(type, typeRepr, ED, getDiagnoser(ED, reason)); + checkType(type, typeRepr, ED, reason); }); } void visitExtensionDecl(ExtensionDecl *ED) { auto extendedType = ED->getExtendedNominal(); assert(extendedType && "valid extension with no extended type?"); - if (!extendedType || shouldSkipChecking(extendedType)) + if (!extendedType) return; - // FIXME: We should allow conforming to implementation-only protocols, - // but just hide that from interfaces. + // The rules here are tricky. + // + // 1) If the extension defines conformances, the conformed-to protocols + // must be exported. llvm::for_each(ED->getInherited(), - [&](TypeLoc nextInherited) { - checkType(nextInherited, ED, getDiagnoser(ED)); + [&](TypeLoc inherited) { + checkType(inherited.getType(), inherited.getTypeRepr(), + ED, ExportabilityReason::General, + /*allowUnavailableProtocol=*/true); }); - bool hasPublicMembers = llvm::any_of(ED->getMembers(), - [](const Decl *member) -> bool { + bool wasExported = Exported; + + // 2) If the extension contains exported members, the as-written + // extended type should be exportable. + bool hasExportedMembers = llvm::any_of(ED->getMembers(), + [](const Decl *member) -> bool { auto *valueMember = dyn_cast(member); if (!valueMember) return false; - return !shouldSkipChecking(valueMember); + return isExported(valueMember); }); - if (hasPublicMembers) { - checkType(ED->getExtendedType(), ED->getExtendedTypeRepr(), ED, - getDiagnoser(ED, Reason::ExtensionWithPublicMembers)); - } + Exported = wasExported && hasExportedMembers; + checkType(ED->getExtendedType(), ED->getExtendedTypeRepr(), ED, + ExportabilityReason::ExtensionWithPublicMembers); - if (hasPublicMembers || !ED->getInherited().empty()) { - Reason reason = - hasPublicMembers ? Reason::ExtensionWithPublicMembers - : Reason::ExtensionWithConditionalConformances; - checkConstrainedExtensionRequirements(ED, reason); - } + // 3) If the extension contains exported members or defines conformances, + // the 'where' clause must only name exported types. + Exported = wasExported && (hasExportedMembers || + !ED->getInherited().empty()); + checkConstrainedExtensionRequirements(ED, hasExportedMembers); } void checkPrecedenceGroup(const PrecedenceGroupDecl *PGD, @@ -1973,7 +1833,7 @@ class ExportabilityChecker : public DeclVisitor { auto diag = DE.diagnose(diagLoc, diag::decl_from_hidden_module, PGD->getDescriptiveKind(), PGD->getName(), - static_cast(Reason::General), M->getName(), + static_cast(ExportabilityReason::General), M->getName(), static_cast(DisallowedOriginKind::ImplementationOnly) ); if (refRange.isValid()) @@ -2007,267 +1867,6 @@ class ExportabilityChecker : public DeclVisitor { } }; -/// Diagnose declarations whose signatures refer to unavailable types. -class DeclAvailabilityChecker : public DeclVisitor { - DeclContext *DC; - Optional ExportReason; - FragileFunctionKind FragileKind; - - void checkType(Type type, const TypeRepr *typeRepr, const Decl *context, - bool allowUnavailableProtocol=false) { - // Don't bother checking errors. - if (type && type->hasError()) - return; - - // Check the TypeRepr for references to unavailable declarations. - if (typeRepr) { - DeclAvailabilityFlags flags = None; - - // We allow a type to conform to a protocol that is less available than - // the type itself. This enables a type to retroactively model or directly - // conform to a protocol only available on newer OSes and yet still be used on - // older OSes. - // - // To support this, inside inheritance clauses we allow references to - // protocols that are unavailable in the current type refinement context. - if (allowUnavailableProtocol) - flags |= DeclAvailabilityFlag::AllowPotentiallyUnavailableProtocol; - - diagnoseTypeReprAvailability(typeRepr, DC, ExportReason, FragileKind, - flags); - } - - // Check the type for references to unavailable conformances. - if (type) - diagnoseTypeAvailability(type, context->getLoc(), DC, - ExportReason, FragileKind); - } - - void checkGenericParams(const GenericContext *ownerCtx, - const ValueDecl *ownerDecl) { - if (!ownerCtx->isGenericContext()) - return; - - if (auto params = ownerCtx->getGenericParams()) { - for (auto param : *params) { - if (param->getInherited().empty()) - continue; - assert(param->getInherited().size() == 1); - auto inherited = param->getInherited().front(); - checkType(inherited.getType(), inherited.getTypeRepr(), ownerDecl); - } - } - - if (ownerCtx->getTrailingWhereClause()) { - forAllRequirementTypes(WhereClauseOwner( - const_cast(ownerCtx)), - [&](Type type, TypeRepr *typeRepr) { - checkType(type, typeRepr, ownerDecl); - }); - } - } - -public: - explicit DeclAvailabilityChecker(Decl *D) - : DC(D->getInnermostDeclContext()) { - FragileKind = DC->getFragileFunctionKind(); - } - - void visit(Decl *D) { - if (D->isImplicit()) - return; - - DeclVisitor::visit(D); - } - - // Force all kinds to be handled at a lower level. - void visitDecl(Decl *D) = delete; - void visitValueDecl(ValueDecl *D) = delete; - -#define UNREACHABLE(KIND, REASON) \ - void visit##KIND##Decl(KIND##Decl *D) { \ - llvm_unreachable(REASON); \ - } - UNREACHABLE(Import, "not applicable") - UNREACHABLE(TopLevelCode, "not applicable") - UNREACHABLE(Module, "not applicable") - - UNREACHABLE(Param, "handled by the enclosing declaration") - UNREACHABLE(GenericTypeParam, "handled by the enclosing declaration") - UNREACHABLE(MissingMember, "handled by the enclosing declaration") -#undef UNREACHABLE - -#define UNINTERESTING(KIND) \ - void visit##KIND##Decl(KIND##Decl *D) {} - - UNINTERESTING(PrefixOperator) // Does not reference other decls. - UNINTERESTING(PostfixOperator) // Does not reference other decls. - UNINTERESTING(InfixOperator) // Does not reference other decls. - UNINTERESTING(IfConfig) // Not applicable. - UNINTERESTING(PoundDiagnostic) // Not applicable. - UNINTERESTING(EnumCase) // Handled at the EnumElement level. - UNINTERESTING(Destructor) // Always correct. - UNINTERESTING(Accessor) // Handled by the Var or Subscript. - UNINTERESTING(OpaqueType) // TODO - UNINTERESTING(PrecedenceGroup) // Doesn't reference anything with availability. - - // Handled at the PatternBinding level; if the pattern has a simple - // "name: TheType" form, we can get better results by diagnosing the TypeRepr. - UNINTERESTING(Var) - - /// \see visitPatternBindingDecl - void checkNamedPattern(const NamedPattern *NP, - const llvm::DenseSet &seenVars) { - const VarDecl *theVar = NP->getDecl(); - - // Only check the type of individual variables if we didn't check an - // enclosing TypedPattern. - if (seenVars.count(theVar)) - return; - - checkType(theVar->getValueInterfaceType(), /*typeRepr*/nullptr, theVar); - } - - /// \see visitPatternBindingDecl - void checkTypedPattern(PatternBindingDecl *PBD, - const TypedPattern *TP, - llvm::DenseSet &seenVars) { - // FIXME: We need to figure out if this is a stored or computed property, - // so we pull out some random VarDecl in the pattern. They're all going to - // be the same, but still, ick. - const VarDecl *anyVar = nullptr; - TP->forEachVariable([&](VarDecl *V) { - seenVars.insert(V); - anyVar = V; - }); - - checkType(TP->hasType() ? TP->getType() : Type(), - TP->getTypeRepr(), PBD); - - // Check the property wrapper types. - if (anyVar) - for (auto attr : anyVar->getAttachedPropertyWrappers()) - checkType(attr->getType(), attr->getTypeRepr(), anyVar); - } - - void visitPatternBindingDecl(PatternBindingDecl *PBD) { - llvm::DenseSet seenVars; - for (auto idx : range(PBD->getNumPatternEntries())) { - PBD->getPattern(idx)->forEachNode([&](const Pattern *P) { - if (auto *NP = dyn_cast(P)) { - checkNamedPattern(NP, seenVars); - return; - } - - auto *TP = dyn_cast(P); - if (!TP) - return; - checkTypedPattern(PBD, TP, seenVars); - }); - seenVars.clear(); - } - } - - void visitTypeAliasDecl(TypeAliasDecl *TAD) { - checkGenericParams(TAD, TAD); - checkType(TAD->getUnderlyingType(), - TAD->getUnderlyingTypeRepr(), TAD); - } - - void visitAssociatedTypeDecl(AssociatedTypeDecl *assocType) { - llvm::for_each(assocType->getInherited(), - [&](TypeLoc requirement) { - checkType(requirement.getType(), requirement.getTypeRepr(), - assocType); - }); - checkType(assocType->getDefaultDefinitionType(), - assocType->getDefaultDefinitionTypeRepr(), assocType); - - if (assocType->getTrailingWhereClause()) { - forAllRequirementTypes(assocType, - [&](Type type, TypeRepr *typeRepr) { - checkType(type, typeRepr, assocType); - }); - } - } - - void visitNominalTypeDecl(const NominalTypeDecl *nominal) { - checkGenericParams(nominal, nominal); - - llvm::for_each(nominal->getInherited(), - [&](TypeLoc inherited) { - checkType(inherited.getType(), inherited.getTypeRepr(), - nominal, /*allowUnavailableProtocol=*/true); - }); - } - - void visitProtocolDecl(ProtocolDecl *proto) { - llvm::for_each(proto->getInherited(), - [&](TypeLoc requirement) { - checkType(requirement.getType(), requirement.getTypeRepr(), proto, - /*allowUnavailableProtocol=*/false); - }); - - if (proto->getTrailingWhereClause()) { - forAllRequirementTypes(proto, [&](Type type, TypeRepr *typeRepr) { - checkType(type, typeRepr, proto); - }); - } - } - - void visitSubscriptDecl(SubscriptDecl *SD) { - checkGenericParams(SD, SD); - - for (auto &P : *SD->getIndices()) { - checkType(P->getInterfaceType(), P->getTypeRepr(), SD); - } - checkType(SD->getElementInterfaceType(), SD->getElementTypeRepr(), SD); - } - - void visitAbstractFunctionDecl(AbstractFunctionDecl *fn) { - checkGenericParams(fn, fn); - - for (auto *P : *fn->getParameters()) - checkType(P->getInterfaceType(), P->getTypeRepr(), fn); - } - - void visitFuncDecl(FuncDecl *FD) { - visitAbstractFunctionDecl(FD); - checkType(FD->getResultInterfaceType(), FD->getResultTypeRepr(), FD); - } - - void visitEnumElementDecl(EnumElementDecl *EED) { - if (!EED->hasAssociatedValues()) - return; - for (auto &P : *EED->getParameterList()) - checkType(P->getInterfaceType(), P->getTypeRepr(), EED); - } - - void checkConstrainedExtensionRequirements(ExtensionDecl *ED) { - if (!ED->getTrailingWhereClause()) - return; - forAllRequirementTypes(ED, [&](Type type, TypeRepr *typeRepr) { - checkType(type, typeRepr, ED); - }); - } - - void visitExtensionDecl(ExtensionDecl *ED) { - auto extendedType = ED->getExtendedNominal(); - assert(extendedType && "valid extension with no extended type?"); - if (!extendedType) - return; - - llvm::for_each(ED->getInherited(), - [&](TypeLoc inherited) { - checkType(inherited.getType(), inherited.getTypeRepr(), - ED, /*allowUnavailableProtocol=*/true); - }); - - checkType(ED->getExtendedType(), ED->getExtendedTypeRepr(), ED); - checkConstrainedExtensionRequirements(ED); - } -}; - } // end anonymous namespace static void checkExtensionGenericParamAccess(const ExtensionDecl *ED) { @@ -2318,6 +1917,9 @@ void swift::checkAccessControl(Decl *D) { checkExtensionGenericParamAccess(ED); } - ExportabilityChecker().visit(D); + //ExportabilityChecker().visit(D); + if (D->isImplicit() || isa(D)) + return; + DeclAvailabilityChecker(D).visit(D); } diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 79d4728d1ec95..904916c4f5adb 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -2432,22 +2432,25 @@ class AvailabilityWalker : public ASTWalker { return skipChildren(); } if (auto T = dyn_cast(E)) { - if (!T->isImplicit()) - if (auto type = T->getType()) - diagnoseTypeAvailability(type, E->getLoc(), DC, - ExportReason, FragileKind); + if (!T->isImplicit()) { + diagnoseTypeAvailability(T->getTypeRepr(), T->getType(), E->getLoc(), + DC, ExportReason, FragileKind); + } } if (auto CE = dyn_cast(E)) { for (auto *param : *CE->getParameters()) { - diagnoseTypeAvailability(param->getInterfaceType(), E->getLoc(), DC, - ExportReason, FragileKind); + diagnoseTypeAvailability(param->getTypeRepr(), param->getInterfaceType(), + E->getLoc(), DC, ExportReason, FragileKind); } - diagnoseTypeAvailability(CE->getResultType(), E->getLoc(), DC, + diagnoseTypeAvailability(CE->hasExplicitResultType() + ? CE->getExplicitResultTypeRepr() + : nullptr, + CE->getResultType(), E->getLoc(), DC, ExportReason, FragileKind); } - if (auto IE = dyn_cast(E)) { - diagnoseTypeAvailability(IE->getCastType(), E->getLoc(), DC, - ExportReason, FragileKind); + if (auto CE = dyn_cast(E)) { + diagnoseTypeAvailability(CE->getCastTypeRepr(), CE->getCastType(), + E->getLoc(), DC, ExportReason, FragileKind); } return visitChildren(); @@ -2460,11 +2463,6 @@ class AvailabilityWalker : public ASTWalker { return E; } - bool walkToTypeReprPre(TypeRepr *T) override { - diagnoseTypeReprAvailability(T, DC, ExportReason, FragileKind); - return false; - } - bool diagAvailability(ConcreteDeclRef declRef, SourceRange R, const ApplyExpr *call = nullptr, DeclAvailabilityFlags flags = None) const; @@ -2907,9 +2905,8 @@ class StmtAvailabilityWalker : public ASTWalker { std::pair walkToPatternPre(Pattern *P) override { if (auto *IP = dyn_cast(P)) - if (auto T = IP->getCastType()) - diagnoseTypeAvailability(T, P->getLoc(), DC, - ExportReason, FragileKind); + diagnoseTypeAvailability(IP->getCastType(), P->getLoc(), DC, + ExportReason, FragileKind); return std::make_pair(true, P); } @@ -3004,6 +3001,8 @@ bool swift::diagnoseTypeReprAvailability(const TypeRepr *T, DeclContext *DC, Optional reason, FragileFunctionKind fragileKind, DeclAvailabilityFlags flags) { + if (!T) + return false; TypeReprAvailabilityWalker walker(DC, reason, fragileKind, flags); const_cast(T)->walk(walker); return walker.foundAnyIssues; @@ -3016,21 +3015,38 @@ class ProblematicTypeFinder : public TypeDeclFinder { DeclContext *DC; Optional ExportReason; FragileFunctionKind FragileKind; + DeclAvailabilityFlags Flags; public: ProblematicTypeFinder(SourceLoc Loc, DeclContext *DC, Optional ExportReason, - FragileFunctionKind FragileKind) + FragileFunctionKind FragileKind, + DeclAvailabilityFlags Flags) : Loc(Loc), DC(DC), ExportReason(ExportReason), - FragileKind(FragileKind) {} + FragileKind(FragileKind), + Flags(Flags) {} + + void visitTypeDecl(TypeDecl *decl) { + // We only need to diagnose exportability here. Availability was + // already checked on the TypeRepr. + if (FragileKind.kind != FragileFunctionKind::None || + ExportReason.hasValue()) { + TypeChecker::diagnoseDeclRefExportability(Loc, decl, DC, + ExportReason, FragileKind); + } + } Action visitNominalType(NominalType *ty) override { + visitTypeDecl(ty->getDecl()); + /// FIXME return Action::Continue; } Action visitBoundGenericType(BoundGenericType *ty) override { + visitTypeDecl(ty->getDecl()); + ModuleDecl *useModule = DC->getParentModule(); auto subs = ty->getContextSubstitutionMap(useModule, ty->getDecl()); (void) diagnoseSubstitutionMapAvailability(Loc, subs, DC, @@ -3040,6 +3056,8 @@ class ProblematicTypeFinder : public TypeDeclFinder { } Action visitTypeAliasType(TypeAliasType *ty) override { + visitTypeDecl(ty->getDecl()); + auto subs = ty->getSubstitutionMap(); (void) diagnoseSubstitutionMapAvailability(Loc, subs, DC, ExportReason, @@ -3075,8 +3093,21 @@ class ProblematicTypeFinder : public TypeDeclFinder { void swift::diagnoseTypeAvailability(Type T, SourceLoc loc, DeclContext *DC, Optional reason, - FragileFunctionKind fragileKind) { - T.walk(ProblematicTypeFinder(loc, DC, reason, fragileKind)); + FragileFunctionKind fragileKind, + DeclAvailabilityFlags flags) { + if (!T) + return; + T.walk(ProblematicTypeFinder(loc, DC, reason, fragileKind, flags)); +} + +void swift::diagnoseTypeAvailability(const TypeRepr *TR, Type T, SourceLoc loc, + DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind, + DeclAvailabilityFlags flags) { + if (diagnoseTypeReprAvailability(TR, DC, reason, fragileKind, flags)) + return; + diagnoseTypeAvailability(T, loc, DC, reason, fragileKind, flags); } bool diff --git a/lib/Sema/TypeCheckAvailability.h b/lib/Sema/TypeCheckAvailability.h index e19a9fe25b61a..40c500bde957b 100644 --- a/lib/Sema/TypeCheckAvailability.h +++ b/lib/Sema/TypeCheckAvailability.h @@ -86,7 +86,16 @@ bool diagnoseTypeReprAvailability(const TypeRepr *T, DeclContext *DC, /// Diagnose uses of unavailable conformances in types. void diagnoseTypeAvailability(Type T, SourceLoc loc, DeclContext *DC, Optional reason, - FragileFunctionKind fragileKind); + FragileFunctionKind fragileKind, + DeclAvailabilityFlags flags = None); + +/// Checks both a TypeRepr and a Type, but avoids emitting duplicate +/// diagnostics by only checking the Type if the TypeRepr succeeded. +void diagnoseTypeAvailability(const TypeRepr *TR, Type T, SourceLoc loc, + DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind, + DeclAvailabilityFlags flags = None); bool diagnoseConformanceAvailability(SourceLoc loc, @@ -110,7 +119,7 @@ bool diagnoseDeclAvailability(const ValueDecl *Decl, DeclContext *DC, Optional reason, FragileFunctionKind fragileKind, - DeclAvailabilityFlags Options); + DeclAvailabilityFlags flags = None); void diagnoseUnavailableOverride(ValueDecl *override, const ValueDecl *base, diff --git a/test/SPI/local_spi_decls.swift b/test/SPI/local_spi_decls.swift index 3b4e68142d525..cc8abe6baeb6c 100644 --- a/test/SPI/local_spi_decls.swift +++ b/test/SPI/local_spi_decls.swift @@ -13,9 +13,9 @@ @_spi() public func emptyParensSPI() {} // expected-error {{expected an SPI identifier as subject of the '@_spi' attribute}} @_spi(set) public func keywordSPI() {} // expected-error {{expected an SPI identifier as subject of the '@_spi' attribute}} -@_spi(S) public class SPIClass { // expected-note 5 {{type declared here}} - // expected-note @-1 3 {{class 'SPIClass' is not '@usableFromInline' or public}} - // expected-note @-2 {{class 'SPIClass' is not public}} +@_spi(S) public class SPIClass { // expected-note 6 {{type declared here}} + // expected-note@-1 3 {{class 'SPIClass' is not '@usableFromInline' or public}} + // expected-note@-2 {{class 'SPIClass' is not public}} public init() {} // expected-note@-1 2 {{initializer 'init()' is not '@usableFromInline' or public}} // expected-note@-2 {{initializer 'init()' is not public}} @@ -38,6 +38,7 @@ func inlinable() -> SPIClass { // expected-error {{class 'SPIClass' is '@_spi' a } @_spi(S) public struct SPIStruct { // expected-note 2 {{struct 'SPIStruct' is not '@usableFromInline' or public}} +// expected-note@-1 2 {{type declared here}} // FIXME: Misleading diagnostic here public init() {} // expected-note@-1 2 {{initializer 'init()' is not '@usableFromInline' or public}} diff --git a/test/Sema/spi-in-decls.swift b/test/Sema/spi-in-decls.swift index e83c88a7cc660..e3fd458bf86fd 100644 --- a/test/Sema/spi-in-decls.swift +++ b/test/Sema/spi-in-decls.swift @@ -22,7 +22,7 @@ extension NormalClass: NormalProto { } @_spi(X) -public struct BadStruct {} // expected-note 27 {{type declared here}} +public struct BadStruct {} // expected-note 34 {{type declared here}} @_spi(X) public protocol BadProto {} // expected-note 20 {{type declared here}} @_spi(X) From e10df96983f337dda96be6797b1e0be7fb676391 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 19 Oct 2020 15:37:09 -0400 Subject: [PATCH 5/9] Sema: Fold TypeChecker::isDeclAvailable() into its callers --- lib/Sema/TypeCheckAvailability.cpp | 39 +++++++++--------------------- lib/Sema/TypeCheckProtocol.cpp | 15 ++++++------ lib/Sema/TypeChecker.h | 11 --------- 3 files changed, 19 insertions(+), 46 deletions(-) diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 904916c4f5adb..b0e5570af7908 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -712,29 +712,6 @@ TypeChecker::overApproximateAvailabilityAtLocation(SourceLoc loc, return OverApproximateContext; } -bool TypeChecker::isDeclAvailable(const Decl *D, SourceLoc referenceLoc, - const DeclContext *referenceDC, - AvailabilityContext &OutAvailableInfo) { - ASTContext &Context = referenceDC->getASTContext(); - - AvailabilityContext safeRangeUnderApprox{ - AvailabilityInference::availableRange(D, Context)}; - AvailabilityContext runningOSOverApprox = - overApproximateAvailabilityAtLocation(referenceLoc, referenceDC); - - // The reference is safe if an over-approximation of the running OS - // versions is fully contained within an under-approximation - // of the versions on which the declaration is available. If this - // containment cannot be guaranteed, we say the reference is - // not available. - if (!(runningOSOverApprox.isContainedIn(safeRangeUnderApprox))) { - OutAvailableInfo = safeRangeUnderApprox; - return false; - } - - return true; -} - Optional TypeChecker::checkDeclarationAvailability(const Decl *D, SourceLoc referenceLoc, const DeclContext *referenceDC) { @@ -749,12 +726,20 @@ TypeChecker::checkDeclarationAvailability(const Decl *D, SourceLoc referenceLoc, return None; } - auto safeRangeUnderApprox = AvailabilityContext::neverAvailable(); - if (isDeclAvailable(D, referenceLoc, referenceDC, safeRangeUnderApprox)) { + AvailabilityContext runningOSOverApprox = + overApproximateAvailabilityAtLocation(referenceLoc, referenceDC); + + AvailabilityContext safeRangeUnderApprox{ + AvailabilityInference::availableRange(D, Context)}; + + // The reference is safe if an over-approximation of the running OS + // versions is fully contained within an under-approximation + // of the versions on which the declaration is available. If this + // containment cannot be guaranteed, we say the reference is + // not available. + if (runningOSOverApprox.isContainedIn(safeRangeUnderApprox)) return None; - } - // safeRangeUnderApprox now holds the safe range. VersionRange version = safeRangeUnderApprox.getOSVersion(); return UnavailabilityReason::requiresVersionRange(version); } diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 01cf489a25a6b..48ff739eb8306 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -5455,14 +5455,13 @@ static void inferStaticInitializeObjCMetadata(ClassDecl *classDecl) { // only statically initialize the Objective-C metadata when running on // a new-enough OS. if (auto sourceFile = classDecl->getParentSourceFile()) { - AvailabilityContext availableInfo = AvailabilityContext::alwaysAvailable(); - for (Decl *enclosingDecl = classDecl; enclosingDecl; - enclosingDecl = enclosingDecl->getDeclContext() - ->getInnermostDeclarationDeclContext()) { - if (!TypeChecker::isDeclAvailable(enclosingDecl, SourceLoc(), sourceFile, - availableInfo)) - return; - } + AvailabilityContext safeRangeUnderApprox{ + AvailabilityInference::availableRange(classDecl, ctx)}; + AvailabilityContext runningOSOverApprox = + AvailabilityContext::forDeploymentTarget(ctx); + + if (!runningOSOverApprox.isContainedIn(safeRangeUnderApprox)) + return; } // Infer @_staticInitializeObjCMetadata. diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index beb69dda5b82b..77bfc6f10ff47 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -1011,17 +1011,6 @@ TypeRefinementContext *getOrBuildTypeRefinementContext(SourceFile *SF); Optional> diagnosticIfDeclCannotBePotentiallyUnavailable(const Decl *D); -/// Checks whether a declaration is available when referred to at the given -/// location (this reference location must be in the passed-in -/// reference DeclContext). -/// If the declaration is available, return true. -/// If the declaration is not available, return false and write the -/// declaration's availability info to the out parameter -/// \p OutAvailableRange. -bool isDeclAvailable(const Decl *D, SourceLoc referenceLoc, - const DeclContext *referenceDC, - AvailabilityContext &OutAvailableRange); - /// Checks whether a declaration should be considered unavailable when /// referred to at the given location and, if so, returns the reason why the /// declaration is unavailable. Returns None is the declaration is From c2ed3476658ec9adad8895694a36f2abf85f7a9e Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 19 Oct 2020 18:11:20 -0400 Subject: [PATCH 6/9] Sema: Pass ValueDecl instead of ConcreteDeclRef in a few places in availability checking The substitution map is checked elsewhere. --- lib/Sema/ResilienceDiagnostics.cpp | 37 +++++++++--------------------- lib/Sema/TypeCheckAvailability.cpp | 4 ++-- lib/Sema/TypeChecker.h | 5 ++-- 3 files changed, 16 insertions(+), 30 deletions(-) diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp index 14b9ba6c23098..3c459162d35f0 100644 --- a/lib/Sema/ResilienceDiagnostics.cpp +++ b/lib/Sema/ResilienceDiagnostics.cpp @@ -28,12 +28,11 @@ using namespace swift; bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc, - ConcreteDeclRef declRef, + const ValueDecl *D, const DeclContext *DC, FragileFunctionKind Kind) { assert(Kind.kind != FragileFunctionKind::None); - const ValueDecl *D = declRef.getDecl(); // Do some important fast-path checks that apply to all cases. // Type parameters are OK. @@ -48,7 +47,7 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc, // Skip this check for accessors because the associated property or subscript // will also be checked, and will provide a better error message. if (!isa(D)) - if (diagnoseDeclRefExportability(loc, declRef, DC, + if (diagnoseDeclRefExportability(loc, D, DC, None, Kind)) return true; @@ -141,11 +140,12 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc, return (downgradeToWarning == DowngradeToWarning::No); } -static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D, - const SourceFile &userSF, - const DeclContext *userDC, - Optional reason, - FragileFunctionKind fragileKind) { +bool +TypeChecker::diagnoseDeclRefExportability(SourceLoc loc, + const ValueDecl *D, + const DeclContext *DC, + Optional reason, + FragileFunctionKind fragileKind) { if (fragileKind.kind == FragileFunctionKind::None && !reason.hasValue()) return false; @@ -153,7 +153,9 @@ static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D, auto downgradeToWarning = DowngradeToWarning::No; auto originKind = getDisallowedOriginKind( - D, userSF, userDC->getInnermostDeclarationDeclContext(), + D, + *DC->getParentSourceFile(), + DC->getInnermostDeclarationDeclContext(), downgradeToWarning); if (originKind == DisallowedOriginKind::None) return false; @@ -211,20 +213,3 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc, static_cast(originKind)); return true; } - -bool -TypeChecker::diagnoseDeclRefExportability(SourceLoc loc, - ConcreteDeclRef declRef, - const DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind) { - // We're only interested in diagnosing uses from source files. - auto userSF = DC->getParentSourceFile(); - if (!userSF) - return false; - - const ValueDecl *D = declRef.getDecl(); - if (diagnoseDeclExportability(loc, D, *userSF, DC, reason, fragileKind)) - return true; - return false; -} diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index b0e5570af7908..37391769bb587 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -2653,11 +2653,11 @@ AvailabilityWalker::diagAvailability(ConcreteDeclRef declRef, SourceRange R, if (FragileKind.kind != FragileFunctionKind::None) { if (R.isValid()) - if (TypeChecker::diagnoseInlinableDeclRef(R.Start, declRef, DC, FragileKind)) + if (TypeChecker::diagnoseInlinableDeclRef(R.Start, D, DC, FragileKind)) return true; } else if (ExportReason.hasValue()) { if (R.isValid()) - if (TypeChecker::diagnoseDeclRefExportability(R.Start, declRef, DC, + if (TypeChecker::diagnoseDeclRefExportability(R.Start, D, DC, ExportReason, FragileKind)) return true; } diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 77bfc6f10ff47..84fba621d72e3 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -947,7 +947,7 @@ DeclName getObjectLiteralConstructorName(ASTContext &ctx, ModuleDecl *getStdlibModule(const DeclContext *dc); /// \name Resilience diagnostics -bool diagnoseInlinableDeclRef(SourceLoc loc, ConcreteDeclRef declRef, +bool diagnoseInlinableDeclRef(SourceLoc loc, const ValueDecl *D, const DeclContext *DC, FragileFunctionKind Kind); Expr *buildDefaultInitializer(Type type); @@ -959,7 +959,8 @@ bool diagnoseInlinableDeclRefAccess(SourceLoc loc, const ValueDecl *D, /// Given that a declaration is used from a particular context which /// exposes it in the interface of the current module, diagnose if it cannot /// reasonably be shared. -bool diagnoseDeclRefExportability(SourceLoc loc, ConcreteDeclRef declRef, +bool diagnoseDeclRefExportability(SourceLoc loc, + const ValueDecl *D, const DeclContext *DC, Optional exportability, FragileFunctionKind fragileKind); From 9f08ab3918a6f83d34671e8c72c4c002e595f801 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 19 Oct 2020 18:45:46 -0400 Subject: [PATCH 7/9] Sema: Remove an unnecessary overload of TypeChecker::diagnosePotentialUnavailability() --- lib/Sema/TypeCheckAvailability.cpp | 15 ++++----------- lib/Sema/TypeChecker.h | 8 -------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 37391769bb587..154db4ef00bdc 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -744,14 +744,6 @@ TypeChecker::checkDeclarationAvailability(const Decl *D, SourceLoc referenceLoc, return UnavailabilityReason::requiresVersionRange(version); } -void TypeChecker::diagnosePotentialUnavailability( - const ValueDecl *D, SourceRange ReferenceRange, - const DeclContext *ReferenceDC, - const UnavailabilityReason &Reason) { - diagnosePotentialUnavailability(D, D->getName(), ReferenceRange, - ReferenceDC, Reason); -} - /// A class that walks the AST to find the innermost (i.e., deepest) node that /// contains a target SourceRange and matches a particular criterion. /// This class finds the innermost nodes of interest by walking @@ -1382,8 +1374,9 @@ void TypeChecker::diagnosePotentialOpaqueTypeUnavailability( } void TypeChecker::diagnosePotentialUnavailability( - const Decl *D, DeclName Name, SourceRange ReferenceRange, - const DeclContext *ReferenceDC, const UnavailabilityReason &Reason) { + const ValueDecl *D, SourceRange ReferenceRange, + const DeclContext *ReferenceDC, + const UnavailabilityReason &Reason) { ASTContext &Context = ReferenceDC->getASTContext(); // We only emit diagnostics for API unavailability, not for explicitly @@ -1398,7 +1391,7 @@ void TypeChecker::diagnosePotentialUnavailability( auto Err = Context.Diags.diagnose( ReferenceRange.Start, diag::availability_decl_only_version_newer, - Name, prettyPlatformString(targetPlatform(Context.LangOpts)), + D->getName(), prettyPlatformString(targetPlatform(Context.LangOpts)), Reason.getRequiredOSVersionRange().getLowerEndpoint()); // Direct a fixit to the error if an existing guard is nearly-correct diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 84fba621d72e3..146a47780cf85 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -1033,14 +1033,6 @@ void diagnosePotentialUnavailability(const ValueDecl *D, const DeclContext *ReferenceDC, const UnavailabilityReason &Reason); -// Emits a diagnostic, if necessary, for a reference to a declaration -// that is potentially unavailable at the given source location, using -// Name as the diagnostic name. -void diagnosePotentialUnavailability(const Decl *D, DeclName Name, - SourceRange ReferenceRange, - const DeclContext *ReferenceDC, - const UnavailabilityReason &Reason); - void diagnosePotentialOpaqueTypeUnavailability(SourceRange ReferenceRange, const DeclContext *ReferenceDC, From 3044c057e8b2a11f14b38d6445c430c54ca11802 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 19 Oct 2020 22:51:59 -0400 Subject: [PATCH 8/9] Sema: Wrap up some availability-related state in a new ExportContext type The ExportContext describes the restrictions, if any, on what declarations can be uttered in a specific place in the program. Here, the place is either the signature of a declaration, or the body of a function. --- lib/Sema/ResilienceDiagnostics.cpp | 53 +++--- lib/Sema/TypeCheckAccess.cpp | 75 +-------- lib/Sema/TypeCheckAvailability.cpp | 258 ++++++++++++++++++----------- lib/Sema/TypeCheckAvailability.h | 62 ++++--- lib/Sema/TypeCheckStmt.cpp | 11 +- lib/Sema/TypeChecker.h | 16 +- 6 files changed, 247 insertions(+), 228 deletions(-) diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp index 3c459162d35f0..17e7e821c6f94 100644 --- a/lib/Sema/ResilienceDiagnostics.cpp +++ b/lib/Sema/ResilienceDiagnostics.cpp @@ -29,9 +29,10 @@ using namespace swift; bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc, const ValueDecl *D, - const DeclContext *DC, - FragileFunctionKind Kind) { - assert(Kind.kind != FragileFunctionKind::None); + ExportContext where) { + auto fragileKind = where.getFragileFunctionKind(); + if (fragileKind.kind == FragileFunctionKind::None) + return false; // Do some important fast-path checks that apply to all cases. @@ -40,15 +41,14 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc, return false; // Check whether the declaration is accessible. - if (diagnoseInlinableDeclRefAccess(loc, D, DC, Kind)) + if (diagnoseInlinableDeclRefAccess(loc, D, where)) return true; // Check whether the declaration comes from a publically-imported module. // Skip this check for accessors because the associated property or subscript // will also be checked, and will provide a better error message. if (!isa(D)) - if (diagnoseDeclRefExportability(loc, D, DC, - None, Kind)) + if (diagnoseDeclRefExportability(loc, D, where)) return true; return false; @@ -56,9 +56,10 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc, bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc, const ValueDecl *D, - const DeclContext *DC, - FragileFunctionKind Kind) { - assert(Kind.kind != FragileFunctionKind::None); + ExportContext where) { + auto *DC = where.getDeclContext(); + auto fragileKind = where.getFragileFunctionKind(); + assert(fragileKind.kind != FragileFunctionKind::None); // Local declarations are OK. if (D->getDeclContext()->isLocalContext()) @@ -66,7 +67,7 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc, // Public declarations or SPI used from SPI are OK. if (D->getFormalAccessScope(/*useDC=*/nullptr, - Kind.allowUsableFromInline).isPublic() && + fragileKind.allowUsableFromInline).isPublic() && !(D->isSPI() && !DC->getInnermostDeclarationDeclContext()->isSPI())) return false; @@ -126,10 +127,10 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc, loc, diagID, D->getDescriptiveKind(), diagName, D->getFormalAccessScope().accessLevelForDiagnostics(), - static_cast(Kind.kind), + static_cast(fragileKind.kind), isAccessor); - if (Kind.allowUsableFromInline) { + if (fragileKind.allowUsableFromInline) { Context.Diags.diagnose(D, diag::resilience_decl_declared_here, D->getDescriptiveKind(), diagName, isAccessor); } else { @@ -143,15 +144,15 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc, bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc, const ValueDecl *D, - const DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind) { - if (fragileKind.kind == FragileFunctionKind::None && !reason.hasValue()) + ExportContext where) { + if (!where.mustOnlyReferenceExportedDecls()) return false; auto definingModule = D->getModuleContext(); auto downgradeToWarning = DowngradeToWarning::No; + + auto *DC = where.getDeclContext(); auto originKind = getDisallowedOriginKind( D, *DC->getParentSourceFile(), @@ -162,6 +163,9 @@ TypeChecker::diagnoseDeclRefExportability(SourceLoc loc, ASTContext &ctx = definingModule->getASTContext(); + auto fragileKind = where.getFragileFunctionKind(); + auto reason = where.getExportabilityReason(); + if (fragileKind.kind == FragileFunctionKind::None) { auto errorOrWarning = downgradeToWarning == DowngradeToWarning::Yes? diag::decl_from_hidden_module_warn: @@ -187,24 +191,25 @@ TypeChecker::diagnoseDeclRefExportability(SourceLoc loc, bool TypeChecker::diagnoseConformanceExportability(SourceLoc loc, const RootProtocolConformance *rootConf, - const SourceFile &userSF, - const DeclContext *userDC, - Optional reason, - FragileFunctionKind fragileKind) { - if (fragileKind.kind == FragileFunctionKind::None && !reason.hasValue()) + ExportContext where) { + if (!where.mustOnlyReferenceExportedDecls()) return false; + auto *DC = where.getDeclContext(); auto originKind = getDisallowedOriginKind( rootConf->getDeclContext()->getAsDecl(), - userSF, userDC->getInnermostDeclarationDeclContext()); + *DC->getParentSourceFile(), + DC->getInnermostDeclarationDeclContext()); if (originKind == DisallowedOriginKind::None) return false; + ModuleDecl *M = rootConf->getDeclContext()->getParentModule(); + ASTContext &ctx = M->getASTContext(); + + auto reason = where.getExportabilityReason(); if (!reason.hasValue()) reason = ExportabilityReason::General; - ModuleDecl *M = rootConf->getDeclContext()->getParentModule(); - ASTContext &ctx = M->getASTContext(); ctx.Diags.diagnose(loc, diag::conformance_from_implementation_only_module, rootConf->getType(), rootConf->getProtocol()->getName(), diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index d5d662d794de4..c31bafb8a922a 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -1483,63 +1483,11 @@ swift::getDisallowedOriginKind(const Decl *decl, return DisallowedOriginKind::None; }; -static bool isExported(const ValueDecl *VD) { - if (VD->getAttrs().hasAttribute()) - return false; - - // Is this part of the module's API or ABI? - AccessScope accessScope = - VD->getFormalAccessScope(nullptr, - /*treatUsableFromInlineAsPublic*/true); - if (accessScope.isPublic()) - return true; - - // Is this a stored property in a non-resilient struct or class? - auto *property = dyn_cast(VD); - if (!property || !property->hasStorage() || property->isStatic()) - return false; - auto *parentNominal = dyn_cast(property->getDeclContext()); - if (!parentNominal || parentNominal->isResilient()) - return false; - - // Is that struct or class part of the module's API or ABI? - AccessScope parentAccessScope = parentNominal->getFormalAccessScope( - nullptr, /*treatUsableFromInlineAsPublic*/true); - if (parentAccessScope.isPublic()) - return true; - - return false; -} - -static bool isExported(Decl *D) { - if (auto *VD = dyn_cast(D)) { - return isExported(VD); - } - if (auto *PBD = dyn_cast(D)) { - for (unsigned i = 0, e = PBD->getNumPatternEntries(); i < e; ++i) { - if (auto *VD = PBD->getAnchoringVarDecl(i)) - return isExported(VD); - } - - return false; - } - if (auto *ED = dyn_cast(D)) { - if (auto *NTD = ED->getExtendedNominal()) - return isExported(NTD); - - return false; - } - - return true; -} - namespace { /// Diagnose declarations whose signatures refer to unavailable types. class DeclAvailabilityChecker : public DeclVisitor { - DeclContext *DC; - FragileFunctionKind FragileKind; - bool Exported; + ExportContext Where; void checkType(Type type, const TypeRepr *typeRepr, const Decl *context, ExportabilityReason reason=ExportabilityReason::General, @@ -1548,10 +1496,6 @@ class DeclAvailabilityChecker : public DeclVisitor { if (type && type->hasError()) return; - Optional optReason; - if (Exported) - optReason = reason; - DeclAvailabilityFlags flags = None; // We allow a type to conform to a protocol that is less available than @@ -1568,8 +1512,8 @@ class DeclAvailabilityChecker : public DeclVisitor { if (auto *varDecl = dyn_cast(context)) loc = varDecl->getNameLoc(); - diagnoseTypeAvailability(typeRepr, type, loc, DC, - optReason, FragileKind, flags); + diagnoseTypeAvailability(typeRepr, type, loc, + Where.forReason(reason), flags); } void checkGenericParams(const GenericContext *ownerCtx, @@ -1598,10 +1542,7 @@ class DeclAvailabilityChecker : public DeclVisitor { public: explicit DeclAvailabilityChecker(Decl *D) - : DC(D->getInnermostDeclContext()) { - FragileKind = DC->getFragileFunctionKind(); - Exported = isExported(D); - } + : Where(ExportContext::forDeclSignature(D)) {} // Force all kinds to be handled at a lower level. void visitDecl(Decl *D) = delete; @@ -1798,7 +1739,7 @@ class DeclAvailabilityChecker : public DeclVisitor { /*allowUnavailableProtocol=*/true); }); - bool wasExported = Exported; + auto wasWhere = Where; // 2) If the extension contains exported members, the as-written // extended type should be exportable. @@ -1810,14 +1751,14 @@ class DeclAvailabilityChecker : public DeclVisitor { return isExported(valueMember); }); - Exported = wasExported && hasExportedMembers; + Where = wasWhere.forExported(hasExportedMembers); checkType(ED->getExtendedType(), ED->getExtendedTypeRepr(), ED, ExportabilityReason::ExtensionWithPublicMembers); // 3) If the extension contains exported members or defines conformances, // the 'where' clause must only name exported types. - Exported = wasExported && (hasExportedMembers || - !ED->getInherited().empty()); + Where = wasWhere.forExported(hasExportedMembers || + !ED->getInherited().empty()); checkConstrainedExtensionRequirements(ED, hasExportedMembers); } diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 154db4ef00bdc..692c4a06d8e7c 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -37,6 +37,111 @@ #include "llvm/Support/SaveAndRestore.h" using namespace swift; +ExportContext::ExportContext(DeclContext *DC, FragileFunctionKind kind, + bool spi, bool exported) + : DC(DC), FragileKind(kind) { + SPI = spi; + Exported = exported; + Reason = ExportabilityReason::General; +} + +bool swift::isExported(const ValueDecl *VD) { + if (VD->getAttrs().hasAttribute()) + return false; + + // Is this part of the module's API or ABI? + AccessScope accessScope = + VD->getFormalAccessScope(nullptr, + /*treatUsableFromInlineAsPublic*/true); + if (accessScope.isPublic()) + return true; + + // Is this a stored property in a non-resilient struct or class? + auto *property = dyn_cast(VD); + if (!property || !property->hasStorage() || property->isStatic()) + return false; + auto *parentNominal = dyn_cast(property->getDeclContext()); + if (!parentNominal || parentNominal->isResilient()) + return false; + + // Is that struct or class part of the module's API or ABI? + AccessScope parentAccessScope = parentNominal->getFormalAccessScope( + nullptr, /*treatUsableFromInlineAsPublic*/true); + if (parentAccessScope.isPublic()) + return true; + + return false; +} + +bool swift::isExported(const Decl *D) { + if (auto *VD = dyn_cast(D)) { + return isExported(VD); + } + if (auto *PBD = dyn_cast(D)) { + for (unsigned i = 0, e = PBD->getNumPatternEntries(); i < e; ++i) { + if (auto *VD = PBD->getAnchoringVarDecl(i)) + return isExported(VD); + } + + return false; + } + if (auto *ED = dyn_cast(D)) { + if (auto *NTD = ED->getExtendedNominal()) + return isExported(NTD); + + return false; + } + + return true; +} + +ExportContext ExportContext::forDeclSignature(Decl *D) { + auto *DC = D->getInnermostDeclContext(); + auto fragileKind = DC->getFragileFunctionKind(); + bool spi = D->isSPI(); + bool exported = ::isExported(D); + + return ExportContext(DC, fragileKind, spi, exported); +} + +ExportContext ExportContext::forFunctionBody(DeclContext *DC) { + ; + auto fragileKind = DC->getFragileFunctionKind(); + + bool spi = false; + bool exported = false; + + if (auto *D = DC->getInnermostDeclarationDeclContext()) { + spi = D->isSPI(); + } else { + assert(fragileKind.kind == FragileFunctionKind::None); + } + + return ExportContext(DC, fragileKind, spi, exported); +} + +ExportContext ExportContext::forReason(ExportabilityReason reason) const { + auto copy = *this; + copy.Reason = reason; + return copy; +} + +ExportContext ExportContext::forExported(bool exported) const { + auto copy = *this; + copy.Exported = isExported() && exported; + return copy; +} + +bool ExportContext::mustOnlyReferenceExportedDecls() const { + return Exported || FragileKind.kind != FragileFunctionKind::None; +} + +Optional ExportContext::getExportabilityReason() const { + if (Exported) + return Reason; + return None; +} + /// Returns the first availability attribute on the declaration that is active /// on the target platform. static const AvailableAttr *getActiveAvailableAttribute(const Decl *D, @@ -2320,15 +2425,14 @@ class AvailabilityWalker : public ASTWalker { }; ASTContext &Context; - DeclContext *DC; MemberAccessContext AccessContext = MemberAccessContext::Getter; SmallVector ExprStack; - Optional ExportReason; - FragileFunctionKind FragileKind; + ExportContext Where; /// Returns true if DC is an \c init(rawValue:) declaration and it is marked /// implicit. bool inSynthesizedInitRawValue() { + auto *DC = Where.getDeclContext(); auto init = dyn_cast_or_null( DC->getInnermostDeclarationDeclContext()); @@ -2340,11 +2444,8 @@ class AvailabilityWalker : public ASTWalker { } public: - AvailabilityWalker(DeclContext *DC, - Optional ExportReason, - FragileFunctionKind FragileKind) - : Context(DC->getASTContext()), DC(DC), - ExportReason(ExportReason), FragileKind(FragileKind) {} + AvailabilityWalker(ExportContext Where) + : Context(Where.getDeclContext()->getASTContext()), Where(Where) {} bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override { return false; @@ -2353,6 +2454,8 @@ class AvailabilityWalker : public ASTWalker { bool shouldWalkIntoTapExpression() override { return false; } std::pair walkToExprPre(Expr *E) override { + auto *DC = Where.getDeclContext(); + ExprStack.push_back(E); auto visitChildren = [&]() { return std::make_pair(true, E); }; @@ -2412,23 +2515,22 @@ class AvailabilityWalker : public ASTWalker { if (auto T = dyn_cast(E)) { if (!T->isImplicit()) { diagnoseTypeAvailability(T->getTypeRepr(), T->getType(), E->getLoc(), - DC, ExportReason, FragileKind); + Where); } } if (auto CE = dyn_cast(E)) { for (auto *param : *CE->getParameters()) { diagnoseTypeAvailability(param->getTypeRepr(), param->getInterfaceType(), - E->getLoc(), DC, ExportReason, FragileKind); + E->getLoc(), Where); } diagnoseTypeAvailability(CE->hasExplicitResultType() ? CE->getExplicitResultTypeRepr() : nullptr, - CE->getResultType(), E->getLoc(), DC, - ExportReason, FragileKind); + CE->getResultType(), E->getLoc(), Where); } if (auto CE = dyn_cast(E)) { diagnoseTypeAvailability(CE->getCastTypeRepr(), CE->getCastType(), - E->getLoc(), DC, ExportReason, FragileKind); + E->getLoc(), Where); } return visitChildren(); @@ -2512,6 +2614,7 @@ class AvailabilityWalker : public ASTWalker { return; // Diagnose for appropriate accessors, given the access context. + auto *DC = Where.getDeclContext(); maybeDiagStorageAccess(D, E->getSourceRange(), DC); } @@ -2644,24 +2747,25 @@ AvailabilityWalker::diagAvailability(ConcreteDeclRef declRef, SourceRange R, return false; } - if (FragileKind.kind != FragileFunctionKind::None) { + if (Where.getFragileFunctionKind().kind != FragileFunctionKind::None) { if (R.isValid()) - if (TypeChecker::diagnoseInlinableDeclRef(R.Start, D, DC, FragileKind)) + if (TypeChecker::diagnoseInlinableDeclRef(R.Start, D, Where)) return true; - } else if (ExportReason.hasValue()) { + } else if (Where.getExportabilityReason().hasValue()) { if (R.isValid()) - if (TypeChecker::diagnoseDeclRefExportability(R.Start, D, DC, - ExportReason, FragileKind)) + if (TypeChecker::diagnoseDeclRefExportability(R.Start, D, Where)) return true; } if (R.isValid()) { if (diagnoseSubstitutionMapAvailability(R.Start, declRef.getSubstitutions(), - DC, ExportReason, FragileKind)) { + Where)) { return true; } } + auto *DC = Where.getDeclContext(); + if (diagnoseExplicitUnavailability(D, R, DC, call, Flags)) return true; @@ -2738,6 +2842,7 @@ AvailabilityWalker::diagnoseIncDecRemoval(const ValueDecl *D, SourceRange R, // If the expression type is integer or floating point, then we can rewrite it // to "lvalue += 1". + auto *DC = Where.getDeclContext(); std::string replacement; if (isIntegerOrFloatingPointType(call->getType(), DC, Context)) replacement = isInc ? " += 1" : " -= 1"; @@ -2853,23 +2958,18 @@ AvailabilityWalker::diagnoseMemoryLayoutMigration(const ValueDecl *D, /// Diagnose uses of unavailable declarations. void swift::diagAvailability(const Expr *E, DeclContext *DC) { - Optional reason = None; - FragileFunctionKind fragileKind = DC->getFragileFunctionKind(); - AvailabilityWalker walker(DC, reason, fragileKind); + AvailabilityWalker walker(ExportContext::forFunctionBody(DC)); const_cast(E)->walk(walker); } namespace { class StmtAvailabilityWalker : public ASTWalker { - DeclContext *DC; - Optional ExportReason; - FragileFunctionKind FragileKind; + ExportContext Where; public: - explicit StmtAvailabilityWalker(DeclContext *DC) : DC(DC) { - FragileKind = DC->getFragileFunctionKind(); - } + explicit StmtAvailabilityWalker(DeclContext *DC) + : Where(ExportContext::forFunctionBody(DC)) {} /// We'll visit the expression from performSyntacticExprDiagnostics(). std::pair walkToExprPre(Expr *E) override { @@ -2877,14 +2977,13 @@ class StmtAvailabilityWalker : public ASTWalker { } bool walkToTypeReprPre(TypeRepr *T) override { - diagnoseTypeReprAvailability(T, DC, ExportReason, FragileKind); + diagnoseTypeReprAvailability(T, Where); return false; } std::pair walkToPatternPre(Pattern *P) override { if (auto *IP = dyn_cast(P)) - diagnoseTypeAvailability(IP->getCastType(), P->getLoc(), DC, - ExportReason, FragileKind); + diagnoseTypeAvailability(IP->getCastType(), P->getLoc(), Where); return std::make_pair(true, P); } @@ -2904,17 +3003,13 @@ void swift::diagAvailability(const Stmt *S, DeclContext *DC) { namespace { class TypeReprAvailabilityWalker : public ASTWalker { - DeclContext *DC; - Optional reason; - FragileFunctionKind fragileKind; + ExportContext where; DeclAvailabilityFlags flags; bool checkComponentIdentTypeRepr(ComponentIdentTypeRepr *ITR) { if (auto *typeDecl = ITR->getBoundDecl()) { auto range = ITR->getNameLoc().getSourceRange(); - if (diagnoseDeclAvailability(typeDecl, range, DC, - reason, fragileKind, - flags)) + if (diagnoseDeclAvailability(typeDecl, range, where, flags)) return true; } @@ -2925,9 +3020,7 @@ class TypeReprAvailabilityWalker : public ASTWalker { genericFlags -= DeclAvailabilityFlag::AllowPotentiallyUnavailableProtocol; for (auto *genericArg : GTR->getGenericArgs()) { - if (diagnoseTypeReprAvailability(genericArg, DC, - reason, fragileKind, - genericFlags)) + if (diagnoseTypeReprAvailability(genericArg, where, genericFlags)) foundAnyIssues = true; } } @@ -2938,11 +3031,9 @@ class TypeReprAvailabilityWalker : public ASTWalker { public: bool foundAnyIssues = false; - TypeReprAvailabilityWalker(DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind, + TypeReprAvailabilityWalker(ExportContext where, DeclAvailabilityFlags flags) - : DC(DC), reason(reason), fragileKind(fragileKind), flags(flags) {} + : where(where), flags(flags) {} bool walkToTypeReprPre(TypeRepr *T) override { if (auto *ITR = dyn_cast(T)) { @@ -2975,13 +3066,11 @@ class TypeReprAvailabilityWalker : public ASTWalker { } -bool swift::diagnoseTypeReprAvailability(const TypeRepr *T, DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind, +bool swift::diagnoseTypeReprAvailability(const TypeRepr *T, ExportContext where, DeclAvailabilityFlags flags) { if (!T) return false; - TypeReprAvailabilityWalker walker(DC, reason, fragileKind, flags); + TypeReprAvailabilityWalker walker(where, flags); const_cast(T)->walk(walker); return walker.foundAnyIssues; } @@ -2990,29 +3079,19 @@ namespace { class ProblematicTypeFinder : public TypeDeclFinder { SourceLoc Loc; - DeclContext *DC; - Optional ExportReason; - FragileFunctionKind FragileKind; + ExportContext Where; DeclAvailabilityFlags Flags; public: - ProblematicTypeFinder(SourceLoc Loc, DeclContext *DC, - Optional ExportReason, - FragileFunctionKind FragileKind, + ProblematicTypeFinder(SourceLoc Loc, ExportContext Where, DeclAvailabilityFlags Flags) - : Loc(Loc), DC(DC), - ExportReason(ExportReason), - FragileKind(FragileKind), - Flags(Flags) {} + : Loc(Loc), Where(Where), Flags(Flags) {} void visitTypeDecl(TypeDecl *decl) { // We only need to diagnose exportability here. Availability was // already checked on the TypeRepr. - if (FragileKind.kind != FragileFunctionKind::None || - ExportReason.hasValue()) { - TypeChecker::diagnoseDeclRefExportability(Loc, decl, DC, - ExportReason, FragileKind); - } + if (Where.mustOnlyReferenceExportedDecls()) + TypeChecker::diagnoseDeclRefExportability(Loc, decl, Where); } Action visitNominalType(NominalType *ty) override { @@ -3025,11 +3104,9 @@ class ProblematicTypeFinder : public TypeDeclFinder { Action visitBoundGenericType(BoundGenericType *ty) override { visitTypeDecl(ty->getDecl()); - ModuleDecl *useModule = DC->getParentModule(); + ModuleDecl *useModule = Where.getDeclContext()->getParentModule(); auto subs = ty->getContextSubstitutionMap(useModule, ty->getDecl()); - (void) diagnoseSubstitutionMapAvailability(Loc, subs, DC, - ExportReason, - FragileKind); + (void) diagnoseSubstitutionMapAvailability(Loc, subs, Where); return Action::Continue; } @@ -3037,9 +3114,7 @@ class ProblematicTypeFinder : public TypeDeclFinder { visitTypeDecl(ty->getDecl()); auto subs = ty->getSubstitutionMap(); - (void) diagnoseSubstitutionMapAvailability(Loc, subs, DC, - ExportReason, - FragileKind); + (void) diagnoseSubstitutionMapAvailability(Loc, subs, Where); return Action::Continue; } @@ -3047,10 +3122,10 @@ class ProblematicTypeFinder : public TypeDeclFinder { // post-visitor so that we diagnose any unexportable component // types first. Action walkToTypePost(Type T) override { - if (FragileKind.kind != FragileFunctionKind::None || - ExportReason.hasValue()) { + if (Where.mustOnlyReferenceExportedDecls()) { if (auto fnType = T->getAs()) { if (auto clangType = fnType->getClangTypeInfo().getType()) { + auto *DC = Where.getDeclContext(); auto &ctx = DC->getASTContext(); auto loader = ctx.getClangModuleLoader(); // Serialization will serialize the sugared type if it can, @@ -3069,58 +3144,47 @@ class ProblematicTypeFinder : public TypeDeclFinder { } -void swift::diagnoseTypeAvailability(Type T, SourceLoc loc, DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind, +void swift::diagnoseTypeAvailability(Type T, SourceLoc loc, ExportContext where, DeclAvailabilityFlags flags) { if (!T) return; - T.walk(ProblematicTypeFinder(loc, DC, reason, fragileKind, flags)); + T.walk(ProblematicTypeFinder(loc, where, flags)); } void swift::diagnoseTypeAvailability(const TypeRepr *TR, Type T, SourceLoc loc, - DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind, + ExportContext where, DeclAvailabilityFlags flags) { - if (diagnoseTypeReprAvailability(TR, DC, reason, fragileKind, flags)) + if (diagnoseTypeReprAvailability(TR, where, flags)) return; - diagnoseTypeAvailability(T, loc, DC, reason, fragileKind, flags); + diagnoseTypeAvailability(T, loc, where, flags); } bool swift::diagnoseConformanceAvailability(SourceLoc loc, ProtocolConformanceRef conformance, - const DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind) { + ExportContext where) { if (!conformance.isConcrete()) return false; const ProtocolConformance *concreteConf = conformance.getConcrete(); + auto *DC = where.getDeclContext(); SubstitutionMap subConformanceSubs = concreteConf->getSubstitutions(DC->getParentModule()); - diagnoseSubstitutionMapAvailability(loc, subConformanceSubs, DC, - reason, fragileKind); - + diagnoseSubstitutionMapAvailability(loc, subConformanceSubs, where); const RootProtocolConformance *rootConf = concreteConf->getRootConformance(); return TypeChecker::diagnoseConformanceExportability( - loc, rootConf, *DC->getParentSourceFile(), DC, - reason, fragileKind); + loc, rootConf, where); } bool swift::diagnoseSubstitutionMapAvailability(SourceLoc loc, SubstitutionMap subs, - const DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind) { + ExportContext where) { bool hadAnyIssues = false; for (ProtocolConformanceRef conformance : subs.getConformances()) { - if (diagnoseConformanceAvailability(loc, conformance, DC, - reason, fragileKind)) + if (diagnoseConformanceAvailability(loc, conformance, where)) hadAnyIssues = true; } return hadAnyIssues; @@ -3131,12 +3195,10 @@ swift::diagnoseSubstitutionMapAvailability(SourceLoc loc, /// TypeReprs. bool swift::diagnoseDeclAvailability(const ValueDecl *Decl, SourceRange R, - DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind, + ExportContext Where, DeclAvailabilityFlags Flags) { - AvailabilityWalker AW(DC, reason, fragileKind); + AvailabilityWalker AW(Where); return AW.diagAvailability(const_cast(Decl), R, nullptr, Flags); } diff --git a/lib/Sema/TypeCheckAvailability.h b/lib/Sema/TypeCheckAvailability.h index 40c500bde957b..3a7747a866073 100644 --- a/lib/Sema/TypeCheckAvailability.h +++ b/lib/Sema/TypeCheckAvailability.h @@ -13,6 +13,7 @@ #ifndef SWIFT_SEMA_TYPE_CHECK_AVAILABILITY_H #define SWIFT_SEMA_TYPE_CHECK_AVAILABILITY_H +#include "swift/AST/DeclContext.h" #include "swift/AST/AttrKind.h" #include "swift/AST/Identifier.h" #include "swift/Basic/LLVM.h" @@ -24,18 +25,15 @@ namespace swift { class ApplyExpr; class AvailableAttr; - class DeclContext; class Expr; class InFlightDiagnostic; class Decl; - struct FragileFunctionKind; class ProtocolConformanceRef; class Stmt; class SubstitutionMap; class Type; class TypeRepr; class ValueDecl; - enum class DeclAvailabilityFlag : uint8_t { /// Do not diagnose uses of protocols in versions before they were introduced. /// Used when type-checking protocol conformances, since conforming to a @@ -70,6 +68,38 @@ enum class ExportabilityReason : unsigned { ExtensionWithConditionalConformances }; +class ExportContext { + DeclContext *DC; + FragileFunctionKind FragileKind; + unsigned SPI : 1; + unsigned Exported : 1; + ExportabilityReason Reason; + + ExportContext(DeclContext *DC, FragileFunctionKind kind, bool spi, bool exported); + +public: + static ExportContext forDeclSignature(Decl *D); + static ExportContext forFunctionBody(DeclContext *DC); + + ExportContext forReason(ExportabilityReason reason) const; + ExportContext forExported(bool exported) const; + + DeclContext *getDeclContext() const { return DC; } + FragileFunctionKind getFragileFunctionKind() const { return FragileKind; } + + bool isSPI() const { return SPI; } + bool isExported() const { return Exported; } + + bool mustOnlyReferenceExportedDecls() const; + + Optional getExportabilityReason() const; +}; + +/// Check if a public declaration is part of a module's API; that is, this +/// will return false if the declaration is @_spi or @_implementationOnly. +bool isExported(const ValueDecl *VD); +bool isExported(const Decl *D); + /// Diagnose uses of unavailable declarations in expressions. void diagAvailability(const Expr *E, DeclContext *DC); @@ -78,47 +108,37 @@ void diagAvailability(const Expr *E, DeclContext *DC); void diagAvailability(const Stmt *S, DeclContext *DC); /// Diagnose uses of unavailable declarations in types. -bool diagnoseTypeReprAvailability(const TypeRepr *T, DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind, +bool diagnoseTypeReprAvailability(const TypeRepr *T, + ExportContext context, DeclAvailabilityFlags flags = None); /// Diagnose uses of unavailable conformances in types. -void diagnoseTypeAvailability(Type T, SourceLoc loc, DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind, +void diagnoseTypeAvailability(Type T, SourceLoc loc, + ExportContext context, DeclAvailabilityFlags flags = None); /// Checks both a TypeRepr and a Type, but avoids emitting duplicate /// diagnostics by only checking the Type if the TypeRepr succeeded. void diagnoseTypeAvailability(const TypeRepr *TR, Type T, SourceLoc loc, - DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind, + ExportContext context, DeclAvailabilityFlags flags = None); bool diagnoseConformanceAvailability(SourceLoc loc, ProtocolConformanceRef conformance, - const DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind); + ExportContext context); bool diagnoseSubstitutionMapAvailability(SourceLoc loc, SubstitutionMap subs, - const DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind); + ExportContext context); /// Run the Availability-diagnostics algorithm otherwise used in an expr /// context, but for non-expr contexts such as TypeDecls referenced from /// TypeReprs. bool diagnoseDeclAvailability(const ValueDecl *Decl, SourceRange R, - DeclContext *DC, - Optional reason, - FragileFunctionKind fragileKind, + ExportContext context, DeclAvailabilityFlags flags = None); void diagnoseUnavailableOverride(ValueDecl *override, diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index c6ffc17b2b1f5..f9e6edd02bbd6 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -1686,7 +1686,7 @@ static bool checkSuperInit(ConstructorDecl *fromCtor, for (auto decl : lookupResults) { auto superclassCtor = dyn_cast(decl); - if (!superclassCtor || !superclassCtor->isDesignatedInit() || + if (!superclassCtor || !superclassCtor->isDesignatedInit() || superclassCtor == ctor) continue; @@ -1696,12 +1696,9 @@ static bool checkSuperInit(ConstructorDecl *fromCtor, } // Make sure we can reference the designated initializer correctly. - auto fragileKind = fromCtor->getFragileFunctionKind(); - if (fragileKind.kind != FragileFunctionKind::None) { - TypeChecker::diagnoseInlinableDeclRef( - fromCtor->getLoc(), ctor, fromCtor, - fragileKind); - } + TypeChecker::diagnoseInlinableDeclRef( + fromCtor->getLoc(), ctor, + ExportContext::forFunctionBody(fromCtor)); } diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 146a47780cf85..551f382d256e1 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -37,6 +37,7 @@ namespace swift { +class ExportContext; class GenericSignatureBuilder; class NominalTypeDecl; class NormalProtocolConformance; @@ -947,33 +948,26 @@ DeclName getObjectLiteralConstructorName(ASTContext &ctx, ModuleDecl *getStdlibModule(const DeclContext *dc); /// \name Resilience diagnostics -bool diagnoseInlinableDeclRef(SourceLoc loc, const ValueDecl *D, - const DeclContext *DC, FragileFunctionKind Kind); +bool diagnoseInlinableDeclRef(SourceLoc loc, const ValueDecl *D, ExportContext where); Expr *buildDefaultInitializer(Type type); bool diagnoseInlinableDeclRefAccess(SourceLoc loc, const ValueDecl *D, - const DeclContext *DC, - FragileFunctionKind Kind); + ExportContext where); /// Given that a declaration is used from a particular context which /// exposes it in the interface of the current module, diagnose if it cannot /// reasonably be shared. bool diagnoseDeclRefExportability(SourceLoc loc, const ValueDecl *D, - const DeclContext *DC, - Optional exportability, - FragileFunctionKind fragileKind); + ExportContext where); /// Given that a conformance is used from a particular context which /// exposes it in the interface of the current module, diagnose if the /// conformance is SPI or visible via an implementation-only import. bool diagnoseConformanceExportability(SourceLoc loc, const RootProtocolConformance *rootConf, - const SourceFile &userSF, - const DeclContext *userDC, - Optional reason, - FragileFunctionKind fragileKind); + ExportContext where); /// \name Availability checking /// From a824e5a33b63a9b2ef7ed49e78993cd79baec968 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 20 Oct 2020 00:16:40 -0400 Subject: [PATCH 9/9] Sema: Refactor getDisallowedOriginKind() to take an ExportContext Converting the innermost DeclContext into a Decl won't work if the innermost DeclContext is the parent context of a VarDecl. Instead, use the relevant bit of state from the ExportContext, which is computed correctly. This fixes a regression caused by an earlier commit in this PR which our test suite did not catch. --- lib/Sema/ResilienceDiagnostics.cpp | 10 ++-------- lib/Sema/TypeCheckAccess.cpp | 17 ++++++++--------- lib/Sema/TypeCheckAccess.h | 7 +++---- lib/Sema/TypeCheckAvailability.cpp | 12 ++++++++++++ lib/Sema/TypeCheckProtocol.cpp | 4 +++- test/SPI/spi_members.swift | 11 +++++++++++ 6 files changed, 39 insertions(+), 22 deletions(-) create mode 100644 test/SPI/spi_members.swift diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp index 17e7e821c6f94..38e08afd1a9ca 100644 --- a/lib/Sema/ResilienceDiagnostics.cpp +++ b/lib/Sema/ResilienceDiagnostics.cpp @@ -152,12 +152,8 @@ TypeChecker::diagnoseDeclRefExportability(SourceLoc loc, auto downgradeToWarning = DowngradeToWarning::No; - auto *DC = where.getDeclContext(); auto originKind = getDisallowedOriginKind( - D, - *DC->getParentSourceFile(), - DC->getInnermostDeclarationDeclContext(), - downgradeToWarning); + D, where, downgradeToWarning); if (originKind == DisallowedOriginKind::None) return false; @@ -195,11 +191,9 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc, if (!where.mustOnlyReferenceExportedDecls()) return false; - auto *DC = where.getDeclContext(); auto originKind = getDisallowedOriginKind( rootConf->getDeclContext()->getAsDecl(), - *DC->getParentSourceFile(), - DC->getInnermostDeclarationDeclContext()); + where); if (originKind == DisallowedOriginKind::None) return false; diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index c31bafb8a922a..447099f037185 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -1460,22 +1460,22 @@ class UsableFromInlineChecker : public AccessControlCheckerBase, /// Local variant to swift::getDisallowedOriginKind for downgrade to warnings. DisallowedOriginKind swift::getDisallowedOriginKind(const Decl *decl, - const SourceFile &userSF, - const Decl *userContext, + ExportContext where, DowngradeToWarning &downgradeToWarning) { downgradeToWarning = DowngradeToWarning::No; ModuleDecl *M = decl->getModuleContext(); - if (userSF.isImportedImplementationOnly(M)) { + auto *SF = where.getDeclContext()->getParentSourceFile(); + if (SF->isImportedImplementationOnly(M)) { // Temporarily downgrade implementation-only exportability in SPI to // a warning. - if (userContext->isSPI()) + if (where.isSPI()) downgradeToWarning = DowngradeToWarning::Yes; // Implementation-only imported, cannot be reexported. return DisallowedOriginKind::ImplementationOnly; - } else if (decl->isSPI() && !userContext->isSPI()) { + } else if (decl->isSPI() && !where.isSPI()) { // SPI can only be exported in SPI. - return userContext->getModuleContext() == M ? + return where.getDeclContext()->getParentModule() == M ? DisallowedOriginKind::SPILocal : DisallowedOriginKind::SPIImported; } @@ -1842,10 +1842,9 @@ static void checkExtensionGenericParamAccess(const ExtensionDecl *ED) { } DisallowedOriginKind swift::getDisallowedOriginKind(const Decl *decl, - const SourceFile &userSF, - const Decl *declContext) { + ExportContext where) { auto downgradeToWarning = DowngradeToWarning::No; - return getDisallowedOriginKind(decl, userSF, declContext, downgradeToWarning); + return getDisallowedOriginKind(decl, where, downgradeToWarning); } void swift::checkAccessControl(Decl *D) { diff --git a/lib/Sema/TypeCheckAccess.h b/lib/Sema/TypeCheckAccess.h index 764603a85ac09..29d82411bba5c 100644 --- a/lib/Sema/TypeCheckAccess.h +++ b/lib/Sema/TypeCheckAccess.h @@ -22,6 +22,7 @@ namespace swift { class Decl; +class ExportContext; class SourceFile; /// Performs access-related checks for \p D. @@ -55,12 +56,10 @@ enum class DowngradeToWarning: bool { /// Returns the kind of origin, implementation-only import or SPI declaration, /// that restricts exporting \p decl from the given file and context. DisallowedOriginKind getDisallowedOriginKind(const Decl *decl, - const SourceFile &userSF, - const Decl *userContext); + ExportContext where); DisallowedOriginKind getDisallowedOriginKind(const Decl *decl, - const SourceFile &userSF, - const Decl *userContext, + ExportContext where, DowngradeToWarning &downgradeToWarning); } // end namespace swift diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 692c4a06d8e7c..4fb4bbe298b64 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -98,7 +98,19 @@ bool swift::isExported(const Decl *D) { ExportContext ExportContext::forDeclSignature(Decl *D) { auto *DC = D->getInnermostDeclContext(); auto fragileKind = DC->getFragileFunctionKind(); + bool spi = D->isSPI(); + if (auto *PBD = dyn_cast(D)) { + for (unsigned i = 0, e = PBD->getNumPatternEntries(); i < e; ++i) { + if (auto *VD = PBD->getAnchoringVarDecl(i)) { + if (VD->isSPI()) { + spi = true; + break; + } + } + } + } + bool exported = ::isExported(D); return ExportContext(DC, fragileKind, spi, exported); diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 48ff739eb8306..79bbcd82b8cb6 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -4337,9 +4337,11 @@ static void checkExportability(Type depTy, Type replacementTy, conformance->getRootConformance(); ModuleDecl *M = rootConformance->getDeclContext()->getParentModule(); + auto where = ExportContext::forDeclSignature( + DC->getInnermostDeclarationDeclContext()); auto originKind = getDisallowedOriginKind( rootConformance->getDeclContext()->getAsDecl(), - *SF, DC->getAsDecl()); + where); if (originKind == DisallowedOriginKind::None) return; diff --git a/test/SPI/spi_members.swift b/test/SPI/spi_members.swift new file mode 100644 index 0000000000000..45c70d9c88c5a --- /dev/null +++ b/test/SPI/spi_members.swift @@ -0,0 +1,11 @@ +// RUN: %target-typecheck-verify-swift + +@_spi(Foo) +public class Bar {} + +public struct Foo { + public init() {} + + @_spi(Foo) public func method(_: Bar) {} + @_spi(Foo) public var property: Bar { Bar() } +}