Skip to content

Commit cc3dc38

Browse files
AnnaZaksAnnaZaks
authored
Merge pull request #21131 from harlanhaskins/trouble-at-the-skate-park
[5.0] [Sema] Diagnose internal(set) from @inlinable functions
2 parents 06c8a0b + 6464b30 commit cc3dc38

File tree

9 files changed

+138
-53
lines changed

9 files changed

+138
-53
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4109,27 +4109,33 @@ ERROR(usable_from_inline_attr_in_protocol,none,
41094109
"a default argument value|" \
41104110
"a property initializer in a '@_fixed_layout' type}"
41114111

4112+
#define DECL_OR_ACCESSOR "%select{%0|%0 for}"
4113+
41124114
ERROR(local_type_in_inlinable_function,
41134115
none, "type %0 cannot be nested inside " FRAGILE_FUNC_KIND "1",
41144116
(DeclName, unsigned))
41154117

41164118
ERROR(resilience_decl_unavailable,
4117-
none, "%0 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
4119+
none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
41184120
"cannot be referenced from " FRAGILE_FUNC_KIND "3",
4119-
(DescriptiveDeclKind, DeclName, AccessLevel, unsigned))
4121+
(DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
41204122

41214123
WARNING(resilience_decl_unavailable_warn,
4122-
none, "%0 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
4124+
none, DECL_OR_ACCESSOR "4 %1 is %select{private|fileprivate|internal|%error|%error}2 and "
41234125
"should not be referenced from " FRAGILE_FUNC_KIND "3",
4124-
(DescriptiveDeclKind, DeclName, AccessLevel, unsigned))
4126+
(DescriptiveDeclKind, DeclName, AccessLevel, unsigned, bool))
41254127

41264128
#undef FRAGILE_FUNC_KIND
41274129

41284130
NOTE(resilience_decl_declared_here_public,
4129-
none, "%0 %1 is not public", (DescriptiveDeclKind, DeclName))
4131+
none, DECL_OR_ACCESSOR "2 %1 is not public",
4132+
(DescriptiveDeclKind, DeclName, bool))
41304133

41314134
NOTE(resilience_decl_declared_here,
4132-
none, "%0 %1 is not '@usableFromInline' or public", (DescriptiveDeclKind, DeclName))
4135+
none, DECL_OR_ACCESSOR "2 %1 is not '@usableFromInline' or public",
4136+
(DescriptiveDeclKind, DeclName, bool))
4137+
4138+
#undef DECL_OR_ACCESSOR
41334139

41344140
ERROR(class_designated_init_inlinable_resilient,none,
41354141
"initializer for class %0 is "

‎lib/AST/Decl.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2411,8 +2411,7 @@ bool ValueDecl::isUsableFromInline() const {
24112411
return true;
24122412

24132413
if (auto *containingProto = dyn_cast<ProtocolDecl>(getDeclContext())) {
2414-
if (isProtocolRequirement() &&
2415-
containingProto->getAttrs().hasAttribute<UsableFromInlineAttr>())
2414+
if (containingProto->getAttrs().hasAttribute<UsableFromInlineAttr>())
24162415
return true;
24172416
}
24182417

‎lib/Sema/ResilienceDiagnostics.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,21 +134,37 @@ bool TypeChecker::diagnoseInlinableDeclRef(SourceLoc loc,
134134
downgradeToWarning = DowngradeToWarning::Yes;
135135
}
136136

137+
auto diagName = D->getFullName();
138+
bool isAccessor = false;
139+
140+
// Swift 4.2 did not check accessor accessiblity.
141+
if (auto accessor = dyn_cast<AccessorDecl>(D)) {
142+
isAccessor = true;
143+
144+
if (!Context.isSwiftVersionAtLeast(5))
145+
downgradeToWarning = DowngradeToWarning::Yes;
146+
147+
// For accessors, diagnose with the name of the storage instead of the
148+
// implicit '_'.
149+
diagName = accessor->getStorage()->getFullName();
150+
}
151+
137152
auto diagID = diag::resilience_decl_unavailable;
138153
if (downgradeToWarning == DowngradeToWarning::Yes)
139154
diagID = diag::resilience_decl_unavailable_warn;
140155

141156
diagnose(loc, diagID,
142-
D->getDescriptiveKind(), D->getFullName(),
157+
D->getDescriptiveKind(), diagName,
143158
D->getFormalAccessScope().accessLevelForDiagnostics(),
144-
static_cast<unsigned>(Kind));
159+
static_cast<unsigned>(Kind),
160+
isAccessor);
145161

146162
if (TreatUsableFromInlineAsPublic) {
147163
diagnose(D, diag::resilience_decl_declared_here,
148-
D->getDescriptiveKind(), D->getFullName());
164+
D->getDescriptiveKind(), diagName, isAccessor);
149165
} else {
150166
diagnose(D, diag::resilience_decl_declared_here_public,
151-
D->getDescriptiveKind(), D->getFullName());
167+
D->getDescriptiveKind(), diagName, isAccessor);
152168
}
153169

