Skip to content

[Sema] Uninhabited stored property downgrade to warning #20211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1371,15 +1371,6 @@ ERROR(pattern_binds_no_variables,none,
"variables",
(unsigned))

ERROR(pattern_no_uninhabited_type,none,
"%select{%select{variable|constant}0|stored property}1 %2 cannot have "
"enum type %3 with no cases",
(bool, bool, Identifier, Type))
ERROR(pattern_no_uninhabited_tuple_type,none,
"%select{%select{variable|constant}0|stored property}1 %2 cannot have "
"tuple type %3 containing enum with no cases",
(bool, bool, Identifier, Type))

ERROR(nscoding_unstable_mangled_name,none,
"%select{private|fileprivate|nested|local}0 class %1 has an "
"unstable name when archiving via 'NSCoding'",
Expand Down Expand Up @@ -3312,6 +3303,12 @@ ERROR(ambiguous_enum_pattern_type,none,
WARNING(type_inferred_to_undesirable_type,none,
"%select{variable|constant}2 %0 inferred to have type %1, "
"which may be unexpected", (Identifier, Type, bool))
WARNING(type_inferred_to_uninhabited_type,none,
"%select{variable|constant}2 %0 inferred to have type %1, "
"which is an enum with no cases", (Identifier, Type, bool))
WARNING(type_inferred_to_uninhabited_tuple_type,none,
"%select{variable|constant}2 %0 inferred to have type %1, "
"which contains an enum with no cases", (Identifier, Type, bool))
NOTE(add_explicit_type_annotation_to_silence,none,
"add an explicit type annotation to silence this warning", ())

Expand Down
19 changes: 0 additions & 19 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2541,25 +2541,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
}
}

// Reject variable if it is a stored property with an uninhabited type
if (VD->hasStorage() &&
VD->getInterfaceType()->isStructurallyUninhabited()) {
auto uninhabitedTypeDiag = diag::pattern_no_uninhabited_type;

if (VD->getInterfaceType()->is<TupleType>()) {
uninhabitedTypeDiag = diag::pattern_no_uninhabited_tuple_type;
} else {
assert((VD->getInterfaceType()->is<EnumType>() ||
VD->getInterfaceType()->is<BoundGenericEnumType>()) &&
"unknown structurally uninhabited type");
}

TC.diagnose(VD->getLoc(), uninhabitedTypeDiag, VD->isLet(),
VD->isInstanceMember(), VD->getName(),
VD->getInterfaceType());
VD->markInvalid();
}

