Skip to content

Commit c0d2ea4

Browse files
xymusxymus
authored
Merge pull request swiftlang#33892 from xymus/require-less-availability
[Sema] Improve -require-explicit-availability precision
2 parents 680f028 + 8248020 commit c0d2ea4

File tree

3 files changed

+149
-27
lines changed

3 files changed

+149
-27
lines changed

‎lib/Sema/TypeCheckAvailability.cpp

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2854,39 +2854,46 @@ bool swift::diagnoseDeclAvailability(const ValueDecl *Decl,
28542854
return AW.diagAvailability(const_cast<ValueDecl *>(Decl), R, nullptr, Flags);
28552855
}
28562856

2857-
/// Should we warn that \p valueDecl needs an explicit availability annotation
2858-
/// in -require-explicit-availaiblity mode?
2859-
static bool declNeedsExplicitAvailability(const ValueDecl *valueDecl) {
2860-
AccessScope scope =
2861-
valueDecl->getFormalAccessScope(/*useDC*/nullptr,
2862-
/*treatUsableFromInlineAsPublic*/true);
2863-
if (!scope.isPublic() ||
2864-
valueDecl->getAttrs().hasAttribute<AlwaysEmitIntoClientAttr>())
2857+
/// Should we warn that \p decl needs an explicit availability annotation
2858+
/// in -require-explicit-availability mode?
2859+
static bool declNeedsExplicitAvailability(const Decl *decl) {
2860+
// Skip non-public decls.
2861+
if (auto valueDecl = dyn_cast<const ValueDecl>(decl)) {
2862+
AccessScope scope =
2863+
valueDecl->getFormalAccessScope(/*useDC*/nullptr,
2864+
/*treatUsableFromInlineAsPublic*/true);
2865+
if (!scope.isPublic())
2866+
return false;
2867+
}
2868+
2869+
// Skip functions emitted into clients or SPI.
2870+
if (decl->getAttrs().hasAttribute<AlwaysEmitIntoClientAttr>() ||
2871+
decl->isSPI())
28652872
return false;
28662873

28672874
// Warn on decls without an introduction version.
2868-
auto &ctx = valueDecl->getASTContext();
2869-
auto safeRangeUnderApprox = AvailabilityInference::availableRange(valueDecl, ctx);
2875+
auto &ctx = decl->getASTContext();
2876+
auto safeRangeUnderApprox = AvailabilityInference::availableRange(decl, ctx);
28702877
return !safeRangeUnderApprox.getOSVersion().hasLowerEndpoint() &&
2871-
!valueDecl->getAttrs().isUnavailable(ctx);
2878+
!decl->getAttrs().isUnavailable(ctx);
28722879
}
28732880

28742881
void swift::checkExplicitAvailability(Decl *decl) {
2875-
// Check only if the command line option was set.
2876-
if (!decl->getASTContext().LangOpts.RequireExplicitAvailability)
2882+
// Skip if the command line option was not set and
2883+
// accessors as we check the pattern binding decl instead.
2884+
if (!decl->getASTContext().LangOpts.RequireExplicitAvailability ||
2885+
isa<AccessorDecl>(decl))
28772886
return;
28782887

2879-
// Skip nominal type members as the type should be annotated.
2880-
auto declContext = decl->getDeclContext();
2881-
if (isa<NominalTypeDecl>(declContext))
2882-
return;
2888+
// Only look at decls at module level or in extensions.
2889+
// This could be changed to force having attributes on all decls.
2890+
if (!decl->getDeclContext()->isModuleScopeContext() &&
2891+
!isa<ExtensionDecl>(decl->getDeclContext())) return;
28832892

2884-
ValueDecl *valueDecl = dyn_cast<ValueDecl>(decl);
2885-
if (valueDecl == nullptr) {
2893+
if (auto extension = dyn_cast<ExtensionDecl>(decl)) {
28862894
// decl should be either a ValueDecl or an ExtensionDecl.
2887-
auto extension = cast<ExtensionDecl>(decl);
2888-
valueDecl = extension->getExtendedNominal();
2889-
if (!valueDecl)
2895+
auto extended = extension->getExtendedNominal();
2896+
if (!extended || !extended->getFormalAccessScope().isPublic())
28902897
return;
28912898

28922899
// Skip extensions without public members or conformances.
@@ -2909,12 +2916,25 @@ void swift::checkExplicitAvailability(Decl *decl) {
29092916
});
29102917

29112918
if (!hasMembers && !hasProtocols) return;
2919+
2920+
} else if (auto pbd = dyn_cast<PatternBindingDecl>(decl)) {
2921+
// Check the first var instead.
2922+
if (pbd->getNumPatternEntries() == 0)
2923+
return;
2924+
2925+
llvm::SmallVector<VarDecl *, 2> vars;
2926+
pbd->getPattern(0)->collectVariables(vars);
2927+
if (vars.empty())
2928+
return;
2929+
2930+
decl = vars.front();
29122931
}
29132932

2914-
if (declNeedsExplicitAvailability(valueDecl)) {
2933+
if (declNeedsExplicitAvailability(decl)) {
29152934
auto diag = decl->diagnose(diag::public_decl_needs_availability);
29162935

2917-
auto suggestPlatform = decl->getASTContext().LangOpts.RequireExplicitAvailabilityTarget;
2936+
auto suggestPlatform =
2937+
decl->getASTContext().LangOpts.RequireExplicitAvailabilityTarget;
29182938
if (!suggestPlatform.empty()) {
29192939
auto InsertLoc = decl->getAttrs().getStartLoc(/*forModifiers=*/false);
29202940
if (InsertLoc.isInvalid())
@@ -2927,7 +2947,7 @@ void swift::checkExplicitAvailability(Decl *decl) {
29272947
{
29282948
llvm::raw_string_ostream Out(AttrText);
29292949

2930-
auto &ctx = valueDecl->getASTContext();
2950+
auto &ctx = decl->getASTContext();
29312951
StringRef OriginalIndent = Lexer::getIndentationForLine(
29322952
ctx.SourceMgr, InsertLoc);
29332953
Out << "@available(" << suggestPlatform << ", *)\n"

‎lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1629,6 +1629,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
16291629

16301630
checkAccessControl(PBD);
16311631

1632+
checkExplicitAvailability(PBD);
1633+
16321634
// If the initializers in the PBD aren't checked yet, do so now.
16331635
for (auto i : range(PBD->getNumPatternEntries())) {
16341636
if (!PBD->isInitialized(i))
@@ -1683,6 +1685,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
16831685

16841686
checkAccessControl(SD);
16851687

1688+
checkExplicitAvailability(SD);
1689+
16861690
if (!checkOverrides(SD)) {
16871691
// If a subscript has an override attribute but does not override
16881692
// anything, complain.
@@ -2642,6 +2646,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
26422646

26432647
checkAccessControl(CD);
26442648

2649+
checkExplicitAvailability(CD);
2650+
26452651
if (requiresDefinition(CD) && !CD->hasBody()) {
26462652
// Complain if we should have a body.
26472653
CD->diagnose(diag::missing_initializer_def);

‎test/attr/require_explicit_availability.swift

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
// Test the -require-explicit-availability flag
22
// REQUIRES: OS=macosx
33

4-
// RUN: %swiftc_driver -typecheck -parse-stdlib -target x86_64-apple-macosx10.10 -Xfrontend -verify -require-explicit-availability -require-explicit-availability-target "macOS 10.10" %s
5-
// RUN: %swiftc_driver -typecheck -parse-stdlib -target x86_64-apple-macosx10.10 -warnings-as-errors %s
4+
// RUN: %swiftc_driver -typecheck -parse-as-library -target x86_64-apple-macosx10.10 -Xfrontend -verify -require-explicit-availability -require-explicit-availability-target "macOS 10.10" %s
65

76
public struct S { // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}}
87
public func method() { }
@@ -52,6 +51,11 @@ extension S { // expected-warning {{public declarations should have an availabil
5251
public func warnForPublicMembers() { } // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{3-3=@available(macOS 10.10, *)\n }}
5352
}
5453

54+
@available(macOS 10.1, *)
55+
extension S {
56+
public func okWhenTheExtensionHasAttribute() { }
57+
}
58+
5559
extension S {
5660
internal func dontWarnWithoutPublicMembers() { }
5761
private func dontWarnWithoutPublicMembers1() { }
@@ -68,3 +72,95 @@ open class OpenClass { } // expected-warning {{public declarations should have a
6872
private class PrivateClass { }
6973

7074
extension PrivateClass { }
75+
76+
@available(macOS 10.1, *)
77+
public protocol PublicProtocol { }
78+
79+
@available(macOS 10.1, *)
80+
extension S : PublicProtocol { }
81+
82+
@_spi(SPIsAreOK)
83+
public func spiFunc() { }
84+
85+
@_spi(SPIsAreOK)
86+
public struct spiStruct {
87+
public func spiMethod() {}
88+
}
89+
90+
extension spiStruct {
91+
public func spiExtensionMethod() {}
92+
}
93+
94+
public var publicVar = S() // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{1-1=@available(macOS 10.10, *)\n}}
95+
96+
@available(macOS 10.10, *)
97+
public var publicVarOk = S()
98+
99+
public var (a, b) = (S(), S()) // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{1-1=@available(macOS 10.10, *)\n}}
100+
101+
@available(macOS 10.10, *)
102+
public var (c, d) = (S(), S())
103+
104+
public var _ = S() // expected-error {{global variable declaration does not bind any variables}}
105+
106+
public var implicitGet: S { // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{1-1=@available(macOS 10.10, *)\n}}
107+
return S()
108+
}
109+
110+
@available(macOS 10.10, *)
111+
public var implicitGetOk: S {
112+
return S()
113+
}
114+
115+
public var computed: S { // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{1-1=@available(macOS 10.10, *)\n}}
116+
get { return S() }
117+
set { }
118+
}
119+
120+
public var computedHalf: S { // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{1-1=@available(macOS 10.10, *)\n}}
121+
@available(macOS 10.10, *)
122+
get { return S() }
123+
set { }
124+
}
125+
126+
// FIXME the following warning is not needed.
127+
public var computedOk: S { // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{1-1=@available(macOS 10.10, *)\n}}
128+
@available(macOS 10.10, *)
129+
get { return S() }
130+
131+
@available(macOS 10.10, *)
132+
set { }
133+
}
134+
135+
@available(macOS 10.10, *)
136+
public var computedOk1: S {
137+
get { return S() }
138+
set { }
139+
}
140+
141+
public class SomeClass { // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{1-1=@available(macOS 10.10, *)\n}}
142+
public init () {}
143+
144+
public subscript(index: String) -> Int {
145+
get { return 42; }
146+
set(newValue) { }
147+
}
148+
}
149+
150+
extension SomeClass { // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{1-1=@available(macOS 10.10, *)\n}}
151+
public convenience init(s : S) {} // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{3-3=@available(macOS 10.10, *)\n }}
152+
153+
@available(macOS 10.10, *)
154+
public convenience init(s : SomeClass) {}
155+
156+
public subscript(index: Int) -> Int { // expected-warning {{public declarations should have an availability attribute when building with -require-explicit-availability}} {{3-3=@available(macOS 10.10, *)\n }}
157+
get { return 42; }
158+
set(newValue) { }
159+
}
160+
161+
@available(macOS 10.10, *)
162+
public subscript(index: S) -> Int {
163+
get { return 42; }
164+
set(newValue) { }
165+
}
166+
}

0 commit comments

Comments
 (0)