154170
return (downgradeToWarning == DowngradeToWarning::No);

‎lib/Sema/TypeCheckAvailability.cpp

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,12 +1355,12 @@ void TypeChecker::diagnosePotentialUnavailability(
13551355
}
13561356

13571357
void TypeChecker::diagnosePotentialAccessorUnavailability(
1358-
AccessorDecl *Accessor, SourceRange ReferenceRange,
1358+
const AccessorDecl *Accessor, SourceRange ReferenceRange,
13591359
const DeclContext *ReferenceDC, const UnavailabilityReason &Reason,
13601360
bool ForInout) {
13611361
assert(Accessor->isGetterOrSetter());
13621362

1363-
AbstractStorageDecl *ASD = Accessor->getStorage();
1363+
const AbstractStorageDecl *ASD = Accessor->getStorage();
13641364
DeclName Name = ASD->getFullName();
13651365

13661366
auto &diag = ForInout ? diag::availability_inout_accessor_only_version_newer
@@ -2355,7 +2355,8 @@ class AvailabilityWalker : public ASTWalker {
23552355
bool diagAvailability(const ValueDecl *D, SourceRange R,
23562356
const ApplyExpr *call = nullptr,
23572357
bool AllowPotentiallyUnavailableProtocol = false,
2358-
bool SignalOnPotentialUnavailability = true);
2358+
bool SignalOnPotentialUnavailability = true,
2359+
bool ForInout = false);
23592360

23602361
private:
23612362
bool diagnoseIncDecRemoval(const ValueDecl *D, SourceRange R,
@@ -2511,32 +2512,13 @@ class AvailabilityWalker : public ASTWalker {
25112512
void diagAccessorAvailability(AccessorDecl *D, SourceRange ReferenceRange,
25122513
const DeclContext *ReferenceDC,
25132514
bool ForInout) const {
2514-
if (!D) {
2515+
if (diagnoseDeclAvailability(D, TC,
2516+
const_cast<DeclContext*>(ReferenceDC),
2517+
ReferenceRange,
2518+
/*AllowPotentiallyUnavailableProtocol*/false,
2519+
/*SignalOnPotentialUnavailability*/false,
2520+
ForInout))
25152521
return;
2516-
}
2517-
2518-
// If the property/subscript is unconditionally unavailable, don't bother
2519-
// with any of the rest of this.
2520-
if (AvailableAttr::isUnavailable(D->getStorage()))
2521-
return;
2522-
2523-
if (diagnoseExplicitUnavailability(D, ReferenceRange, ReferenceDC,
2524-
/*call*/nullptr)) {
2525-
return;
2526-
}
2527-
2528-
// Make sure not to diagnose an accessor's deprecation if we already
2529-
// complained about the property/subscript.
2530-
if (!TypeChecker::getDeprecated(D->getStorage()))
2531-
TC.diagnoseIfDeprecated(ReferenceRange, ReferenceDC, D, /*call*/nullptr);
2532-
2533-
auto MaybeUnavail = TC.checkDeclarationAvailability(D, ReferenceRange.Start,
2534-
DC);
2535-
if (MaybeUnavail.hasValue()) {
2536-
TC.diagnosePotentialAccessorUnavailability(D, ReferenceRange, ReferenceDC,
2537-
MaybeUnavail.getValue(),
2538-
ForInout);
2539-
}
25402522
}
25412523
};
25422524
} // end anonymous namespace
@@ -2546,7 +2528,8 @@ class AvailabilityWalker : public ASTWalker {
25462528
bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
25472529
const ApplyExpr *call,
25482530
bool AllowPotentiallyUnavailableProtocol,
2549-
bool SignalOnPotentialUnavailability) {
2531+
bool SignalOnPotentialUnavailability,
2532+
bool ForInout) {
25502533
if (!D)
25512534
return false;
25522535

@@ -2557,6 +2540,16 @@ bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
25572540
return true;
25582541
}
25592542

2543+
// Keep track if this is an accessor.
2544+
auto accessor = dyn_cast<AccessorDecl>(D);
2545+
2546+
if (accessor) {
2547+
// If the property/subscript is unconditionally unavailable, don't bother
2548+
// with any of the rest of this.
2549+
if (AvailableAttr::isUnavailable(accessor->getStorage()))
2550+
return false;
2551+
}
2552+
25602553
if (FragileKind)
25612554
if (R.isValid())
25622555
if (TC.diagnoseInlinableDeclRef(R.Start, D, DC, *FragileKind,
@@ -2566,16 +2559,28 @@ bool AvailabilityWalker::diagAvailability(const ValueDecl *D, SourceRange R,
25662559
if (diagnoseExplicitUnavailability(D, R, DC, call))
25672560
return true;
25682561

2562+
// Make sure not to diagnose an accessor's deprecation if we already
2563+
// complained about the property/subscript.
2564+
bool isAccessorWithDeprecatedStorage =
2565+
accessor && TypeChecker::getDeprecated(accessor->getStorage());
2566+
25692567
// Diagnose for deprecation
2570-
TC.diagnoseIfDeprecated(R, DC, D, call);
2568+
if (!isAccessorWithDeprecatedStorage)
2569+
TC.diagnoseIfDeprecated(R, DC, D, call);
25712570

25722571
if (AllowPotentiallyUnavailableProtocol && isa<ProtocolDecl>(D))
25732572
return false;
25742573

25752574
// Diagnose (and possibly signal) for potential unavailability
25762575
auto maybeUnavail = TC.checkDeclarationAvailability(D, R.Start, DC);
25772576
if (maybeUnavail.hasValue()) {
2578-
TC.diagnosePotentialUnavailability(D, R, DC, maybeUnavail.getValue());
2577+
if (accessor) {
2578+
TC.diagnosePotentialAccessorUnavailability(accessor, R, DC,
2579+
maybeUnavail.getValue(),
2580+
ForInout);
2581+
} else {
2582+
TC.diagnosePotentialUnavailability(D, R, DC, maybeUnavail.getValue());
2583+
}
25792584
if (SignalOnPotentialUnavailability)
25802585
return true;
25812586
}
@@ -2751,10 +2756,12 @@ bool swift::diagnoseDeclAvailability(const ValueDecl *Decl,
27512756
DeclContext *DC,
27522757
SourceRange R,
27532758
bool AllowPotentiallyUnavailableProtocol,
2754-
bool SignalOnPotentialUnavailability)
2759+
bool SignalOnPotentialUnavailability,
2760+
bool ForInout)
27552761
{
27562762
AvailabilityWalker AW(TC, DC);
27572763
return AW.diagAvailability(Decl, R, nullptr,
27582764
AllowPotentiallyUnavailableProtocol,
2759-
SignalOnPotentialUnavailability);
2765+
SignalOnPotentialUnavailability,
2766+
ForInout);
27602767
}

‎lib/Sema/TypeCheckAvailability.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ bool diagnoseDeclAvailability(const ValueDecl *Decl,
4141
DeclContext *DC,
4242
SourceRange R,
4343
bool AllowPotentiallyUnavailableProtocol,
44-
bool SignalOnPotentialUnavailability);
44+
bool SignalOnPotentialUnavailability,
45+
bool ForInout);
4546

4647
void diagnoseUnavailableOverride(ValueDecl *override,
4748
const ValueDecl *base,

‎lib/Sema/TypeCheckType.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1515,7 +1515,8 @@ static bool diagnoseAvailability(IdentTypeRepr *IdType,
15151515
TypeChecker &tc = static_cast<TypeChecker &>(*ctx.getLazyResolver());
15161516
if (diagnoseDeclAvailability(typeDecl, tc, DC, comp->getIdLoc(),
15171517
AllowPotentiallyUnavailableProtocol,
1518-
/*SignalOnPotentialUnavailability*/false)) {
1518+
/*SignalOnPotentialUnavailability*/false,
1519+
/*ForInout*/false)) {
15191520
return true;
15201521
}
15211522
}

‎lib/Sema/TypeChecker.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2077,7 +2077,7 @@ class TypeChecker final : public LazyResolver {
20772077
/// Emits a diagnostic for a reference to a storage accessor that is
20782078
/// potentially unavailable.
20792079
void diagnosePotentialAccessorUnavailability(
2080-
AccessorDecl *Accessor, SourceRange ReferenceRange,
2080+
const AccessorDecl *Accessor, SourceRange ReferenceRange,
20812081
const DeclContext *ReferenceDC, const UnavailabilityReason &Reason,
20822082
bool ForInout);
20832083

‎test/Compatibility/attr_inlinable_swift42.swift

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// RUN: %target-typecheck-verify-swift -swift-version 4.2
2+
// RUN: %target-typecheck-verify-swift -swift-version 4.2 -enable-testing
3+
// RUN: %target-typecheck-verify-swift -swift-version 4.2 -enable-resilience
4+
// RUN: %target-typecheck-verify-swift -swift-version 4.2 -enable-resilience -enable-testing
5+
6+
enum InternalEnum {
7+
// expected-note@-1 {{type declared here}}
8+
case apple
9+
case orange
10+
}
11+
12+
@usableFromInline enum VersionedEnum {
13+
case apple
14+
case orange
15+
case pear(InternalEnum)
16+
// expected-warning@-1 {{type of enum case in '@usableFromInline' enum should be '@usableFromInline' or public}}
17+
case persimmon(String)
18+
}
19+
20+
public struct HasInternalSetProperty {
21+
public internal(set) var x: Int // expected-note {{setter for 'x' is not '@usableFromInline' or public}}
22+
23+
@inlinable public mutating func setsX() {
24+
x = 10 // expected-warning {{setter for 'x' is internal and should not be referenced from an '@inlinable' function}}
25+
}
26+
}
27+
28+
@usableFromInline protocol P {
29+
typealias T = Int
30+
}
31+
32+
extension P {
33+
@inlinable func f() {
34+
_ = T.self // typealiases were not checked in Swift 4.2, but P.T inherits @usableFromInline in Swift 4.2 mode
35+
}
36+
}

‎test/attr/attr_inlinable.swift

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
// RUN: %target-typecheck-verify-swift -swift-version 4.2
2-
// RUN: %target-typecheck-verify-swift -swift-version 4.2 -enable-testing
3-
// RUN: %target-typecheck-verify-swift -swift-version 4.2 -enable-resilience
4-
// RUN: %target-typecheck-verify-swift -swift-version 4.2 -enable-resilience -enable-testing
1+
// RUN: %target-typecheck-verify-swift -swift-version 5
2+
// RUN: %target-typecheck-verify-swift -swift-version 5 -enable-testing
3+
// RUN: %target-typecheck-verify-swift -swift-version 5 -enable-resilience
4+
// RUN: %target-typecheck-verify-swift -swift-version 5 -enable-resilience -enable-testing
55
@inlinable struct TestInlinableStruct {}
66
// expected-error@-1 {{'@inlinable' attribute cannot be applied to this declaration}}
77

@@ -157,7 +157,7 @@ enum InternalEnum {
157157
case apple
158158
case orange
159159
case pear(InternalEnum)
160-
// expected-warning@-1 {{type of enum case in '@usableFromInline' enum should be '@usableFromInline' or public}}
160+
// expected-error@-1 {{type of enum case in '@usableFromInline' enum must be '@usableFromInline' or public}}
161161
case persimmon(String)
162162
}
163163

@@ -248,3 +248,22 @@ public struct KeypathStruct {
248248
// expected-error@-1 {{property 'x' is internal and cannot be referenced from an '@inlinable' function}}
249249
}
250250
}
251+
252+
public struct HasInternalSetProperty {
253+
public internal(set) var x: Int // expected-note {{setter for 'x' is not '@usableFromInline' or public}}
254+
255+
@inlinable public mutating func setsX() {
256+
x = 10 // expected-error {{setter for 'x' is internal and cannot be referenced from an '@inlinable' function}}
257+
}
258+
}
259+
260+
@usableFromInline protocol P {
261+
typealias T = Int
262+
}
263+
264+
extension P {
265+
@inlinable func f() {
266+
_ = T.self // ok, typealias inherits @usableFromInline from P
267+
}
268+
}
269+

0 commit comments

Comments
 (0)