From 1a5a47f54075622db83a3cb694bc074d481669f7 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Thu, 5 Sep 2019 19:25:19 -0500 Subject: [PATCH 01/26] warn on empty OptionSet static constants --- include/swift/AST/DiagnosticsSema.def | 4 ++++ lib/Sema/TypeCheckProtocol.cpp | 27 +++++++++++++++++++++++++++ test/Sema/option-set-empty.swift | 4 ++++ 3 files changed, 35 insertions(+) create mode 100644 test/Sema/option-set-empty.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index ab57e404b8f9..8df9d3f6d673 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4423,6 +4423,10 @@ ERROR(property_delegate_type_not_usable_from_inline,none, "must be '@usableFromInline' or public", (bool, bool)) +WARNING(option_set_zero_constant,none, + "'static let' constant inside %0 that conforms to OptionSet produces an empty option set", + (Type)) + #ifndef DIAG_NO_UNDEF # if defined(DIAG) # undef DIAG diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index b2bd8e8e99a3..01dbbc842e99 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -4959,6 +4959,33 @@ void TypeChecker::checkConformancesInContext(DeclContext *dc, diagnoseMissingAppendInterpolationMethod(*this, typeDecl); } } + + if (conformance->getProtocol()-> + isSpecificProtocol(KnownProtocolKind::OptionSet)) { + for (auto member : idc->getMembers()) { + if (auto pbd = dyn_cast(member)) { + if (pbd->isStatic()) + for (auto entry : pbd->getPatternList()) { + if (auto ctor = dyn_cast(entry.getInit())) { + auto type = ctor->getType(); + auto argLabels = ctor->getArgumentLabels(); + auto *args = cast(ctor->getArg()); + if (type->isEqual(dc->getSelfTypeInContext())) { + if (argLabels[0].is(StringRef("rawValue"))) { + if (auto i = dyn_cast(args->getElement(0))) { + auto a = i->getValue(); + if (a == 0) { + auto loc = member->getLoc(); + diagnose(loc, diag::option_set_zero_constant, type); + } + } + } + } + } + } + } + } + } } // Check all conformances. diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift new file mode 100644 index 000000000000..8f23b42e155e --- /dev/null +++ b/test/Sema/option-set-empty.swift @@ -0,0 +1,4 @@ +struct MyOptions: OptionSet { + var rawValue: Int + static let none = MyOptions(rawValue: 0) // expected-warning {{'static let' constant inside 'MyOptions' that conforms to OptionSet produces an empty option set}} +} From acb99fb79edbecedee85fa37f25ef4584b1971ea Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Thu, 12 Sep 2019 16:23:15 -0500 Subject: [PATCH 02/26] refactor --- lib/Sema/TypeCheckDecl.cpp | 38 ++++++++++++++++++++++++++++++++++ lib/Sema/TypeCheckProtocol.cpp | 27 ------------------------ 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 82d97cb8c809..558f4ebdbaa6 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2360,6 +2360,8 @@ class DeclChecker : public DeclVisitor { TC.checkDeclAttributes(VD); + checkForEmptyOptionSet(VD); + triggerAccessorSynthesis(TC, VD); // Under the Swift 3 inference rules, if we have @IBInspectable or @@ -2383,6 +2385,42 @@ class DeclChecker : public DeclVisitor { if (VD->getAttrs().hasAttribute()) TC.checkDynamicReplacementAttribute(VD); } + + void checkForEmptyOptionSet(VarDecl *VD) { + if (!VD->isStatic()) + return; + auto DC = VD->getDeclContext(); + auto protocols = DC->getLocalProtocols(); + auto conformsToOptionSet = false; + for (auto protocol : protocols) { + if (protocol->isSpecificProtocol(KnownProtocolKind::OptionSet)) { + conformsToOptionSet = true; + break; + } + } + if (!conformsToOptionSet) + return; + auto type = VD->getType(); + if (!type->isEqual(DC->getSelfTypeInContext())) + return; + if (!VD->getParentPatternBinding()) + return; + auto PBD = VD->getParentPatternBinding(); + for (auto entry : PBD->getPatternList()) { + auto ctor = dyn_cast(entry.getInit()); + if (!ctor) continue; + auto argLabels = ctor->getArgumentLabels(); + if (!argLabels.front().is(StringRef("rawValue"))) continue; + auto *args = cast(ctor->getArg()); + auto intArg = dyn_cast(args->getElement(0)); + if (!intArg) continue; + auto val = intArg->getValue(); + if (val == 0) { + auto loc = VD->getLoc(); + TC.diagnose(loc, diag::option_set_zero_constant, type); + } + } + } void visitBoundVars(Pattern *P) { P->forEachVariable([&] (VarDecl *VD) { this->visitBoundVariable(VD); }); diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 01dbbc842e99..b2bd8e8e99a3 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -4959,33 +4959,6 @@ void TypeChecker::checkConformancesInContext(DeclContext *dc, diagnoseMissingAppendInterpolationMethod(*this, typeDecl); } } - - if (conformance->getProtocol()-> - isSpecificProtocol(KnownProtocolKind::OptionSet)) { - for (auto member : idc->getMembers()) { - if (auto pbd = dyn_cast(member)) { - if (pbd->isStatic()) - for (auto entry : pbd->getPatternList()) { - if (auto ctor = dyn_cast(entry.getInit())) { - auto type = ctor->getType(); - auto argLabels = ctor->getArgumentLabels(); - auto *args = cast(ctor->getArg()); - if (type->isEqual(dc->getSelfTypeInContext())) { - if (argLabels[0].is(StringRef("rawValue"))) { - if (auto i = dyn_cast(args->getElement(0))) { - auto a = i->getValue(); - if (a == 0) { - auto loc = member->getLoc(); - diagnose(loc, diag::option_set_zero_constant, type); - } - } - } - } - } - } - } - } - } } // Check all conformances. From 9c9a327f93cbfa0ada84eea02d73c6fea52b46d1 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Thu, 12 Sep 2019 16:28:12 -0500 Subject: [PATCH 03/26] cleanup --- lib/Sema/TypeCheckDecl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 558f4ebdbaa6..98dd7a298d2c 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2403,9 +2403,9 @@ class DeclChecker : public DeclVisitor { auto type = VD->getType(); if (!type->isEqual(DC->getSelfTypeInContext())) return; - if (!VD->getParentPatternBinding()) - return; auto PBD = VD->getParentPatternBinding(); + if (!PBD) + return; for (auto entry : PBD->getPatternList()) { auto ctor = dyn_cast(entry.getInit()); if (!ctor) continue; From 6152f8e1f9935ea8370a7470a2003fe78ee9496d Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Wed, 18 Sep 2019 07:58:27 -0500 Subject: [PATCH 04/26] feedback --- include/swift/AST/DiagnosticsSema.def | 6 +- lib/Sema/TypeCheckDecl.cpp | 86 +++++++++++++++------------ 2 files changed, 52 insertions(+), 40 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 8df9d3f6d673..db231687844a 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4424,8 +4424,10 @@ ERROR(property_delegate_type_not_usable_from_inline,none, (bool, bool)) WARNING(option_set_zero_constant,none, - "'static let' constant inside %0 that conforms to OptionSet produces an empty option set", - (Type)) + "%0 %1 produces an empty option set", + (DescriptiveDeclKind, Identifier)) +NOTE(option_set_empty_set_init,none, + "use [] to silence this warning", ()) #ifndef DIAG_NO_UNDEF # if defined(DIAG) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 98dd7a298d2c..d1ac98387106 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -478,6 +478,52 @@ static void checkInheritanceClause( } } +// Check for static properties that produce empty option sets +static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { + if (!VD->isStatic() || !VD->isLet()) + return; + + auto DC = VD->getDeclContext(); + + auto *optionSetProto = tc.Context.getProtocol(KnownProtocolKind::OptionSet); + auto protocolConformance = tc.containsProtocol(DC->getSelfTypeInContext(), optionSetProto, DC, /*Flags*/None); + if (!protocolConformance) + return; + + auto type = VD->getType(); + if (!type->isEqual(DC->getSelfTypeInContext())) + return; + + auto PBD = VD->getParentPatternBinding(); + if (!PBD) + return; + + for (auto entry : PBD->getPatternList()) { + if (entry.getPattern()->getSingleVar() != VD) continue; + + auto ctor = dyn_cast(entry.getInit()); + if (!ctor) continue; + if (!isa(ctor->getCalledValue())) continue; + + if (ctor->getNumArguments() != 1) continue; + auto argLabels = ctor->getArgumentLabels(); + if (argLabels.front() != tc.Context.Id_rawValue) continue; + + auto *args = cast(ctor->getArg()); + auto intArg = dyn_cast(args->getElement(0)); + if (!intArg) continue; + + auto val = intArg->getValue(); + if (val != 0) continue; + + auto loc = VD->getLoc(); + tc.diagnose(loc, diag::option_set_zero_constant, DescriptiveDeclKind::StaticProperty, VD->getName()); + auto range = ctor->getArg()->getSourceRange(); + tc.diagnose(loc, diag::option_set_empty_set_init).fixItReplace(range, "([])"); + } +} + + /// Check the inheritance clauses generic parameters along with any /// requirements stored within the generic parameter list. static void checkGenericParams(GenericParamList *genericParams, @@ -2360,7 +2406,7 @@ class DeclChecker : public DeclVisitor { TC.checkDeclAttributes(VD); - checkForEmptyOptionSet(VD); + checkForEmptyOptionSet(VD, TC); triggerAccessorSynthesis(TC, VD); @@ -2385,43 +2431,7 @@ class DeclChecker : public DeclVisitor { if (VD->getAttrs().hasAttribute()) TC.checkDynamicReplacementAttribute(VD); } - - void checkForEmptyOptionSet(VarDecl *VD) { - if (!VD->isStatic()) - return; - auto DC = VD->getDeclContext(); - auto protocols = DC->getLocalProtocols(); - auto conformsToOptionSet = false; - for (auto protocol : protocols) { - if (protocol->isSpecificProtocol(KnownProtocolKind::OptionSet)) { - conformsToOptionSet = true; - break; - } - } - if (!conformsToOptionSet) - return; - auto type = VD->getType(); - if (!type->isEqual(DC->getSelfTypeInContext())) - return; - auto PBD = VD->getParentPatternBinding(); - if (!PBD) - return; - for (auto entry : PBD->getPatternList()) { - auto ctor = dyn_cast(entry.getInit()); - if (!ctor) continue; - auto argLabels = ctor->getArgumentLabels(); - if (!argLabels.front().is(StringRef("rawValue"))) continue; - auto *args = cast(ctor->getArg()); - auto intArg = dyn_cast(args->getElement(0)); - if (!intArg) continue; - auto val = intArg->getValue(); - if (val == 0) { - auto loc = VD->getLoc(); - TC.diagnose(loc, diag::option_set_zero_constant, type); - } - } - } - + void visitBoundVars(Pattern *P) { P->forEachVariable([&] (VarDecl *VD) { this->visitBoundVariable(VD); }); } From 5ae04840b013c431afb55654f1ce87cf533c7749 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Wed, 18 Sep 2019 11:17:28 -0500 Subject: [PATCH 05/26] inline variables --- lib/Sema/TypeCheckDecl.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index d1ac98387106..88024463df6b 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -490,8 +490,7 @@ static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { if (!protocolConformance) return; - auto type = VD->getType(); - if (!type->isEqual(DC->getSelfTypeInContext())) + if (!VD->getType()->isEqual(DC->getSelfTypeInContext())) return; auto PBD = VD->getParentPatternBinding(); @@ -506,20 +505,16 @@ static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { if (!isa(ctor->getCalledValue())) continue; if (ctor->getNumArguments() != 1) continue; - auto argLabels = ctor->getArgumentLabels(); - if (argLabels.front() != tc.Context.Id_rawValue) continue; + if (ctor->getArgumentLabels().front() != tc.Context.Id_rawValue) continue; auto *args = cast(ctor->getArg()); auto intArg = dyn_cast(args->getElement(0)); if (!intArg) continue; - - auto val = intArg->getValue(); - if (val != 0) continue; + if (intArg->getValue() != 0) continue; auto loc = VD->getLoc(); - tc.diagnose(loc, diag::option_set_zero_constant, DescriptiveDeclKind::StaticProperty, VD->getName()); - auto range = ctor->getArg()->getSourceRange(); - tc.diagnose(loc, diag::option_set_empty_set_init).fixItReplace(range, "([])"); + tc.diagnose(loc, diag::option_set_zero_constant, VD->getName()); + tc.diagnose(loc, diag::option_set_empty_set_init).fixItReplace(args->getSourceRange(), "([])"); } } From 76a8fa6efed5ba39eb06ae531d6067ccb8ebf2d6 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Wed, 18 Sep 2019 11:17:38 -0500 Subject: [PATCH 06/26] simplify diagnostic --- include/swift/AST/DiagnosticsSema.def | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index db231687844a..907facbfe068 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4424,8 +4424,8 @@ ERROR(property_delegate_type_not_usable_from_inline,none, (bool, bool)) WARNING(option_set_zero_constant,none, - "%0 %1 produces an empty option set", - (DescriptiveDeclKind, Identifier)) + "static property %0 produces an empty option set", + (Identifier)) NOTE(option_set_empty_set_init,none, "use [] to silence this warning", ()) From a665524709887164ebe6aed5981306fad7716627 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Wed, 18 Sep 2019 11:17:45 -0500 Subject: [PATCH 07/26] fix test --- test/Sema/option-set-empty.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index 8f23b42e155e..bfd347390e81 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -1,4 +1,6 @@ +// RUN: %target-typecheck-verify-swift + struct MyOptions: OptionSet { var rawValue: Int - static let none = MyOptions(rawValue: 0) // expected-warning {{'static let' constant inside 'MyOptions' that conforms to OptionSet produces an empty option set}} + static let none = MyOptions(rawValue: 0) // expected-warning {{static property 'none' produces an empty option set}} expected-note {{use [] to silence this warning}}{{32-45=([])}} } From 1ba47ff13bf36b8ec6d4f682c6bc5b0c25c1509b Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Wed, 18 Sep 2019 11:24:14 -0500 Subject: [PATCH 08/26] cleanup --- lib/Sema/TypeCheckDecl.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index a999cbfb838b..f905fd33341a 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2244,7 +2244,6 @@ class DeclChecker : public DeclVisitor { if (VD->getDeclContext()->getSelfClassDecl()) { checkDynamicSelfType(VD, VD->getValueInterfaceType()); - if (VD->getValueInterfaceType()->hasDynamicSelfType()) { if (VD->hasStorage()) VD->diagnose(diag::dynamic_self_in_stored_property); @@ -2278,7 +2277,7 @@ class DeclChecker : public DeclVisitor { visit(accessor); }); } - + void visitBoundVars(Pattern *P) { P->forEachVariable([&] (VarDecl *VD) { this->visitBoundVariable(VD); }); } From cc0afc06a9bb802ccb49fdb062a6f6ce12e4a84d Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Wed, 18 Sep 2019 12:12:41 -0500 Subject: [PATCH 09/26] feedback --- include/swift/AST/DiagnosticsSema.def | 12 ++++++------ lib/Sema/TypeCheckDecl.cpp | 16 +++++++++++----- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 907facbfe068..878a262c5a64 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1317,6 +1317,12 @@ ERROR(corresponding_param_not_defaulted,none, NOTE(inherited_default_param_here,none, "corresponding parameter declared here", ()) +WARNING(option_set_zero_constant,none, + "static property %0 produces an empty option set", + (Identifier)) +NOTE(option_set_empty_set_init,none, + "use [] to silence this warning", ()) + // Alignment attribute ERROR(alignment_not_power_of_two,none, "alignment value must be a power of two", ()) @@ -4423,12 +4429,6 @@ ERROR(property_delegate_type_not_usable_from_inline,none, "must be '@usableFromInline' or public", (bool, bool)) -WARNING(option_set_zero_constant,none, - "static property %0 produces an empty option set", - (Identifier)) -NOTE(option_set_empty_set_init,none, - "use [] to silence this warning", ()) - #ifndef DIAG_NO_UNDEF # if defined(DIAG) # undef DIAG diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 88024463df6b..de3d58443fad 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -485,12 +485,17 @@ static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { auto DC = VD->getDeclContext(); - auto *optionSetProto = tc.Context.getProtocol(KnownProtocolKind::OptionSet); - auto protocolConformance = tc.containsProtocol(DC->getSelfTypeInContext(), optionSetProto, DC, /*Flags*/None); - if (!protocolConformance) + if (!VD->getType()->isEqual(DC->getSelfTypeInContext())) return; - if (!VD->getType()->isEqual(DC->getSelfTypeInContext())) + auto *optionSetProto = tc.Context.getProtocol(KnownProtocolKind::OptionSet); + auto protocolConformance = !tc.containsProtocol( + DC->getSelfTypeInContext(), + optionSetProto, + DC, + /*Flags*/None); + + if (!protocolConformance) return; auto PBD = VD->getParentPatternBinding(); @@ -514,7 +519,8 @@ static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { auto loc = VD->getLoc(); tc.diagnose(loc, diag::option_set_zero_constant, VD->getName()); - tc.diagnose(loc, diag::option_set_empty_set_init).fixItReplace(args->getSourceRange(), "([])"); + tc.diagnose(loc, diag::option_set_empty_set_init) + .fixItReplace(args->getSourceRange(), "([])"); } } From 2104984cfd041f19b46f2920afa95da820b6065f Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Sat, 21 Sep 2019 17:03:45 -0500 Subject: [PATCH 10/26] more unit test cases --- test/Sema/option-set-empty.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index bfd347390e81..11e0ec625243 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -1,6 +1,17 @@ // RUN: %target-typecheck-verify-swift +struct SomeOptions: OptionSet { + var rawValue: Int + static let some = MyOptions(rawValue: 4) +} + struct MyOptions: OptionSet { + init() { + rawValue = 0 + } var rawValue: Int static let none = MyOptions(rawValue: 0) // expected-warning {{static property 'none' produces an empty option set}} expected-note {{use [] to silence this warning}}{{32-45=([])}} + static var nothing = MyOptions(rawValue: 0) + static let nope = MyOptions() + static let other = SomeOptions(rawValue: 8) } From bc308e0b24bd6582a6bb304256f4bf63b781081c Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Sat, 21 Sep 2019 17:03:52 -0500 Subject: [PATCH 11/26] add comments --- lib/Sema/TypeCheckDecl.cpp | 57 +++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 6bca59bc8855..d2e1cc2d490d 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -483,49 +483,60 @@ static void checkInheritanceClause( } // Check for static properties that produce empty option sets +// using a rawValue initializer with a value of '0' static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { + // Check if property is a 'static let' if (!VD->isStatic() || !VD->isLet()) return; auto DC = VD->getDeclContext(); + // Make sure property is of same type as the type it is declared in if (!VD->getType()->isEqual(DC->getSelfTypeInContext())) return; + // Make sure this type conforms to OptionSet auto *optionSetProto = tc.Context.getProtocol(KnownProtocolKind::OptionSet); - auto protocolConformance = !tc.containsProtocol( + bool conformsToOptionSet = tc.containsProtocol( DC->getSelfTypeInContext(), optionSetProto, DC, - /*Flags*/None); + /*Flags*/None).hasValue(); - if (!protocolConformance) + if (!conformsToOptionSet) return; auto PBD = VD->getParentPatternBinding(); if (!PBD) return; - for (auto entry : PBD->getPatternList()) { - if (entry.getPattern()->getSingleVar() != VD) continue; - - auto ctor = dyn_cast(entry.getInit()); - if (!ctor) continue; - if (!isa(ctor->getCalledValue())) continue; - - if (ctor->getNumArguments() != 1) continue; - if (ctor->getArgumentLabels().front() != tc.Context.Id_rawValue) continue; - - auto *args = cast(ctor->getArg()); - auto intArg = dyn_cast(args->getElement(0)); - if (!intArg) continue; - if (intArg->getValue() != 0) continue; - - auto loc = VD->getLoc(); - tc.diagnose(loc, diag::option_set_zero_constant, VD->getName()); - tc.diagnose(loc, diag::option_set_empty_set_init) - .fixItReplace(args->getSourceRange(), "([])"); - } + auto entry = PBD->getPatternEntryForVarDecl(VD); + + // Make sure property is being set with a constructor + auto ctor = dyn_cast(entry.getInit()); + if (!ctor) + return; + if (!isa(ctor->getCalledValue())) + return; + + // Make sure it is calling the rawValue constructor + if (ctor->getNumArguments() != 1) + return; + if (ctor->getArgumentLabels().front() != tc.Context.Id_rawValue) + return; + + // Make sure the rawValue parameter is a '0' integer literal + auto *args = cast(ctor->getArg()); + auto intArg = dyn_cast(args->getElement(0)); + if (!intArg) + return; + if (intArg->getValue() != 0) + return; + + auto loc = VD->getLoc(); + tc.diagnose(loc, diag::option_set_zero_constant, VD->getName()); + tc.diagnose(loc, diag::option_set_empty_set_init) + .fixItReplace(args->getSourceRange(), "([])"); } From 06c057136ee8a2b923cbfedef389fda4680c3e97 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 1 Oct 2019 16:16:13 -0500 Subject: [PATCH 12/26] test more cases --- test/Sema/option-set-empty.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index 11e0ec625243..20b01889beaa 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -3,6 +3,8 @@ struct SomeOptions: OptionSet { var rawValue: Int static let some = MyOptions(rawValue: 4) + let empty = SomeOptions(rawValue: 0) + var otherVal = SomeOptions(rawValue: 0) } struct MyOptions: OptionSet { From c6063b217134f5a7493a322c0096aa11a77491bc Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 8 Oct 2019 07:43:28 -0500 Subject: [PATCH 13/26] fix tests --- test/Sema/option-set-empty.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index 20b01889beaa..c217cb72df0b 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -2,16 +2,16 @@ struct SomeOptions: OptionSet { var rawValue: Int + + let someVal = MyOptions(rawValue: 6) static let some = MyOptions(rawValue: 4) - let empty = SomeOptions(rawValue: 0) - var otherVal = SomeOptions(rawValue: 0) + static let empty = SomeOptions(rawValue: 0) // expected-warning {{static property 'empty' produces an empty option set}} expected-note {{use [] to silence this warning}}{{35-48=([])}} + static var otherVal = SomeOptions(rawValue: 0) } struct MyOptions: OptionSet { - init() { - rawValue = 0 - } - var rawValue: Int + let rawValue: Int + static let none = MyOptions(rawValue: 0) // expected-warning {{static property 'none' produces an empty option set}} expected-note {{use [] to silence this warning}}{{32-45=([])}} static var nothing = MyOptions(rawValue: 0) static let nope = MyOptions() From 2d5438d7c13b1bf45158a53143a1352b3d5c7744 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 8 Oct 2019 08:20:51 -0500 Subject: [PATCH 14/26] add more tests --- test/Sema/option-set-empty.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index c217cb72df0b..551fa88b061e 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -7,13 +7,24 @@ struct SomeOptions: OptionSet { static let some = MyOptions(rawValue: 4) static let empty = SomeOptions(rawValue: 0) // expected-warning {{static property 'empty' produces an empty option set}} expected-note {{use [] to silence this warning}}{{35-48=([])}} static var otherVal = SomeOptions(rawValue: 0) + let option = MyOptions(float: Float.infinity) } struct MyOptions: OptionSet { let rawValue: Int + init(rawValue: Int) { + self.rawValue = rawValue + } + + init(float: Float) { + self.rawValue = float.exponent + } + static let none = MyOptions(rawValue: 0) // expected-warning {{static property 'none' produces an empty option set}} expected-note {{use [] to silence this warning}}{{32-45=([])}} static var nothing = MyOptions(rawValue: 0) static let nope = MyOptions() static let other = SomeOptions(rawValue: 8) + static let piVal = MyOptions(float: Float.pi) + static let zero = MyOptions(float: 0.0) } From 4250d7fde06194758dd58bd5336d3160cdb6e20a Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 8 Oct 2019 08:47:51 -0500 Subject: [PATCH 15/26] Revert "add more tests" This reverts commit 2d5438d7c13b1bf45158a53143a1352b3d5c7744. --- test/Sema/option-set-empty.swift | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index 551fa88b061e..c217cb72df0b 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -7,24 +7,13 @@ struct SomeOptions: OptionSet { static let some = MyOptions(rawValue: 4) static let empty = SomeOptions(rawValue: 0) // expected-warning {{static property 'empty' produces an empty option set}} expected-note {{use [] to silence this warning}}{{35-48=([])}} static var otherVal = SomeOptions(rawValue: 0) - let option = MyOptions(float: Float.infinity) } struct MyOptions: OptionSet { let rawValue: Int - init(rawValue: Int) { - self.rawValue = rawValue - } - - init(float: Float) { - self.rawValue = float.exponent - } - static let none = MyOptions(rawValue: 0) // expected-warning {{static property 'none' produces an empty option set}} expected-note {{use [] to silence this warning}}{{32-45=([])}} static var nothing = MyOptions(rawValue: 0) static let nope = MyOptions() static let other = SomeOptions(rawValue: 8) - static let piVal = MyOptions(float: Float.pi) - static let zero = MyOptions(float: 0.0) } From b7e059d4af0201d4b2d870ec9ca8fc77d1553736 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 8 Oct 2019 08:47:55 -0500 Subject: [PATCH 16/26] Revert "fix tests" This reverts commit c6063b217134f5a7493a322c0096aa11a77491bc. --- test/Sema/option-set-empty.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index c217cb72df0b..20b01889beaa 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -2,16 +2,16 @@ struct SomeOptions: OptionSet { var rawValue: Int - - let someVal = MyOptions(rawValue: 6) static let some = MyOptions(rawValue: 4) - static let empty = SomeOptions(rawValue: 0) // expected-warning {{static property 'empty' produces an empty option set}} expected-note {{use [] to silence this warning}}{{35-48=([])}} - static var otherVal = SomeOptions(rawValue: 0) + let empty = SomeOptions(rawValue: 0) + var otherVal = SomeOptions(rawValue: 0) } struct MyOptions: OptionSet { - let rawValue: Int - + init() { + rawValue = 0 + } + var rawValue: Int static let none = MyOptions(rawValue: 0) // expected-warning {{static property 'none' produces an empty option set}} expected-note {{use [] to silence this warning}}{{32-45=([])}} static var nothing = MyOptions(rawValue: 0) static let nope = MyOptions() From f7f908678eb4801a567bc728fd5ca751e9f605d5 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 8 Oct 2019 08:48:19 -0500 Subject: [PATCH 17/26] Revert "test more cases" This reverts commit 06c057136ee8a2b923cbfedef389fda4680c3e97. --- test/Sema/option-set-empty.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index 20b01889beaa..11e0ec625243 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -3,8 +3,6 @@ struct SomeOptions: OptionSet { var rawValue: Int static let some = MyOptions(rawValue: 4) - let empty = SomeOptions(rawValue: 0) - var otherVal = SomeOptions(rawValue: 0) } struct MyOptions: OptionSet { From d70b0a41afaddb6bb7fec4ac932a3f4ac382f89a Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 22 Oct 2019 16:55:26 -0500 Subject: [PATCH 18/26] check for null ctor called value --- lib/Sema/TypeCheckDecl.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index ebef3d17a1a2..ff680974da46 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -518,7 +518,10 @@ static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { auto ctor = dyn_cast(entry.getInit()); if (!ctor) return; - if (!isa(ctor->getCalledValue())) + auto ctorCalledVal = ctor->getCalledValue(); + if (!ctorCalledVal) + return; + if (!isa(ctorCalledVal)) return; // Make sure it is calling the rawValue constructor From b276a7e7cd9ba803fea7654ce7b728b224d4d8dc Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 22 Oct 2019 17:00:18 -0500 Subject: [PATCH 19/26] Revert "Revert "test more cases"" This reverts commit f7f908678eb4801a567bc728fd5ca751e9f605d5. --- test/Sema/option-set-empty.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index 11e0ec625243..20b01889beaa 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -3,6 +3,8 @@ struct SomeOptions: OptionSet { var rawValue: Int static let some = MyOptions(rawValue: 4) + let empty = SomeOptions(rawValue: 0) + var otherVal = SomeOptions(rawValue: 0) } struct MyOptions: OptionSet { From 1da9215a2fe7f504b5a54f8fda721189ebec381f Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 22 Oct 2019 17:00:22 -0500 Subject: [PATCH 20/26] Revert "Revert "fix tests"" This reverts commit b7e059d4af0201d4b2d870ec9ca8fc77d1553736. --- test/Sema/option-set-empty.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index 20b01889beaa..c217cb72df0b 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -2,16 +2,16 @@ struct SomeOptions: OptionSet { var rawValue: Int + + let someVal = MyOptions(rawValue: 6) static let some = MyOptions(rawValue: 4) - let empty = SomeOptions(rawValue: 0) - var otherVal = SomeOptions(rawValue: 0) + static let empty = SomeOptions(rawValue: 0) // expected-warning {{static property 'empty' produces an empty option set}} expected-note {{use [] to silence this warning}}{{35-48=([])}} + static var otherVal = SomeOptions(rawValue: 0) } struct MyOptions: OptionSet { - init() { - rawValue = 0 - } - var rawValue: Int + let rawValue: Int + static let none = MyOptions(rawValue: 0) // expected-warning {{static property 'none' produces an empty option set}} expected-note {{use [] to silence this warning}}{{32-45=([])}} static var nothing = MyOptions(rawValue: 0) static let nope = MyOptions() From 67cd80977d5ed0593df18c9ab50ccb3a560a9bff Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 22 Oct 2019 17:00:25 -0500 Subject: [PATCH 21/26] Revert "Revert "add more tests"" This reverts commit 4250d7fde06194758dd58bd5336d3160cdb6e20a. --- test/Sema/option-set-empty.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index c217cb72df0b..551fa88b061e 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -7,13 +7,24 @@ struct SomeOptions: OptionSet { static let some = MyOptions(rawValue: 4) static let empty = SomeOptions(rawValue: 0) // expected-warning {{static property 'empty' produces an empty option set}} expected-note {{use [] to silence this warning}}{{35-48=([])}} static var otherVal = SomeOptions(rawValue: 0) + let option = MyOptions(float: Float.infinity) } struct MyOptions: OptionSet { let rawValue: Int + init(rawValue: Int) { + self.rawValue = rawValue + } + + init(float: Float) { + self.rawValue = float.exponent + } + static let none = MyOptions(rawValue: 0) // expected-warning {{static property 'none' produces an empty option set}} expected-note {{use [] to silence this warning}}{{32-45=([])}} static var nothing = MyOptions(rawValue: 0) static let nope = MyOptions() static let other = SomeOptions(rawValue: 8) + static let piVal = MyOptions(float: Float.pi) + static let zero = MyOptions(float: 0.0) } From 9cc2d65bb04431bc32f85e3914b449477db196a6 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 22 Oct 2019 17:07:00 -0500 Subject: [PATCH 22/26] refine test --- test/Sema/option-set-empty.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index 551fa88b061e..9d2aa9ba52c0 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -3,11 +3,13 @@ struct SomeOptions: OptionSet { var rawValue: Int - let someVal = MyOptions(rawValue: 6) static let some = MyOptions(rawValue: 4) static let empty = SomeOptions(rawValue: 0) // expected-warning {{static property 'empty' produces an empty option set}} expected-note {{use [] to silence this warning}}{{35-48=([])}} static var otherVal = SomeOptions(rawValue: 0) + + let someVal = MyOptions(rawValue: 6) let option = MyOptions(float: Float.infinity) + let none = MyOptions(rawValue: 0) } struct MyOptions: OptionSet { From 0ba29805854c55de166ec49dcba74f1f1ec51005 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Mon, 28 Oct 2019 17:17:07 -0500 Subject: [PATCH 23/26] use new API --- lib/Sema/TypeCheckDecl.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 8a1f6b1a13e2..736892506bb6 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -512,10 +512,13 @@ static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { if (!PBD) return; - auto entry = PBD->getPatternEntryForVarDecl(VD); - + auto initIndex = PBD->getPatternEntryIndexForVarDecl(VD); + auto init = PBD->getInit(initIndex); + if (!init) + return; + // Make sure property is being set with a constructor - auto ctor = dyn_cast(entry.getInit()); + auto ctor = dyn_cast(init); if (!ctor) return; auto ctorCalledVal = ctor->getCalledValue(); From 9090d7fb010645a7078203c6a70f06a866a1ecae Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Mon, 28 Oct 2019 18:17:24 -0500 Subject: [PATCH 24/26] add test --- test/Sema/option-set-empty.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Sema/option-set-empty.swift b/test/Sema/option-set-empty.swift index 9d2aa9ba52c0..3416e15f1e26 100644 --- a/test/Sema/option-set-empty.swift +++ b/test/Sema/option-set-empty.swift @@ -9,7 +9,7 @@ struct SomeOptions: OptionSet { let someVal = MyOptions(rawValue: 6) let option = MyOptions(float: Float.infinity) - let none = MyOptions(rawValue: 0) + let none = SomeOptions(rawValue: 0) // expected-error {{value type 'SomeOptions' cannot have a stored property that recursively contains it}} } struct MyOptions: OptionSet { From cba8684a45fdf1b22ea079dfdb7b1c9a6d46d959 Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Tue, 29 Oct 2019 07:46:19 -0500 Subject: [PATCH 25/26] use dyn_cast_or_null --- lib/Sema/TypeCheckDecl.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 736892506bb6..99a240a083bb 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -514,11 +514,9 @@ static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { auto initIndex = PBD->getPatternEntryIndexForVarDecl(VD); auto init = PBD->getInit(initIndex); - if (!init) - return; // Make sure property is being set with a constructor - auto ctor = dyn_cast(init); + auto ctor = dyn_cast_or_null(init); if (!ctor) return; auto ctorCalledVal = ctor->getCalledValue(); From 3dba00f1c7e69b1dca3f31b1a9b4980e7b57ae0e Mon Sep 17 00:00:00 2001 From: Miguel Salinas Date: Thu, 31 Oct 2019 08:15:10 -0500 Subject: [PATCH 26/26] use bool conversion for protocol conformance instead of hasValue() --- lib/Sema/TypeCheckDecl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 7a5b4f12f599..68a5a4f6893c 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -499,11 +499,11 @@ static void checkForEmptyOptionSet(const VarDecl *VD, TypeChecker &tc) { // Make sure this type conforms to OptionSet auto *optionSetProto = tc.Context.getProtocol(KnownProtocolKind::OptionSet); - bool conformsToOptionSet = tc.containsProtocol( + bool conformsToOptionSet = (bool)tc.containsProtocol( DC->getSelfTypeInContext(), optionSetProto, DC, - /*Flags*/None).hasValue(); + /*Flags*/None); if (!conformsToOptionSet) return;