Skip to content

Commit

Permalink
Merge pull request #23808 from jrose-apple/conformance-conformance
Browse files Browse the repository at this point in the history
Implementation-only import checking for conformances
  • Loading branch information
jrose-apple committed Apr 11, 2019
2 parents 3496078 + 7006aa0 commit 6ea773e
Show file tree
Hide file tree
Showing 14 changed files with 757 additions and 121 deletions.
12 changes: 6 additions & 6 deletions include/swift/AST/AccessScopeChecker.h
Expand Up @@ -54,13 +54,13 @@ class TypeDeclFinder : public TypeWalker {
Action walkToTypePre(Type T) override;

public:
virtual Action visitNominalType(const NominalType *ty) {
virtual Action visitNominalType(NominalType *ty) {
return Action::Continue;
}
virtual Action visitBoundGenericType(const BoundGenericType *ty) {
virtual Action visitBoundGenericType(BoundGenericType *ty) {
return Action::Continue;
}
virtual Action visitTypeAliasType(const TypeAliasType *ty) {
virtual Action visitTypeAliasType(TypeAliasType *ty) {
return Action::Continue;
}
};
Expand All @@ -72,9 +72,9 @@ class SimpleTypeDeclFinder : public TypeDeclFinder {
/// The function to call when a ComponentIdentTypeRepr is seen.
llvm::function_ref<Action(const TypeDecl *)> Callback;

Action visitNominalType(const NominalType *ty) override;
Action visitBoundGenericType(const BoundGenericType *ty) override;
Action visitTypeAliasType(const TypeAliasType *ty) override;
Action visitNominalType(NominalType *ty) override;
Action visitBoundGenericType(BoundGenericType *ty) override;
Action visitTypeAliasType(TypeAliasType *ty) override;

public:
explicit SimpleTypeDeclFinder(
Expand Down
8 changes: 8 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Expand Up @@ -2391,6 +2391,14 @@ ERROR(decl_from_implementation_only_module,none,
"cannot use %0 here; %1 has been imported as "
"'@_implementationOnly'",
(DeclName, Identifier))
ERROR(conformance_from_implementation_only_module,none,
"cannot use conformance of %0 to %1 here; %2 has been imported as "
"'@_implementationOnly'",
(Type, DeclName, Identifier))
ERROR(assoc_conformance_from_implementation_only_module,none,
"cannot use conformance of %0 to %1 in associated type %3 (inferred as "
"%4); %2 has been imported as '@_implementationOnly'",
(Type, DeclName, Identifier, Type, Type))

// Derived conformances
ERROR(cannot_synthesize_init_in_extension_of_nonfinal,none,
Expand Down
6 changes: 3 additions & 3 deletions lib/AST/AccessScopeChecker.cpp
Expand Up @@ -71,17 +71,17 @@ TypeWalker::Action TypeDeclFinder::walkToTypePre(Type T) {
}

TypeWalker::Action
SimpleTypeDeclFinder::visitNominalType(const NominalType *ty) {
SimpleTypeDeclFinder::visitNominalType(NominalType *ty) {
return Callback(ty->getDecl());
}

TypeWalker::Action
SimpleTypeDeclFinder::visitBoundGenericType(const BoundGenericType *ty) {
SimpleTypeDeclFinder::visitBoundGenericType(BoundGenericType *ty) {
return Callback(ty->getDecl());
}

TypeWalker::Action
SimpleTypeDeclFinder::visitTypeAliasType(const TypeAliasType *ty) {
SimpleTypeDeclFinder::visitTypeAliasType(TypeAliasType *ty) {
return Callback(ty->getDecl());
}

Expand Down
127 changes: 101 additions & 26 deletions lib/Sema/ResilienceDiagnostics.cpp
Expand Up @@ -15,10 +15,13 @@
//===----------------------------------------------------------------------===//

#include "TypeChecker.h"
#include "TypeCheckAvailability.h"
#include "swift/AST/AccessScopeChecker.h"
#include "swift/AST/Attr.h"
#include "swift/AST/Decl.h"
#include "swift/AST/Initializer.h"
#include "swift/AST/DeclContext.h"
#include "swift/AST/Initializer.h"
#include "swift/AST/ProtocolConformance.h"

using namespace swift;
using FragileFunctionKind = TypeChecker::FragileFunctionKind;
Expand Down Expand Up @@ -99,16 +102,13 @@ enum class DowngradeToWarning: bool {
};

bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
const ValueDecl *D,
ConcreteDeclRef declRef,
const DeclContext *DC,
FragileFunctionKind Kind,
bool TreatUsableFromInlineAsPublic) {
const ValueDecl *D = declRef.getDecl();
// Do some important fast-path checks that apply to all cases.

// Local declarations are OK.
if (D->getDeclContext()->isLocalContext())
return false;

// Type parameters are OK.
if (isa<AbstractTypeParamDecl>(D))
return false;
Expand All @@ -119,7 +119,7 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
return true;

// Check whether the declaration comes from a publically-imported module.
if (diagnoseDeclRefExportability(loc, D, DC))
if (diagnoseDeclRefExportability(loc, declRef, DC))
return true;

return false;
Expand All @@ -130,6 +130,10 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
const DeclContext *DC,
FragileFunctionKind Kind,
bool TreatUsableFromInlineAsPublic) {
// Local declarations are OK.
if (D->getDeclContext()->isLocalContext())
return false;

// Public declarations are OK.
if (D->getFormalAccessScope(/*useDC=*/nullptr,
TreatUsableFromInlineAsPublic).isPublic())
Expand Down Expand Up @@ -203,8 +207,89 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
return (downgradeToWarning == DowngradeToWarning::No);
}

static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D,
const SourceFile &userSF) {
auto definingModule = D->getModuleContext();
if (!userSF.isImportedImplementationOnly(definingModule))
return false;

// TODO: different diagnostics
ASTContext &ctx = definingModule->getASTContext();
ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_implementation_only,
D->getDescriptiveKind(), D->getFullName());
return true;
}

static bool
diagnoseGenericArgumentsExportability(SourceLoc loc,
const SubstitutionMap &subs,
const SourceFile &userSF) {
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);

const RootProtocolConformance *rootConf =
concreteConf->getRootConformance();
ModuleDecl *M = rootConf->getDeclContext()->getParentModule();
if (!userSF.isImportedImplementationOnly(M))
continue;

ASTContext &ctx = M->getASTContext();
ctx.Diags.diagnose(loc, diag::conformance_from_implementation_only_module,
rootConf->getType(),
rootConf->getProtocol()->getFullName(), M->getName());
hadAnyIssues = true;
}
return hadAnyIssues;
}

void TypeChecker::diagnoseGenericTypeExportability(const TypeLoc &TL,
const DeclContext *DC) {
class GenericTypeFinder : public TypeDeclFinder {
using Callback = llvm::function_ref<void(SubstitutionMap)>;

const SourceFile &SF;
Callback callback;
public:
GenericTypeFinder(const SourceFile &SF, Callback callback)
: SF(SF), callback(callback) {}

Action visitBoundGenericType(BoundGenericType *ty) override {
ModuleDecl *useModule = SF.getParentModule();
SubstitutionMap subs = ty->getContextSubstitutionMap(useModule,
ty->getDecl());
callback(subs);
return Action::Continue;
}

Action visitTypeAliasType(TypeAliasType *ty) override {
callback(ty->getSubstitutionMap());
return Action::Continue;
}
};

assert(TL.getType() && "type not validated yet");

const SourceFile *SF = DC->getParentSourceFile();
if (!SF)
return;

TL.getType().walk(GenericTypeFinder(*SF, [&](SubstitutionMap subs) {
// 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.
(void)diagnoseGenericArgumentsExportability(TL.getLoc(), subs, *SF);
}));
}

bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
const ValueDecl *D,
ConcreteDeclRef declRef,
const DeclContext *DC) {
// We're only interested in diagnosing uses from source files.
auto userSF = DC->getParentSourceFile();
Expand All @@ -219,22 +304,12 @@ bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
if (!userSF->hasImplementationOnlyImports())
return false;

auto userModule = userSF->getParentModule();
auto definingModule = D->getModuleContext();

// Nothing to diagnose in the very common case of the same module.
if (userModule == definingModule)
return false;

// Nothing to diagnose in the very common case that the module is
// imported for use in signatures.
if (!userSF->isImportedImplementationOnly(definingModule))
return false;

// TODO: different diagnostics
diagnose(loc, diag::inlinable_decl_ref_implementation_only,
D->getDescriptiveKind(), D->getFullName());

// TODO: notes explaining why
return true;
const ValueDecl *D = declRef.getDecl();
if (diagnoseDeclExportability(loc, D, *userSF))
return true;
if (diagnoseGenericArgumentsExportability(loc, declRef.getSubstitutions(),
*userSF)) {
return true;
}
return false;
}

0 comments on commit 6ea773e

Please sign in to comment.