if (!checkOverrides(VD)) {
// If a property has an override attribute but does not override
// anything, complain.
Expand Down
26 changes: 19 additions & 7 deletions lib/Sema/TypeCheckPattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1091,14 +1091,16 @@ bool TypeChecker::coercePatternToType(Pattern *&P, TypeResolution resolution,
var->getTypeLoc().setType(var->getType());

// If we are inferring a variable to have type AnyObject.Type,
// "()", or optional thereof, emit a diagnostic. In the first 2 cases, the
// coder probably forgot a cast and expected a concrete type. In the later
// case, they probably didn't mean to bind to a variable, or there is some
// other bug. We always tell them that they can silence the warning with an
// explicit type annotation (and provide a fixit) as a note.
// "()", an uninhabited type, or optional thereof, emit a diagnostic.
// In the first 2 cases, the coder probably forgot a cast and expected a
// concrete type. In the later case, they probably didn't mean to bind to
// a variable, or there is some other bug. We always tell them that they
// can silence the warning with an explicit type annotation
// (and provide a fixit) as a note.
Type diagTy = type->getOptionalObjectType();
if (!diagTy) diagTy = type;

auto diag = diag::type_inferred_to_undesirable_type;
bool shouldRequireType = false;
if (NP->isImplicit()) {
// If the whole pattern is implicit, the user didn't write it.
Expand All @@ -1108,14 +1110,24 @@ bool TypeChecker::coercePatternToType(Pattern *&P, TypeResolution resolution,
} else if (auto MTT = diagTy->getAs<AnyMetatypeType>()) {
if (MTT->getInstanceType()->isAnyObject())
shouldRequireType = true;
} else if (diagTy->isStructurallyUninhabited()) {
shouldRequireType = true;
diag = diag::type_inferred_to_uninhabited_type;

if (diagTy->is<TupleType>()) {
diag = diag::type_inferred_to_uninhabited_tuple_type;
} else {
assert((diagTy->is<EnumType>() || diagTy->is<BoundGenericEnumType>()) &&
"unknown structurally uninhabited type");
}
}

if (shouldRequireType &&
!options.is(TypeResolverContext::ForEachStmt) &&
!options.is(TypeResolverContext::EditorPlaceholderExpr) &&
!(options & TypeResolutionFlags::FromNonInferredPattern)) {
diagnose(NP->getLoc(), diag::type_inferred_to_undesirable_type,
NP->getDecl()->getName(), type, NP->getDecl()->isLet());
diagnose(NP->getLoc(), diag, NP->getDecl()->getName(), type,
NP->getDecl()->isLet());
diagnose(NP->getLoc(), diag::add_explicit_type_annotation_to_silence);
}

Expand Down
4 changes: 1 addition & 3 deletions test/IRGen/generic_enums.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,4 @@ struct Bar<A1, A2> {
var b: Foo<A1>
}

enum Foo<A>{
case bar
}
enum Foo<A> {}
8 changes: 1 addition & 7 deletions test/Reflection/Inputs/TypeLowering.swift
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,8 @@ public struct MetatypeStruct {
public let abstractMetatype: MetadataHolder<BasicStruct.Type, BasicStruct>
}

// We don't allow stored properties to have uninhabited types now, but make a
// wrapper over one to continue testing this
public enum EmptyEnum {}

public struct EmptyEnumWrapper<T> {
public var value: T
}

public enum NoPayloadEnum {
case A
case B
Expand Down Expand Up @@ -224,7 +218,7 @@ public enum MultiPayloadGenericDynamic<T, U> {
}

public struct EnumStruct {
public let empty: EmptyEnumWrapper<EmptyEnum>
public let empty: EmptyEnum
public let noPayload: NoPayloadEnum
public let sillyNoPayload: SillyNoPayloadEnum
public let singleton: SingletonEnum
Expand Down
4 changes: 1 addition & 3 deletions test/Reflection/typeref_lowering.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1032,9 +1032,7 @@
// CHECK-64: (struct TypeLowering.EnumStruct)
// CHECK-64-NEXT: (struct size=81 alignment=8 stride=88 num_extra_inhabitants=[[PTR_XI]] bitwise_takable=1
// CHECK-64-NEXT: (field name=empty offset=0
// CHECK-64-NEXT: (struct size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64-NEXT: (field name=value offset=0
// CHECK-64-NEXT: (no_payload_enum size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))))
// CHECK-64-NEXT: (no_payload_enum size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))
// CHECK-64-NEXT: (field name=noPayload offset=0
// CHECK-64-NEXT: (no_payload_enum size=1 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))
// CHECK-64-NEXT: (field name=sillyNoPayload offset=1
Expand Down
3 changes: 1 addition & 2 deletions test/attr/attr_noreturn.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ func noReturn3(_: Int)
// expected-error@-2 {{'@noreturn' has been removed; functions that never return should have a return type of 'Never' instead}}{{1-11=}}{{53-56=Never}}

// Test that error recovery gives us the 'Never' return type
let x: Never = noReturn1(0)
// expected-error@-1 {{constant 'x' cannot have enum type 'Never' with no cases}}
let x: Never = noReturn1(0) // No error

// @noreturn in function type declarations
let valueNoReturn: @noreturn () -> ()
Expand Down
4 changes: 1 addition & 3 deletions test/attr/attr_objc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -941,12 +941,11 @@ class infer_instanceVar1 {
// expected-note@-2 {{Swift structs cannot be represented in Objective-C}}

var var_PlainEnum: PlainEnum
// expected-error@-1 {{stored property 'var_PlainEnum' cannot have enum type 'PlainEnum' with no cases}}
// CHECK-LABEL: {{^}} var var_PlainEnum: PlainEnum

@objc var var_PlainEnum_: PlainEnum
// expected-error@-1 {{property cannot be marked @objc because its type cannot be represented in Objective-C}}
// expected-note@-2 {{non-'@objc' enums cannot be represented in Objective-C}}
// expected-error@-3 {{stored property 'var_PlainEnum_' cannot have enum type 'PlainEnum' with no cases}}

var var_PlainProtocol: PlainProtocol
// CHECK-LABEL: {{^}} var var_PlainProtocol: PlainProtocol
Expand Down Expand Up @@ -1272,7 +1271,6 @@ class infer_instanceVar1 {
// expected-error@-1 {{'unowned' may only be applied to class and class-bound protocol types, not 'PlainStruct'}}
unowned var var_Unowned_bad3: PlainEnum
// expected-error@-1 {{'unowned' may only be applied to class and class-bound protocol types, not 'PlainEnum'}}
// expected-error@-2 {{stored property 'var_Unowned_bad3' cannot have enum type 'PlainEnum' with no cases}}
unowned var var_Unowned_bad4: String
// expected-error@-1 {{'unowned' may only be applied to class and class-bound protocol types, not 'String'}}
// CHECK-NOT: @objc{{.*}}Unowned_fail
Expand Down
22 changes: 5 additions & 17 deletions test/decl/var/properties.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1268,24 +1268,12 @@ class WeakFixItTest {
weak var bar : WFI_P1 & WFI_P2
}

// SR-8811
// Stored properties cannot have uninhabited types
// SR-8811 (Warning)

struct SR8811 {
var x: Never // expected-error {{stored property 'x' cannot have enum type 'Never' with no cases}}

var y: (Int, Never, Bool) // expected-error {{stored property 'y' cannot have tuple type '(Int, Never, Bool)' containing enum with no cases}}
}

let sr8811x: Never // expected-error {{constant 'sr8811x' cannot have enum type 'Never' with no cases}}
let sr8811a = fatalError() // expected-warning {{constant 'sr8811a' inferred to have type 'Never', which is an enum with no cases}} expected-note {{add an explicit type annotation to silence this warning}}

var sr8811y: (Int, Never) // expected-error {{variable 'sr8811y' cannot have tuple type '(Int, Never)' containing enum with no cases}}

// Ok
var sr8811z: Never {
return fatalError()
}
let sr8811b: Never = fatalError() // Ok

enum SR8811EmptyGenericEnum<A> {}
let sr8811c = (16, fatalError()) // expected-warning {{constant 'sr8811c' inferred to have type '(Int, Never)', which contains an enum with no cases}} expected-note {{add an explicit type annotation to silence this warning}}

let sr8811z: SR8811EmptyGenericEnum<Int> // expected-error {{constant 'sr8811z' cannot have enum type 'SR8811EmptyGenericEnum<Int>' with no cases}}
let sr8811d: (Int, Never) = (16, fatalError()) // Ok
4 changes: 2 additions & 2 deletions test/expr/expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -584,9 +584,9 @@ func conversionTest(_ a: inout Double, b: inout Int) {
var pi_f3 = float.init(getPi()) // expected-error {{ambiguous use of 'init(_:)'}}
var pi_f4 = float.init(pi_f)

var e = Empty(f) // expected-error {{variable 'e' cannot have enum type 'Empty' with no cases}}
var e = Empty(f) // expected-warning {{variable 'e' inferred to have type 'Empty', which is an enum with no cases}} expected-note {{add an explicit type annotation to silence this warning}}
var e2 = Empty(d) // expected-error{{cannot convert value of type 'Double' to expected argument type 'Float'}}
var e3 = Empty(Float(d)) // expected-error {{variable 'e3' cannot have enum type 'Empty' with no cases}}
var e3 = Empty(Float(d)) // expected-warning {{variable 'e3' inferred to have type 'Empty', which is an enum with no cases}} expected-note {{add an explicit type annotation to silence this warning}}
}

struct Rule { // expected-note {{'init(target:dependencies:)' declared here}}
Expand Down