Skip to content

Commit cc3b6a0

Browse files
rjmccallrjmccall
authored
Merge pull request swiftlang#29587 from rjmccall/export-check-diagnoser
Remove some unnecessary generalization in exportability checking
2 parents 2c018ce + 47cd59b commit cc3b6a0

File tree

1 file changed

+39
-77
lines changed

1 file changed

+39
-77
lines changed

‎lib/Sema/TypeCheckAccess.cpp‎

Lines changed: 39 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,15 +1445,11 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
14451445
};
14461446

14471447
class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
1448-
using CheckExportabilityTypeCallback =
1449-
llvm::function_ref<void(const TypeDecl *, const TypeRepr *)>;
1450-
using CheckExportabilityConformanceCallback =
1451-
llvm::function_ref<void(const ProtocolConformance *)>;
1448+
class Diagnoser;
14521449

14531450
void checkTypeImpl(
14541451
Type type, const TypeRepr *typeRepr, const SourceFile &SF,
1455-
CheckExportabilityTypeCallback diagnoseType,
1456-
CheckExportabilityConformanceCallback diagnoseConformance) {
1452+
const Diagnoser &diagnoser) {
14571453
// Don't bother checking errors.
14581454
if (type && type->hasError())
14591455
return;
@@ -1469,7 +1465,7 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
14691465
if (!SF.isImportedImplementationOnly(M))
14701466
return true;
14711467

1472-
diagnoseType(component->getBoundDecl(), component);
1468+
diagnoser.diagnoseType(component->getBoundDecl(), component);
14731469
foundAnyIssues = true;
14741470
// We still continue even in the diagnostic case to report multiple
14751471
// violations.
@@ -1488,22 +1484,17 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
14881484

14891485
class ProblematicTypeFinder : public TypeDeclFinder {
14901486
const SourceFile &SF;
1491-
CheckExportabilityTypeCallback diagnoseType;
1492-
CheckExportabilityConformanceCallback diagnoseConformance;
1487+
const Diagnoser &diagnoser;
14931488
public:
1494-
ProblematicTypeFinder(
1495-
const SourceFile &SF,
1496-
CheckExportabilityTypeCallback diagnoseType,
1497-
CheckExportabilityConformanceCallback diagnoseConformance)
1498-
: SF(SF), diagnoseType(diagnoseType),
1499-
diagnoseConformance(diagnoseConformance) {}
1489+
ProblematicTypeFinder(const SourceFile &SF, const Diagnoser &diagnoser)
1490+
: SF(SF), diagnoser(diagnoser) {}
15001491

15011492
void visitTypeDecl(const TypeDecl *typeDecl) {
15021493
ModuleDecl *M = typeDecl->getModuleContext();
15031494
if (!SF.isImportedImplementationOnly(M))
15041495
return;
15051496

1506-
diagnoseType(typeDecl, /*typeRepr*/nullptr);
1497+
diagnoser.diagnoseType(typeDecl, /*typeRepr*/nullptr);
15071498
}
15081499

15091500
void visitSubstitutionMap(SubstitutionMap subs) {
@@ -1521,7 +1512,7 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
15211512
ModuleDecl *M = rootConf->getDeclContext()->getParentModule();
15221513
if (!SF.isImportedImplementationOnly(M))
15231514
continue;
1524-
diagnoseConformance(rootConf);
1515+
diagnoser.diagnoseConformance(rootConf);
15251516
}
15261517
}
15271518

@@ -1545,25 +1536,20 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
15451536
}
15461537
};
15471538

1548-
type.walk(ProblematicTypeFinder(SF, diagnoseType, diagnoseConformance));
1539+
type.walk(ProblematicTypeFinder(SF, diagnoser));
15491540
}
15501541

