Skip to content

Commit 5f67a9a

Browse files
authored
Merge pull request #84107 from tshortli/custom-availability-in-library-level-api-swiftinterfaces
AST: Use availability to control decl visibility in public swiftinterfaces
2 parents ac00a21 + b047396 commit 5f67a9a

File tree

9 files changed

+251
-51
lines changed

9 files changed

+251
-51
lines changed

lib/AST/ASTPrinter.cpp

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include "swift/AST/ASTMangler.h"
2121
#include "swift/AST/ASTVisitor.h"
2222
#include "swift/AST/Attr.h"
23+
#include "swift/AST/AvailabilityConstraint.h"
24+
#include "swift/AST/AvailabilityContext.h"
2325
#include "swift/AST/Builtins.h"
2426
#include "swift/AST/ClangModuleLoader.h"
2527
#include "swift/AST/Comment.h"
@@ -189,6 +191,38 @@ static bool shouldPrintAllSemanticDetails(const PrintOptions &options) {
189191
return false;
190192
}
191193

194+
static bool shouldSkipDeclInPublicInterface(const Decl *D) {
195+
// @_spi should be skipped in the public interface.
196+
if (D->isSPI())
197+
return true;
198+
199+
// Decls that are unavailable at runtime in an availability domain that has
200+
// been @_spiOnly imported should be hidden from the public interface of
201+
// a -library-level=api module.
202+
auto &ctx = D->getDeclContext()->getASTContext();
203+
if (ctx.LangOpts.LibraryLevel != LibraryLevel::API)
204+
return false;
205+
206+
auto *SF = D->getDeclContext()->getParentSourceFile();
207+
if (!SF)
208+
return false;
209+
210+
auto constraints = getAvailabilityConstraintsForDecl(
211+
D, AvailabilityContext::forDeploymentTarget(ctx));
212+
llvm::SmallVector<AvailabilityDomain, 4> unavailableDomains;
213+
getRuntimeUnavailableDomains(constraints, unavailableDomains, ctx);
214+
215+
for (auto domain : unavailableDomains) {
216+
if (auto *domainDecl = domain.getDecl()) {
217+
if (SF->getRestrictedImportKind(domainDecl->getModuleContext()) ==
218+
RestrictedImportKind::SPIOnly)
219+
return true;
220+
}
221+
}
222+
223+
return false;
224+
}
225+
192226
/// Get the non-recursive printing options that should be applied when
193227
/// printing the type of a value decl.
194228
static NonRecursivePrintOptions getNonRecursiveOptions(const ValueDecl *D) {
@@ -293,8 +327,7 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
293327
if (D->getAttrs().hasAttribute<ImplementationOnlyAttr>())
294328
return false;
295329

296-
// Skip SPI decls if `PrintSPIs`.
297-
if (options.printPublicInterface() && D->isSPI())
330+
if (options.printPublicInterface() && shouldSkipDeclInPublicInterface(D))
298331
return false;
299332

300333
if (auto *VD = dyn_cast<ValueDecl>(D)) {
@@ -410,8 +443,6 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
410443
result.CurrentPrintabilityChecker =
411444
std::make_shared<ShouldPrintForModuleInterface>();
412445

413-
// FIXME: We don't really need 'public' on everything; we could just change
414-
// the default to 'public' and mark the 'internal' things.
415446
result.PrintAccess = true;
416447

417448
result.ExcludeAttrList = {

lib/AST/Attr.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,7 @@ void DeclAttributes::print(ASTPrinter &Printer, const PrintOptions &Options,
805805
AttributeVector modifiers;
806806
bool libraryLevelAPI =
807807
D && D->getASTContext().LangOpts.LibraryLevel == LibraryLevel::API;
808+
auto *SF = D ? D->getDeclContext()->getParentSourceFile() : nullptr;
808809

809810
for (auto DA : llvm::reverse(FlattenedAttrs)) {
810811
// Don't skip implicit custom attributes. Custom attributes like global
@@ -849,6 +850,16 @@ void DeclAttributes::print(ASTPrinter &Printer, const PrintOptions &Options,
849850
if (!semanticAttr)
850851
continue;
851852

853+
// In the public interfaces of -library-level=api modules, skip @available
854+
// attributes that refer to domains imported from @_spiOnly modules.
855+
if (Options.printPublicInterface() && libraryLevelAPI) {
856+
if (auto *domainDecl = semanticAttr->getDomain().getDecl()) {
857+
if (SF->getRestrictedImportKind(domainDecl->getModuleContext()) ==
858+
RestrictedImportKind::SPIOnly)
859+
continue;
860+
}
861+
}
862+
852863
if (isShortAvailable(*semanticAttr)) {
853864
if (semanticAttr->isSwiftLanguageModeSpecific())
854865
swiftVersionAvailableAttribute.emplace(*semanticAttr);

lib/AST/FeatureSet.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -354,14 +354,7 @@ static bool usesFeatureCoroutineAccessors(Decl *decl) {
354354
}
355355

356356
UNINTERESTING_FEATURE(GeneralizedIsSameMetaTypeBuiltin)
357-
358-
static bool usesFeatureCustomAvailability(Decl *decl) {
359-
for (auto attr : decl->getSemanticAvailableAttrs()) {
360-
if (attr.getDomain().isCustom())
361-
return true;
362-
}
363-
return false;
364-
}
357+
UNINTERESTING_FEATURE(CustomAvailability)
365358

366359
static bool usesFeatureAsyncExecutionBehaviorAttributes(Decl *decl) {
367360
// Explicit `@concurrent` attribute on the declaration.

lib/AST/Module.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2879,6 +2879,7 @@ bool SourceFile::hasTestableOrPrivateImport(
28792879
});
28802880
}
28812881

2882+
// FIXME: This should probably be requestified.
28822883
RestrictedImportKind SourceFile::getRestrictedImportKind(const ModuleDecl *module) const {
28832884
auto &imports = getASTContext().getImportCache();
28842885
RestrictedImportKind importKind = RestrictedImportKind::MissingImport;

lib/Sema/ResilienceDiagnostics.cpp

Lines changed: 72 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,42 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
219219
return true;
220220
}
221221

222+
/// Returns true if access to \p D should be diagnosed during exportability
223+
/// checking. These diagnostics would typically be handled by the access
224+
/// checker, and therefore should be suppressed to avoid duplicate diagnostics.
225+
/// However, extensions are special because they do not have an intrinsic access
226+
/// level and therefore the access checker does not currently handle them.
227+
/// Instead, diagnostics for decls referenced in extension signatures are
228+
/// deferred to exportability checking. An exportable extension is effectively a
229+
/// public extension.
230+
static bool shouldDiagnoseDeclAccess(const ValueDecl *D,
231+
const ExportContext &where) {
232+
auto reason = where.getExportabilityReason();
233+
auto DC = where.getDeclContext();
234+
if (!reason)
235+
return false;
236+
237+
switch (*reason) {
238+
case ExportabilityReason::ExtensionWithPublicMembers:
239+
case ExportabilityReason::ExtensionWithConditionalConformances:
240+
return true;
241+
case ExportabilityReason::Inheritance:
242+
return isa<ProtocolDecl>(D);
243+
case ExportabilityReason::AvailableAttribute:
244+
// If the context is an extension and that extension has an explicit
245+
// access level then availability domains access has already been
246+
// diagnosed.
247+
if (auto *ED = dyn_cast_or_null<ExtensionDecl>(DC->getAsDecl()))
248+
return !ED->getAttrs().getAttribute<AccessControlAttr>();
249+
return false;
250+
251+
case ExportabilityReason::General:
252+
case ExportabilityReason::ResultBuilder:
253+
case ExportabilityReason::PropertyWrapper:
254+
return false;
255+
}
256+
}
257+
222258
static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
223259
const ExportContext &where) {
224260
assert(where.mustOnlyReferenceExportedDecls());
@@ -247,41 +283,44 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
247283
}
248284
});
249285

250-
// Access levels from imports are reported with the others access levels.
251-
// Except for extensions and protocol conformances, we report them here.
252-
if (originKind == DisallowedOriginKind::NonPublicImport) {
253-
bool reportHere = [&] {
254-
switch (*reason) {
255-
case ExportabilityReason::ExtensionWithPublicMembers:
256-
case ExportabilityReason::ExtensionWithConditionalConformances:
257-
return true;
258-
case ExportabilityReason::Inheritance:
259-
return isa<ProtocolDecl>(D);
260-
case ExportabilityReason::AvailableAttribute:
261-
// If the context is an extension and that extension has an explicit
262-
// access level, then access has already been diagnosed for the
263-
// @available attribute.
264-
if (auto *ED = dyn_cast_or_null<ExtensionDecl>(DC->getAsDecl()))
265-
return !ED->getAttrs().getAttribute<AccessControlAttr>();
266-
return false;
267-
default:
268-
return false;
269-
}
270-
}();
271-
if (!reportHere)
272-
return false;
273-
}
274-
275-
if (originKind == DisallowedOriginKind::None)
286+
switch (originKind) {
287+
case DisallowedOriginKind::None:
288+
// The decl does not come from a source that needs to be checked for
289+
// exportability.
276290
return false;
277291

278-
// Some diagnostics emitted with the `MemberImportVisibility` feature enabled
279-
// subsume these diagnostics.
280-
if (originKind == DisallowedOriginKind::MissingImport &&
281-
ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
282-
/*allowMigration=*/true) &&
283-
SF)
284-
return false;
292+
case DisallowedOriginKind::NonPublicImport:
293+
// With a few exceptions, access levels from imports are diagnosed during
294+
// access checking and should be skipped here.
295+
if (!shouldDiagnoseDeclAccess(D, where))
296+
return false;
297+
break;
298+
299+
case DisallowedOriginKind::MissingImport:
300+
// Some diagnostics emitted with the `MemberImportVisibility` feature
301+
// enabled subsume these diagnostics.
302+
if (ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
303+
/*allowMigration=*/true) &&
304+
SF)
305+
return false;
306+
break;
307+
308+
case DisallowedOriginKind::SPIOnly:
309+
// Availability attributes referring to availability domains from modules
310+
// that are imported @_spiOnly in a -library-level=api will not be printed
311+
// in the public swiftinterface of the module and should therefore not be
312+
// diagnosed for exportability.
313+
if (reason && reason == ExportabilityReason::AvailableAttribute &&
314+
ctx.LangOpts.LibraryLevel == LibraryLevel::API)
315+
return false;
316+
break;
317+
318+
case DisallowedOriginKind::ImplementationOnly:
319+
case DisallowedOriginKind::SPIImported:
320+
case DisallowedOriginKind::SPILocal:
321+
case DisallowedOriginKind::FragileCxxAPI:
322+
break;
323+
}
285324

286325
if (auto accessor = dyn_cast<AccessorDecl>(D)) {
287326
// Only diagnose accessors if their disallowed origin kind differs from

test/Inputs/custom-modules/availability-domains/Lakes.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
#include <availability_domain.h>
22

3+
int huron_pred(void);
4+
35
CLANG_ENABLED_AVAILABILITY_DOMAIN(Salt);
6+
CLANG_DISABLED_AVAILABILITY_DOMAIN(Erie);
7+
CLANG_DYNAMIC_AVAILABILITY_DOMAIN(Huron, huron_pred);
48

59
#define AVAIL 0
610
#define UNAVAIL 1

test/Inputs/custom-modules/availability-domains/Seas.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
#include <availability_domain.h>
22

3+
int aegean_pred(void);
4+
35
CLANG_ENABLED_AVAILABILITY_DOMAIN(Baltic);
46
CLANG_DISABLED_AVAILABILITY_DOMAIN(Mediterranean);
7+
CLANG_DYNAMIC_AVAILABILITY_DOMAIN(Aegean, aegean_pred);
58

69
#define AVAIL 0
710
#define UNAVAIL 1

test/ModuleInterface/availability_custom_domains.swift

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,16 @@
1515

1616
import Oceans // re-exports Rivers
1717

18-
// CHECK: #if compiler(>=5.3) && $CustomAvailability
19-
// CHECK-NEXT: @available(Colorado)
18+
// CHECK-NOT: $CustomAvailability
19+
20+
// CHECK: @available(Colorado)
2021
// CHECK-NEXT: public func availableInColorado()
21-
// CHECK-NEXT: #endif
2222
@available(Colorado)
2323
public func availableInColorado() { }
2424

25-
// CHECK: #if compiler(>=5.3) && $CustomAvailability
26-
// CHECK-NEXT: @available(Arctic, unavailable)
25+
// CHECK: @available(Arctic, unavailable)
2726
// CHECK-NEXT: @available(Pacific)
2827
// CHECK-NEXT: public func unavailableInArcticButAvailableInPacific()
29-
// CHECK-NEXT: #endif
3028
@available(Arctic, unavailable)
3129
@available(Pacific)
3230
public func unavailableInArcticButAvailableInPacific() { }

0 commit comments

Comments
 (0)