Skip to content

Commit c8f99af

Browse files
xymusxymus
authored
Merge pull request #86335 from xymus/exportability-nle-warn
Sema: Enable exportability checks in non-library-evolution mode as warnings by default
2 parents 411d396 + 9d4ed92 commit c8f99af

22 files changed

+361
-130
lines changed

‎include/swift/AST/DiagnosticEngine.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,12 @@ namespace swift {
367367
/// emitted as a warning, but a note will still be emitted as a note.
368368
InFlightDiagnostic &limitBehavior(DiagnosticBehavior limit);
369369

370+
/// Prevent the diagnostic from behaving more severely than \p limit or the
371+
/// previously set limit. Use to compose conditions to downgrade a
372+
/// diagnostic in succession.
373+
InFlightDiagnostic
374+
&limitBehaviorIfMorePermissive(DiagnosticBehavior limit);
375+
370376
/// Conditionally prevent the diagnostic from behaving more severely than \p
371377
/// limit. If the condition is false, no limit is imposed.
372378
InFlightDiagnostic &limitBehaviorIf(bool shouldLimit,

‎include/swift/AST/DiagnosticsSema.def‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1272,8 +1272,9 @@ ERROR(module_not_compiled_with_library_evolution,none,
12721272
(Identifier, Identifier))
12731273
GROUPED_WARNING(implementation_only_requires_library_evolution,
12741274
ImplementationOnlyDeprecated,none,
1275-
"using '@_implementationOnly' without enabling library evolution "
1276-
"for %0 may lead to instability during execution",
1275+
"safely use '@_implementationOnly' without library evolution "
1276+
"by setting '-enable-experimental-feature CheckImplementationOnly' "
1277+
"for %0",
12771278
(Identifier))
12781279
GROUPED_WARNING(implementation_only_deprecated,
12791280
ImplementationOnlyDeprecated,none,

‎lib/AST/AccessRequests.cpp‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,19 +104,15 @@ AccessLevelRequest::evaluate(Evaluator &evaluator, ValueDecl *D) const {
104104
// Special case for dtors and enum elements: inherit from container
105105
if (D->getKind() == DeclKind::Destructor ||
106106
D->getKind() == DeclKind::EnumElement) {
107-
if (D->hasInterfaceType() && D->isInvalid()) {
108-
return AccessLevel::Private;
109-
} else {
110-
auto container = dyn_cast<NominalTypeDecl>(DC);
111-
if (D->getKind() == DeclKind::Destructor && !container) {
112-
// A destructor in an extension means @_objcImplementation. An
113-
// @_objcImplementation class's deinit is only called by the ObjC thunk,
114-
// if at all, so it is nonpublic.
115-
return AccessLevel::Internal;
116-
}
117-
118-
return std::max(container->getFormalAccess(), AccessLevel::Internal);
107+
auto container = dyn_cast<NominalTypeDecl>(DC);
108+
if (D->getKind() == DeclKind::Destructor && !container) {
109+
// A destructor in an extension means @_objcImplementation. An
110+
// @_objcImplementation class's deinit is only called by the ObjC thunk,
111+
// if at all, so it is nonpublic.
112+
return AccessLevel::Internal;
119113
}
114+
115+
return std::max(container->getFormalAccess(), AccessLevel::Internal);
120116
}
121117

122118
switch (DC->getContextKind()) {

‎lib/AST/Availability.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,8 +1014,6 @@ ExportedLevel swift::isExported(const ValueDecl *VD) {
10141014

10151015
// Is this a type exposed by default in a non-resilient module?
10161016
if (isa<NominalTypeDecl>(VD) &&
1017-
VD->getASTContext().LangOpts.hasFeature(
1018-
Feature::CheckImplementationOnly) &&
10191017
VD->getDeclContext()->getParentModule()->getResilienceStrategy() !=
10201018
ResilienceStrategy::Resilient &&
10211019
!VD->getAttrs().hasAttribute<ImplementationOnlyAttr>())
@@ -1064,5 +1062,5 @@ ExportedLevel swift::isExported(const ExtensionDecl *ED) {
10641062
for (const Decl *D : ED->getMembers())
10651063
membersExported = std::max(membersExported, isExported(D));
10661064

1067-
return membersExported;
1065+
return std::min(exported, membersExported);
10681066
}

‎lib/AST/AvailabilityScopeBuilder.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ class AvailabilityScopeBuilder : private ASTWalker {
479479
if (decl->isSPI())
480480
return true;
481481

482-
return isExported(decl) == ExportedLevel::None;
482+
return isExported(decl) != ExportedLevel::Exported;
483483
}
484484

485485
/// Returns the source range which should be refined by declaration. This

‎lib/AST/Decl.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2810,8 +2810,7 @@ ExportedLevel VarDecl::isLayoutExposedToClients() const {
28102810
return ExportedLevel::None;
28112811

28122812
auto M = getDeclContext()->getParentModule();
2813-
if (getASTContext().LangOpts.hasFeature(Feature::CheckImplementationOnly) &&
2814-
M->getResilienceStrategy() != ResilienceStrategy::Resilient) {
2813+
if (M->getResilienceStrategy() != ResilienceStrategy::Resilient) {
28152814
// Non-resilient module expose layouts by default.
28162815
return ExportedLevel::ImplicitlyExported;
28172816
} else {

‎lib/AST/DiagnosticEngine.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,13 @@ InFlightDiagnostic::limitBehavior(DiagnosticBehavior limit) {
458458
return *this;
459459
}
460460

461+
InFlightDiagnostic &
462+
InFlightDiagnostic::limitBehaviorIfMorePermissive(DiagnosticBehavior limit) {
463+
auto prev = getDiag().getBehaviorLimit();
464+
getDiag().setBehaviorLimit(prev.merge(limit));
465+
return *this;
466+
}
467+
461468
InFlightDiagnostic &
462469
InFlightDiagnostic::limitBehaviorUntilLanguageMode(DiagnosticBehavior limit,
463470
unsigned majorVersion) {

‎lib/Sema/ImportResolution.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -929,8 +929,8 @@ void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
929929
diag::implementation_only_deprecated);
930930
inFlight.fixItReplace(import.implementationOnlyRange, "internal");
931931
}
932-
} else if ( // Non-resilient client
933-
!shouldSuppressNonResilientImplementationOnlyImportDiagnostic(
932+
} else if (!ctx.LangOpts.hasFeature(Feature::CheckImplementationOnly) &&
933+
!shouldSuppressNonResilientImplementationOnlyImportDiagnostic(
934934
targetName.str(), importerName.str())) {
935935
ctx.Diags.diagnose(import.importLoc,
936936
diag::implementation_only_requires_library_evolution,

‎lib/Sema/ResilienceDiagnostics.cpp‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
194194
auto ignoredDowngradeToWarning = DowngradeToWarning::No;
195195
auto originKind =
196196
getDisallowedOriginKind(D, where, ignoredDowngradeToWarning);
197-
if (where.canReferenceOrigin(originKind))
197+
auto commonBehavior = where.behaviorForReferenceToOrigin(originKind);
198+
if (commonBehavior == DiagnosticBehavior::Ignore)
198199
return false;
199200

200201
// As an exception, if the import of the module that defines the desugared
@@ -216,15 +217,17 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
216217
TAD, definingModule->getNameStr(), D->getNameStr(),
217218
static_cast<unsigned>(*reason), definingModule->getName(),
218219
static_cast<unsigned>(originKind))
219-
.warnUntilLanguageModeIf(warnPreSwift6, 6);
220+
.warnUntilLanguageModeIf(warnPreSwift6, 6)
221+
.limitBehaviorIfMorePermissive(commonBehavior);
220222
} else {
221223
ctx.Diags
222224
.diagnose(loc,
223225
diag::inlinable_typealias_desugars_to_type_from_hidden_module,
224226
TAD, definingModule->getNameStr(), D->getNameStr(),
225227
fragileKind.getSelector(), definingModule->getName(),
226228
static_cast<unsigned>(originKind))
227-
.warnUntilLanguageModeIf(warnPreSwift6, 6);
229+
.warnUntilLanguageModeIf(warnPreSwift6, 6)
230+
.limitBehaviorIfMorePermissive(commonBehavior);
228231
}
229232
D->diagnose(diag::kind_declared_here, DescriptiveDeclKind::Type);
230233

@@ -269,7 +272,13 @@ static bool shouldDiagnoseDeclAccess(const ValueDecl *D,
269272
switch (*reason) {
270273
case ExportabilityReason::ExtensionWithPublicMembers:
271274
case ExportabilityReason::ExtensionWithConditionalConformances:
272-
return true;
275+
// Allow public members in extensions of implicitly exported types.
276+
// Extensions cannot define stored variables avoiding the memory layout
277+
// concerns and we don't print swiftinterfaces in non-library-evolution
278+
// mode.
279+
// We should be able to always allow this if it's correctly guarded
280+
// at generating module interfaces and generated headers.
281+
return where.getExportedLevel() == ExportedLevel::Exported;
273282
case ExportabilityReason::Inheritance:
274283
case ExportabilityReason::ImplicitlyPublicInheritance:
275284
return isa<ProtocolDecl>(D);
@@ -323,7 +332,8 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
323332
}
324333
});
325334

326-
if (where.canReferenceOrigin(originKind))
335+
auto commonBehavior = where.behaviorForReferenceToOrigin(originKind);
336+
if (commonBehavior == DiagnosticBehavior::Ignore)
327337
return false;
328338

329339
auto fragileKind = where.getFragileFunctionKind();
@@ -384,21 +394,16 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
384394
static_cast<unsigned>(*reason),
385395
definingModule->getName(),
386396
static_cast<unsigned>(originKind))
387-
.limitBehavior(limit);
397+
.limitBehavior(limit.merge(commonBehavior));
388398

389399
D->diagnose(diag::kind_declared_here, D->getDescriptiveKind());
390400
} else {
391-
// Only implicitly imported decls should be reported as a warning,
392-
// and only for language versions below Swift 6.
393-
assert(downgradeToWarning == DowngradeToWarning::No ||
394-
originKind == DisallowedOriginKind::MissingImport &&
395-
"Only implicitly imported decls should be reported as a warning.");
396-
397401
ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_from_hidden_module, D,
398402
fragileKind.getSelector(), definingModule->getName(),
399403
static_cast<unsigned>(originKind))
400404
.warnUntilLanguageModeIf(downgradeToWarning == DowngradeToWarning::Yes,
401-
6);
405+
6)
406+
.limitBehaviorIfMorePermissive(commonBehavior);
402407

403408
if (originKind == DisallowedOriginKind::MissingImport &&
404409
downgradeToWarning == DowngradeToWarning::Yes)
@@ -470,7 +475,8 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
470475
});
471476

472477
auto originKind = getDisallowedOriginKind(ext, where);
473-
if (where.canReferenceOrigin(originKind))
478+
auto commonBehavior = where.behaviorForReferenceToOrigin(originKind);
479+
if (commonBehavior == DiagnosticBehavior::Ignore)
474480
return false;
475481

476482
auto reason = where.getExportabilityReason();
@@ -486,8 +492,9 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
486492
(warnIfConformanceUnavailablePreSwift6 &&
487493
originKind != DisallowedOriginKind::SPIOnly &&
488494
originKind != DisallowedOriginKind::NonPublicImport) ||
489-
originKind == DisallowedOriginKind::MissingImport,
490-
6);
495+
originKind == DisallowedOriginKind::MissingImport,
496+
6)
497+
.limitBehaviorIfMorePermissive(commonBehavior);
491498

492499
if (!ctx.LangOpts.hasFeature(Feature::StrictAccessControl) &&
493500
originKind == DisallowedOriginKind::MissingImport &&

‎lib/Sema/TypeCheckAccess.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2169,8 +2169,6 @@ swift::getDisallowedOriginKind(const Decl *decl,
21692169

21702170
// Implementation-only memory layouts for non-library-evolution mode.
21712171
if (isa<NominalTypeDecl>(decl) &&
2172-
Context.LangOpts.hasFeature(
2173-
Feature::CheckImplementationOnly) &&
21742172
decl->getAttrs().hasAttribute<ImplementationOnlyAttr>())
21752173
return DisallowedOriginKind::ImplementationOnlyMemoryLayout;
21762174

0 commit comments

Comments
 (0)