15511542
void checkType(
15521543
Type type, const TypeRepr *typeRepr, const Decl *context,
1553-
CheckExportabilityTypeCallback diagnoseType,
1554-
CheckExportabilityConformanceCallback diagnoseConformance) {
1544+
const Diagnoser &diagnoser) {
15551545
auto *SF = context->getDeclContext()->getParentSourceFile();
15561546
assert(SF && "checking a non-source declaration?");
1557-
return checkTypeImpl(type, typeRepr, *SF, diagnoseType,
1558-
diagnoseConformance);
1547+
return checkTypeImpl(type, typeRepr, *SF, diagnoser);
15591548
}
15601549

15611550
void checkType(
1562-
const TypeLoc &TL, const Decl *context,
1563-
CheckExportabilityTypeCallback diagnoseType,
1564-
CheckExportabilityConformanceCallback diagnoseConformance) {
1565-
checkType(TL.getType(), TL.getTypeRepr(), context, diagnoseType,
1566-
diagnoseConformance);
1551+
const TypeLoc &TL, const Decl *context, const Diagnoser &diagnoser) {
1552+
checkType(TL.getType(), TL.getTypeRepr(), context, diagnoser);
15671553
}
15681554

15691555
void checkGenericParams(const GenericContext *ownerCtx,
@@ -1577,15 +1563,13 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
15771563
continue;
15781564
assert(param->getInherited().size() == 1);
15791565
checkType(param->getInherited().front(), ownerDecl,
1580-
getDiagnoseCallback(ownerDecl),
1581-
getDiagnoseCallback(ownerDecl));
1566+
getDiagnoser(ownerDecl));
15821567
}
15831568

15841569
forAllRequirementTypes(WhereClauseOwner(
15851570
const_cast<GenericContext *>(ownerCtx)),
15861571
[&](Type type, TypeRepr *typeRepr) {
1587-
checkType(type, typeRepr, ownerDecl, getDiagnoseCallback(ownerDecl),
1588-
getDiagnoseCallback(ownerDecl));
1572+
checkType(type, typeRepr, ownerDecl, getDiagnoser(ownerDecl));
15891573
});
15901574
}
15911575

@@ -1598,14 +1582,14 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
15981582
ExtensionWithConditionalConformances
15991583
};
16001584

