Skip to content

Commit 7006aa0

Browse files
committed
Check for use of implementation-only conformances in inlinable code
This includes both the types and the values (generic functions, etc) used in the inlinable code. We get some effectively duplicate diagnostics at this point because of this, but we can deal with that at a future date. Last part of rdar://problem/48991061
1 parent 5905517 commit 7006aa0

File tree

5 files changed

+392
-27
lines changed

5 files changed

+392
-27
lines changed

lib/Sema/ResilienceDiagnostics.cpp

Lines changed: 99 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@
1515
//===----------------------------------------------------------------------===//
1616

1717
#include "TypeChecker.h"
18+
#include "TypeCheckAvailability.h"
19+
#include "swift/AST/AccessScopeChecker.h"
1820
#include "swift/AST/Attr.h"
1921
#include "swift/AST/Decl.h"
20-
#include "swift/AST/Initializer.h"
2122
#include "swift/AST/DeclContext.h"
23+
#include "swift/AST/Initializer.h"
24+
#include "swift/AST/ProtocolConformance.h"
2225

2326
using namespace swift;
2427
using FragileFunctionKind = TypeChecker::FragileFunctionKind;
@@ -106,10 +109,6 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
106109
const ValueDecl *D = declRef.getDecl();
107110
// Do some important fast-path checks that apply to all cases.
108111

109-
// Local declarations are OK.
110-
if (D->getDeclContext()->isLocalContext())
111-
return false;
112-
113112
// Type parameters are OK.
114113
if (isa<AbstractTypeParamDecl>(D))
115114
return false;
@@ -120,7 +119,7 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
120119
return true;
121120

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

126125
return false;
@@ -131,6 +130,10 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
131130
const DeclContext *DC,
132131
FragileFunctionKind Kind,
133132
bool TreatUsableFromInlineAsPublic) {
133+
// Local declarations are OK.
134+
if (D->getDeclContext()->isLocalContext())
135+
return false;
136+
134137
// Public declarations are OK.
135138
if (D->getFormalAccessScope(/*useDC=*/nullptr,
136139
TreatUsableFromInlineAsPublic).isPublic())
@@ -204,8 +207,89 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
204207
return (downgradeToWarning == DowngradeToWarning::No);
205208
}
206209

210+
static bool diagnoseDeclExportability(SourceLoc loc, const ValueDecl *D,
211+
const SourceFile &userSF) {
212+
auto definingModule = D->getModuleContext();
213+
if (!userSF.isImportedImplementationOnly(definingModule))
214+
return false;
215+
216+
// TODO: different diagnostics
217+
ASTContext &ctx = definingModule->getASTContext();
218+
ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_implementation_only,
219+
D->getDescriptiveKind(), D->getFullName());
220+
return true;
221+
}
222+
223+
static bool
224+
diagnoseGenericArgumentsExportability(SourceLoc loc,
225+
const SubstitutionMap &subs,
226+
const SourceFile &userSF) {
227+
bool hadAnyIssues = false;
228+
for (ProtocolConformanceRef conformance : subs.getConformances()) {
229+
if (!conformance.isConcrete())
230+
continue;
231+
const ProtocolConformance *concreteConf = conformance.getConcrete();
232+
233+
SubstitutionMap subConformanceSubs =
234+
concreteConf->getSubstitutions(userSF.getParentModule());
235+
diagnoseGenericArgumentsExportability(loc, subConformanceSubs, userSF);
236+
237+
const RootProtocolConformance *rootConf =
238+
concreteConf->getRootConformance();
239+
ModuleDecl *M = rootConf->getDeclContext()->getParentModule();
240+
if (!userSF.isImportedImplementationOnly(M))
241+
continue;
242+
243+
ASTContext &ctx = M->getASTContext();
244+
ctx.Diags.diagnose(loc, diag::conformance_from_implementation_only_module,
245+
rootConf->getType(),
246+
rootConf->getProtocol()->getFullName(), M->getName());
247+
hadAnyIssues = true;
248+
}
249+
return hadAnyIssues;
250+
}
251+
252+
void TypeChecker::diagnoseGenericTypeExportability(const TypeLoc &TL,
253+
const DeclContext *DC) {
254+
class GenericTypeFinder : public TypeDeclFinder {
255+
using Callback = llvm::function_ref<void(SubstitutionMap)>;
256+
257+
const SourceFile &SF;
258+
Callback callback;
259+
public:
260+
GenericTypeFinder(const SourceFile &SF, Callback callback)
261+
: SF(SF), callback(callback) {}
262+
263+
Action visitBoundGenericType(BoundGenericType *ty) override {
264+
ModuleDecl *useModule = SF.getParentModule();
265+
SubstitutionMap subs = ty->getContextSubstitutionMap(useModule,
266+
ty->getDecl());
267+
callback(subs);
268+
return Action::Continue;
269+
}
270+
271+
Action visitTypeAliasType(TypeAliasType *ty) override {
272+
callback(ty->getSubstitutionMap());
273+
return Action::Continue;
274+
}
275+
};
276+
277+
assert(TL.getType() && "type not validated yet");
278+
279+
const SourceFile *SF = DC->getParentSourceFile();
280+
if (!SF)
281+
return;
282+
283+
TL.getType().walk(GenericTypeFinder(*SF, [&](SubstitutionMap subs) {
284+
// FIXME: It would be nice to highlight just the part of the type that's
285+
// problematic, but unfortunately the TypeRepr doesn't have the
286+
// information we need and the Type doesn't easily map back to it.
287+
(void)diagnoseGenericArgumentsExportability(TL.getLoc(), subs, *SF);
288+
}));
289+
}
290+
207291
bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
208-
const ValueDecl *D,
292+
ConcreteDeclRef declRef,
209293
const DeclContext *DC) {
210294
// We're only interested in diagnosing uses from source files.
211295
auto userSF = DC->getParentSourceFile();
@@ -220,22 +304,12 @@ bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
220304
if (!userSF->hasImplementationOnlyImports())
221305
return false;
222306

223-
auto userModule = userSF->getParentModule();
224-
auto definingModule = D->getModuleContext();
225-
226-
// Nothing to diagnose in the very common case of the same module.
227-
if (userModule == definingModule)
228-
return false;
229-
230-
// Nothing to diagnose in the very common case that the module is
231-
// imported for use in signatures.
232-
if (!userSF->isImportedImplementationOnly(definingModule))
233-
return false;
234-
235-
// TODO: different diagnostics
236-
diagnose(loc, diag::inlinable_decl_ref_implementation_only,
237-
D->getDescriptiveKind(), D->getFullName());
238-
239-
// TODO: notes explaining why
240-
return true;
307+
const ValueDecl *D = declRef.getDecl();
308+
if (diagnoseDeclExportability(loc, D, *userSF))
309+
return true;
310+
if (diagnoseGenericArgumentsExportability(loc, declRef.getSubstitutions(),
311+
*userSF)) {
312+
return true;
313+
}
314+
return false;
241315
}

