Skip to content

Commit 5213153

Browse files
committed
Sema: Catch invalid stored properties in the StorageImplInfoRequest
Previously we were only checking in declarations in primary files, which is insufficient since SILGen crashes if presented with an invalid stored property. Fixes <rdar://problem/54471229>, <https://bugs.swift.org/browse/SR-11322>.
1 parent c3bb0af commit 5213153

File tree

3 files changed

+37
-27
lines changed

3 files changed

+37
-27
lines changed

lib/Sema/TypeCheckDecl.cpp

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2159,24 +2159,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
21592159
// Reject cases where this is a variable that has storage but it isn't
21602160
// allowed.
21612161
if (VD->hasStorage()) {
2162-
// Note: Stored properties in protocols are diagnosed in
2163-
// finishProtocolStorageImplInfo().
2164-
2165-
// Enums and extensions cannot have stored instance properties.
2166-
// Static stored properties are allowed, with restrictions
2167-
// enforced below.
2168-
if (isa<EnumDecl>(VD->getDeclContext()) &&
2169-
!VD->isStatic() && !VD->isInvalid()) {
2170-
// Enums can only have computed properties.
2171-
TC.diagnose(VD->getLoc(), diag::enum_stored_property);
2172-
VD->markInvalid();
2173-
} else if (isa<ExtensionDecl>(VD->getDeclContext()) &&
2174-
!VD->isStatic() && !VD->isInvalid() &&
2175-
!VD->getAttrs().getAttribute<DynamicReplacementAttr>()) {
2176-
TC.diagnose(VD->getLoc(), diag::extension_stored_property);
2177-
VD->markInvalid();
2178-
}
2179-
2162+
// Note: Stored properties in protocols, enums, etc are diagnosed in
2163+
// finishStorageImplInfo().
2164+
21802165
// We haven't implemented type-level storage in some contexts.
21812166
if (VD->isStatic()) {
21822167
auto PBD = VD->getParentPatternBinding();
@@ -2197,13 +2182,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
21972182

21982183
auto DC = VD->getDeclContext();
21992184

2200-
// Non-stored properties are fine.
2201-
if (!PBD->hasStorage()) {
2202-
// do nothing
2203-
22042185
// Stored type variables in a generic context need to logically
22052186
// occur once per instantiation, which we don't yet handle.
2206-
} else if (DC->getExtendedProtocolDecl()) {
2187+
if (DC->getExtendedProtocolDecl()) {
22072188
unimplementedStatic(ProtocolExtensions);
22082189
} else if (DC->isGenericContext()
22092190
&& !DC->getGenericSignatureOfContext()->areAllParamsConcrete()) {

lib/Sema/TypeCheckStorage.cpp

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2567,6 +2567,8 @@ static void finishNSManagedImplInfo(VarDecl *var,
25672567

25682568
static void finishStorageImplInfo(AbstractStorageDecl *storage,
25692569
StorageImplInfo &info) {
2570+
auto dc = storage->getDeclContext();
2571+
25702572
if (auto var = dyn_cast<VarDecl>(storage)) {
25712573
if (!info.hasStorage()) {
25722574
if (auto *init = var->getParentInitializer()) {
@@ -2585,8 +2587,28 @@ static void finishStorageImplInfo(AbstractStorageDecl *storage,
25852587
}
25862588
}
25872589

2588-
if (isa<ProtocolDecl>(storage->getDeclContext()))
2590+
if (isa<ProtocolDecl>(dc))
25892591
finishProtocolStorageImplInfo(storage, info);
2592+
2593+
// If we have a stored property in an unsupported context, diagnose
2594+
// and change it to computed to avoid confusing SILGen.
2595+
2596+
// Note: Stored properties in protocols are diagnosed in
2597+
// finishProtocolStorageImplInfo().
2598+
2599+
if (info.hasStorage() && !storage->isStatic()) {
2600+
if (isa<EnumDecl>(dc)) {
2601+
storage->diagnose(diag::enum_stored_property);
2602+
info = StorageImplInfo::getMutableComputed();
2603+
} else if (isa<ExtensionDecl>(dc) &&
2604+
!storage->getAttrs().getAttribute<DynamicReplacementAttr>()) {
2605+
storage->diagnose(diag::extension_stored_property);
2606+
2607+
info = (info.supportsMutation()
2608+
? StorageImplInfo::getMutableComputed()
2609+
: StorageImplInfo::getImmutableComputed());
2610+
}
2611+
}
25902612
}
25912613

25922614
/// Gets the storage info of the provided storage decl if it has the
@@ -2634,6 +2656,13 @@ static StorageImplInfo classifyWithHasStorageAttr(VarDecl *var) {
26342656
llvm::Expected<StorageImplInfo>
26352657
StorageImplInfoRequest::evaluate(Evaluator &evaluator,
26362658
AbstractStorageDecl *storage) const {
2659+
if (auto *param = dyn_cast<ParamDecl>(storage)) {
2660+
return StorageImplInfo::getSimpleStored(
2661+
param->isInOut()
2662+
? StorageIsMutable
2663+
: StorageIsNotMutable);
2664+
}
2665+
26372666
if (auto *var = dyn_cast<VarDecl>(storage)) {
26382667
// Allow the @_hasStorage attribute to override all the accessors we parsed
26392668
// when making the final classification.

test/NameBinding/name_lookup.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ class ThisDerived1 : ThisBase1 {
239239

240240
self.baseExtProp = 42 // expected-error {{member 'baseExtProp' cannot be used on type 'ThisDerived1'}}
241241
self.baseExtFunc0() // expected-error {{instance member 'baseExtFunc0' cannot be used on type 'ThisDerived1'}}
242-
self.baseExtStaticVar = 42
242+
self.baseExtStaticVar = 42 // expected-error {{instance member 'baseExtStaticVar' cannot be used on type 'ThisDerived1'}}
243243
self.baseExtStaticProp = 42 // expected-error {{member 'baseExtStaticProp' cannot be used on type 'ThisDerived1'}}
244244
self.baseExtStaticFunc0()
245245

@@ -264,7 +264,7 @@ class ThisDerived1 : ThisBase1 {
264264

265265
self.derivedExtProp = 42 // expected-error {{member 'derivedExtProp' cannot be used on type 'ThisDerived1'}}
266266
self.derivedExtFunc0() // expected-error {{instance member 'derivedExtFunc0' cannot be used on type 'ThisDerived1'}}
267-
self.derivedExtStaticVar = 42
267+
self.derivedExtStaticVar = 42 // expected-error {{instance member 'derivedExtStaticVar' cannot be used on type 'ThisDerived1'}}
268268
self.derivedExtStaticProp = 42 // expected-error {{member 'derivedExtStaticProp' cannot be used on type 'ThisDerived1'}}
269269
self.derivedExtStaticFunc0()
270270

@@ -305,7 +305,7 @@ class ThisDerived1 : ThisBase1 {
305305

306306
super.baseExtProp = 42 // expected-error {{member 'baseExtProp' cannot be used on type 'ThisBase1'}}
307307
super.baseExtFunc0() // expected-error {{instance member 'baseExtFunc0' cannot be used on type 'ThisBase1'}}
308-
super.baseExtStaticVar = 42
308+
super.baseExtStaticVar = 42 // expected-error {{instance member 'baseExtStaticVar' cannot be used on type 'ThisBase1'}}
309309
super.baseExtStaticProp = 42 // expected-error {{member 'baseExtStaticProp' cannot be used on type 'ThisBase1'}}
310310
super.baseExtStaticFunc0()
311311

0 commit comments

Comments
 (0)