1601-
class DiagnoseGenerically {
1585+
class Diagnoser {
16021586
const Decl *D;
16031587
Reason reason;
16041588
public:
1605-
DiagnoseGenerically(const Decl *D, Reason reason) : D(D), reason(reason) {}
1589+
Diagnoser(const Decl *D, Reason reason) : D(D), reason(reason) {}
16061590

1607-
void operator()(const TypeDecl *offendingType,
1608-
const TypeRepr *complainRepr) {
1591+
void diagnoseType(const TypeDecl *offendingType,
1592+
const TypeRepr *complainRepr) const {
16091593
ModuleDecl *M = offendingType->getModuleContext();
16101594
auto diag = D->diagnose(diag::decl_from_implementation_only_module,
16111595
offendingType->getDescriptiveKind(),
@@ -1614,7 +1598,7 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
16141598
highlightOffendingType(diag, complainRepr);
16151599
}
16161600

1617-
void operator()(const ProtocolConformance *offendingConformance) {
1601+
void diagnoseConformance(const ProtocolConformance *offendingConformance) const {
16181602
ModuleDecl *M = offendingConformance->getDeclContext()->getParentModule();
16191603
D->diagnose(diag::conformance_from_implementation_only_module,
16201604
offendingConformance->getType(),
@@ -1623,18 +1607,8 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
16231607
}
16241608
};
16251609

1626-
static_assert(
1627-
std::is_convertible<DiagnoseGenerically,
1628-
CheckExportabilityTypeCallback>::value,
1629-
"DiagnoseGenerically has wrong call signature");
1630-
static_assert(
1631-
std::is_convertible<DiagnoseGenerically,
1632-
CheckExportabilityConformanceCallback>::value,
1633-
"DiagnoseGenerically has wrong call signature for conformance diags");
1634-
1635-
DiagnoseGenerically getDiagnoseCallback(const Decl *D,
1636-
Reason reason = Reason::General) {
1637-
return DiagnoseGenerically(D, reason);
1610+
Diagnoser getDiagnoser(const Decl *D, Reason reason = Reason::General) {
1611+
return Diagnoser(D, reason);
16381612
}
16391613

16401614
public:
@@ -1768,7 +1742,7 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
17681742
return;
17691743

17701744
checkType(theVar->getInterfaceType(), /*typeRepr*/nullptr, theVar,
1771-
getDiagnoseCallback(theVar), getDiagnoseCallback(theVar));
1745+
getDiagnoser(theVar));
17721746
}
17731747

17741748
/// \see visitPatternBindingDecl
@@ -1787,8 +1761,7 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
17871761
if (shouldSkipChecking(anyVar))
17881762
return;
17891763

1790-
checkType(TP->getTypeLoc(), anyVar, getDiagnoseCallback(anyVar),
1791-
getDiagnoseCallback(anyVar));
1764+
checkType(TP->getTypeLoc(), anyVar, getDiagnoser(anyVar));
17921765
}
17931766

17941767
void visitPatternBindingDecl(PatternBindingDecl *PBD) {
@@ -1812,25 +1785,22 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
18121785
void visitTypeAliasDecl(TypeAliasDecl *TAD) {
18131786
checkGenericParams(TAD, TAD);
18141787
checkType(TAD->getUnderlyingType(),
1815-
TAD->getUnderlyingTypeRepr(), TAD, getDiagnoseCallback(TAD),
1816-
getDiagnoseCallback(TAD));
1788+
TAD->getUnderlyingTypeRepr(), TAD, getDiagnoser(TAD));
18171789
}
18181790

18191791
void visitAssociatedTypeDecl(AssociatedTypeDecl *assocType) {
18201792
llvm::for_each(assocType->getInherited(),
18211793
[&](TypeLoc requirement) {
1822-
checkType(requirement, assocType, getDiagnoseCallback(assocType),
1823-
getDiagnoseCallback(assocType));
1794+
checkType(requirement, assocType, getDiagnoser(assocType));
18241795
});
18251796
checkType(assocType->getDefaultDefinitionType(),
18261797
assocType->getDefaultDefinitionTypeRepr(), assocType,
1827-
getDiagnoseCallback(assocType), getDiagnoseCallback(assocType));
1798+
getDiagnoser(assocType));
18281799

18291800
if (assocType->getTrailingWhereClause()) {
18301801
forAllRequirementTypes(assocType,
18311802
[&](Type type, TypeRepr *typeRepr) {
1832-
checkType(type, typeRepr, assocType, getDiagnoseCallback(assocType),
1833-
getDiagnoseCallback(assocType));
1803+
checkType(type, typeRepr, assocType, getDiagnoser(assocType));
18341804
});
18351805
}
18361806
}
@@ -1840,22 +1810,19 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
18401810

18411811
llvm::for_each(nominal->getInherited(),
18421812
[&](TypeLoc nextInherited) {
1843-
checkType(nextInherited, nominal, getDiagnoseCallback(nominal),
1844-
getDiagnoseCallback(nominal));
1813+
checkType(nextInherited, nominal, getDiagnoser(nominal));
18451814
});
18461815
}
18471816

18481817
void visitProtocolDecl(ProtocolDecl *proto) {
18491818
llvm::for_each(proto->getInherited(),
18501819
[&](TypeLoc requirement) {
1851-
checkType(requirement, proto, getDiagnoseCallback(proto),
1852-
getDiagnoseCallback(proto));
1820+
checkType(requirement, proto, getDiagnoser(proto));
18531821
});
18541822

18551823
if (proto->getTrailingWhereClause()) {
18561824
forAllRequirementTypes(proto, [&](Type type, TypeRepr *typeRepr) {
1857-
checkType(type, typeRepr, proto, getDiagnoseCallback(proto),
1858-
getDiagnoseCallback(proto));
1825+
checkType(type, typeRepr, proto, getDiagnoser(proto));
18591826
});
18601827
}
18611828
}
@@ -1865,41 +1832,38 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
18651832

18661833
for (auto &P : *SD->getIndices()) {
18671834
checkType(P->getInterfaceType(), P->getTypeRepr(), SD,
1868-
getDiagnoseCallback(SD), getDiagnoseCallback(SD));
1835+
getDiagnoser(SD));
18691836
}
1870-
checkType(SD->getElementTypeLoc(), SD, getDiagnoseCallback(SD),
1871-
getDiagnoseCallback(SD));
1837+
checkType(SD->getElementTypeLoc(), SD, getDiagnoser(SD));
18721838
}
18731839