lib/Sema/TypeCheckType.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,6 +1742,13 @@ bool TypeChecker::validateType(TypeLoc &Loc, TypeResolution resolution,
17421742
}
17431743

17441744
Loc.setType(type);
1745+
if (!type->hasError()) {
1746+
const DeclContext *DC = resolution.getDeclContext();
1747+
if (options.isAnyExpr() || DC->getParent()->isLocalContext())
1748+
if (DC->getResilienceExpansion() == ResilienceExpansion::Minimal)
1749+
diagnoseGenericTypeExportability(Loc, DC);
1750+
}
1751+
17451752
return type->hasError();
17461753
}
17471754

lib/Sema/TypeChecker.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1887,13 +1887,23 @@ class TypeChecker final : public LazyResolver {
18871887
FragileFunctionKind Kind,
18881888
bool TreatUsableFromInlineAsPublic);
18891889

1890-
public:
18911890
/// Given that a declaration is used from a particular context which
18921891
/// exposes it in the interface of the current module, diagnose if it cannot
18931892
/// reasonably be shared.
1894-
bool diagnoseDeclRefExportability(SourceLoc loc, const ValueDecl *D,
1893+
bool diagnoseDeclRefExportability(SourceLoc loc, ConcreteDeclRef declRef,
18951894
const DeclContext *DC);
18961895

1896+
public:
1897+
/// Given that a type is used from a particular context which
1898+
/// exposes it in the interface of the current module, diagnose if its
1899+
/// generic arguments require the use of conformances that cannot reasonably
1900+
/// be shared.
1901+
///
1902+
/// This method \e only checks how generic arguments are used; it is assumed
1903+
/// that the declarations involved have already been checked elsewhere.
1904+
void diagnoseGenericTypeExportability(const TypeLoc &TL,
1905+
const DeclContext *DC);
1906+
18971907
/// Given that \p DC is within a fragile context for some reason, describe
18981908
/// why.
18991909
///

test/Sema/implementation-only-import-in-decls.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public protocol TestAssocTypeWhereClause {
6464

6565
public enum TestRawType: IntLike { // expected-error {{cannot use 'IntLike' here; 'BADLibrary' has been imported as '@_implementationOnly'}}
6666
case x = 1
67+
// FIXME: expected-error@-1 {{cannot use conformance of 'IntLike' to 'Equatable' here; 'BADLibrary' has been imported as '@_implementationOnly'}}
6768
}
6869

6970
public class TestSubclass: BadClass { // expected-error {{cannot use 'BadClass' here; 'BADLibrary' has been imported as '@_implementationOnly'}}
@@ -245,3 +246,13 @@ public struct RequirementsHandleSpecializationsToo: SlightlyMoreComplicatedRequi
245246
public struct ClassConstrainedGenericArg<T: NormalClass>: PublicInferredAssociatedType { // expected-error {{cannot use conformance of 'NormalClass' to 'NormalProto' in associated type 'Self.Assoc' (inferred as 'T'); 'BADLibrary' has been imported as '@_implementationOnly'}}
246247
public func takesAssoc(_: T) {}
247248
}
249+
250+
251+
public protocol RecursiveRequirements {
252+
associatedtype Other: RecursiveRequirements
253+
}
254+
extension GenericStruct: RecursiveRequirements {
255+
public typealias Other = GenericStruct<T>
256+
}
257+
public struct RecursiveRequirementsHolder<T: RecursiveRequirements> {}
258+
public func makeSureRecursiveRequirementsDontBreakEverything(_: RecursiveRequirementsHolder<GenericStruct<Int>>) {}

0 commit comments

Comments
 (0)