18741840
void visitAbstractFunctionDecl(AbstractFunctionDecl *fn) {
18751841
checkGenericParams(fn, fn);
18761842

18771843
for (auto *P : *fn->getParameters())
18781844
checkType(P->getInterfaceType(), P->getTypeRepr(), fn,
1879-
getDiagnoseCallback(fn), getDiagnoseCallback(fn));
1845+
getDiagnoser(fn));
18801846
}
18811847

18821848
void visitFuncDecl(FuncDecl *FD) {
18831849
visitAbstractFunctionDecl(FD);
1884-
checkType(FD->getBodyResultTypeLoc(), FD, getDiagnoseCallback(FD),
1885-
getDiagnoseCallback(FD));
1850+
checkType(FD->getBodyResultTypeLoc(), FD, getDiagnoser(FD));
18861851
}
18871852

18881853
void visitEnumElementDecl(EnumElementDecl *EED) {
18891854
if (!EED->hasAssociatedValues())
18901855
return;
18911856
for (auto &P : *EED->getParameterList())
18921857
checkType(P->getInterfaceType(), P->getTypeRepr(), EED,
1893-
getDiagnoseCallback(EED), getDiagnoseCallback(EED));
1858+
getDiagnoser(EED));
18941859
}
18951860

18961861
void checkConstrainedExtensionRequirements(ExtensionDecl *ED,
18971862
Reason reason) {
18981863
if (!ED->getTrailingWhereClause())
18991864
return;
19001865
forAllRequirementTypes(ED, [&](Type type, TypeRepr *typeRepr) {
1901-
checkType(type, typeRepr, ED, getDiagnoseCallback(ED, reason),
1902-
getDiagnoseCallback(ED, reason));
1866+
checkType(type, typeRepr, ED, getDiagnoser(ED, reason));
19031867
});
19041868
}
19051869

@@ -1913,8 +1877,7 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
19131877
// but just hide that from interfaces.
19141878
llvm::for_each(ED->getInherited(),
19151879
[&](TypeLoc nextInherited) {
1916-
checkType(nextInherited, ED, getDiagnoseCallback(ED),
1917-
getDiagnoseCallback(ED));
1880+
checkType(nextInherited, ED, getDiagnoser(ED));
19181881
});
19191882

19201883
bool hasPublicMembers = llvm::any_of(ED->getMembers(),
@@ -1927,8 +1890,7 @@ class ExportabilityChecker : public DeclVisitor<ExportabilityChecker> {
19271890

19281891
if (hasPublicMembers) {
19291892
checkType(ED->getExtendedType(), ED->getExtendedTypeRepr(), ED,
1930-
getDiagnoseCallback(ED, Reason::ExtensionWithPublicMembers),
1931-
getDiagnoseCallback(ED, Reason::ExtensionWithPublicMembers));
1893+
getDiagnoser(ED, Reason::ExtensionWithPublicMembers));
19321894
}
19331895

19341896
if (hasPublicMembers || !ED->getInherited().empty()) {

0 commit comments

Comments
 (0)