Skip to content

Commit e3fa190

Browse files
ffriedffried
committed
Check for duplicate literal switch cases
This adds a check for duplicate literal switch cases. It covers string, integer and float literals. Corresponding tests were added as well.
1 parent 1b9d19a commit e3fa190

File tree

3 files changed

+319
-2
lines changed

3 files changed

+319
-2
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3773,6 +3773,11 @@ NOTE(missing_particular_case,none,
37733773
"add missing case: '%0'", (StringRef))
37743774
WARNING(redundant_particular_case,none,
37753775
"case is already handled by previous patterns; consider removing it",())
3776+
WARNING(redundant_particular_literal_case,none,
3777+
"literal value is already handled by previous pattern; "
3778+
"consider removing it",())
3779+
NOTE(redundant_particular_literal_case_here,none,
3780+
"first occurrence of identical literal pattern is here", ())
37763781

37773782
// HACK: Downgrades the above to warnings if any of the cases is marked
37783783
// @_downgrade_exhaustivity_check.

‎lib/Sema/TypeCheckSwitchStmt.cpp

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,53 @@
1919
#include "swift/AST/DiagnosticsSema.h"
2020
#include "swift/AST/Pattern.h"
2121

22+
#include <llvm/ADT/APInt.h>
23+
#include <llvm/ADT/APFloat.h>
24+
2225
#include <numeric>
2326
#include <forward_list>
2427

2528
using namespace swift;
2629

2730
#define DEBUG_TYPE "TypeCheckSwitchStmt"
2831

32+
namespace {
33+
struct DenseMapAPIntKeyInfo {
34+
static inline APInt getEmptyKey() { return APInt(); }
35+
36+
static inline APInt getTombstoneKey() {
37+
return APInt::getAllOnesValue(/*bitwidth*/1);
38+
}
39+
40+
static unsigned getHashValue(const APInt &Key) {
41+
return static_cast<unsigned>(hash_value(Key));
42+
}
43+
44+
static bool isEqual(const APInt &LHS, const APInt &RHS) {
45+
return LHS.getBitWidth() == RHS.getBitWidth() && LHS == RHS;
46+
}
47+
};
48+
49+
struct DenseMapAPFloatKeyInfo {
50+
static inline APFloat getEmptyKey() { return APFloat(APFloat::Bogus(), 1); }
51+
static inline APFloat getTombstoneKey() { return APFloat(APFloat::Bogus(), 2); }
52+
53+
static unsigned getHashValue(const APFloat &Key) {
54+
return static_cast<unsigned>(hash_value(Key));
55+
}
56+
57+
static bool isEqual(const APFloat &LHS, const APFloat &RHS) {
58+
return LHS.bitwiseIsEqual(RHS);
59+
}
60+
};
61+
}
62+
2963
namespace {
3064

3165
/// The SpaceEngine encapsulates an algorithm for computing the exhaustiveness
3266
/// of a switch statement using an algebra of spaces described by Fengyun Liu
3367
/// and an algorithm for computing warnings for pattern matching by
34-
// Luc Maranget.
68+
/// Luc Maranget.
3569
///
3670
/// The main algorithm centers around the computation of the difference and
3771
/// the intersection of the "Spaces" given in each case, which reduces the
@@ -914,9 +948,62 @@ namespace {
914948

915949
TypeChecker &TC;
916950
SwitchStmt *Switch;
917-
951+
llvm::DenseMap<APInt, Expr *, ::DenseMapAPIntKeyInfo> IntLiteralCache;
952+
llvm::DenseMap<APFloat, Expr *, ::DenseMapAPFloatKeyInfo> FloatLiteralCache;
953+
llvm::DenseMap<StringRef, Expr *> StringLiteralCache;
954+
918955
SpaceEngine(TypeChecker &C, SwitchStmt *SS) : TC(C), Switch(SS) {}
919956

957+
bool checkRedundantLiteral(const Pattern *Pat, Expr *&PrevPattern) {
958+
if (Pat->getKind() != PatternKind::Expr) {
959+
return false;
960+
}
961+
auto *ExprPat = cast<ExprPattern>(Pat);
962+
auto *MatchExpr = ExprPat->getSubExpr();
963+
if (!MatchExpr || !isa<LiteralExpr>(MatchExpr)) {
964+
return false;
965+
}
966+
auto *EL = cast<LiteralExpr>(MatchExpr);
967+
switch (EL->getKind()) {
968+
case ExprKind::StringLiteral: {
969+
auto *SLE = cast<StringLiteralExpr>(EL);
970+
auto cacheVal =
971+
StringLiteralCache.insert({SLE->getValue(), SLE});
972+
PrevPattern = (cacheVal.first != StringLiteralCache.end())
973+
? cacheVal.first->getSecond()
974+
: nullptr;
975+
return !cacheVal.second;
976+
}
977+
case ExprKind::IntegerLiteral: {
978+
// FIXME: The magic number 128 is bad and we should actually figure out
979+
// the bitwidth. But it's too early in Sema to get it.
980+
auto *ILE = cast<IntegerLiteralExpr>(EL);
981+
auto cacheVal =
982+
IntLiteralCache.insert(
983+
{ILE->getValue(ILE->getDigitsText(), 128), ILE});
984+
PrevPattern = (cacheVal.first != IntLiteralCache.end())
985+
? cacheVal.first->getSecond()
986+
: nullptr;
987+
return !cacheVal.second;
988+
}
989+
case ExprKind::FloatLiteral: {
990+
// FIXME: Pessimistically using IEEEquad here is bad and we should
991+
// actually figure out the bitwidth. But it's too early in Sema.
992+
auto *FLE = cast<FloatLiteralExpr>(EL);
993+
auto cacheVal =
994+
FloatLiteralCache.insert(
995+
{FLE->getValue(FLE->getDigitsText(),
996+
APFloat::IEEEquad()), FLE});
997+
PrevPattern = (cacheVal.first != FloatLiteralCache.end())
998+
? cacheVal.first->getSecond()
999+
: nullptr;
1000+
return !cacheVal.second;
1001+
}
1002+
default:
1003+
return false;
1004+
}
1005+
}
1006+
9201007
void checkExhaustiveness(bool limitedChecking) {
9211008
// If the type of the scrutinee is uninhabited, we're already dead.
9221009
// Allow any well-typed patterns through.
@@ -958,6 +1045,17 @@ namespace {
9581045
TC.diagnose(caseItem.getStartLoc(),
9591046
diag::redundant_particular_case)
9601047
.highlight(caseItem.getSourceRange());
1048+
} else {
1049+
Expr *cachedExpr = nullptr;
1050+
if (checkRedundantLiteral(caseItem.getPattern(), cachedExpr)) {
1051+
assert(cachedExpr && "Cache found hit but no expr?");
1052+
TC.diagnose(caseItem.getStartLoc(),
1053+
diag::redundant_particular_literal_case)
1054+
.highlight(caseItem.getSourceRange());
1055+
TC.diagnose(cachedExpr->getLoc(),
1056+
diag::redundant_particular_literal_case_here)
1057+
.highlight(cachedExpr->getSourceRange());
1058+
}
9611059
}
9621060
spaces.push_back(projection);
9631061
}

‎test/Sema/exhaustive_switch.swift

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,3 +558,217 @@ func infinitelySized() -> Bool {
558558
case (.two, .two): return true
559559
}
560560
}
561+
562+
// Literal Duplicate checks
563+
func checkLiterals() {
564+
let str = "def"
565+
let int = 2
566+
let dbl = 2.5
567+
568+
// No Diagnostics
569+
switch str {
570+
case "abc": break
571+
case "def": break
572+
case "ghi": break
573+
default: break
574+
}
575+
576+
switch str {
577+
case "abc": break
578+
case "def": break // expected-note {{first occurrence of identical literal pattern is here}}
579+
case "def": break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
580+
case "ghi": break
581+
default: break
582+
}
583+
584+
switch str {
585+
case "abc", "def": break // expected-note 2 {{first occurrence of identical literal pattern is here}}
586+
case "ghi", "jkl": break
587+
case "abc", "def": break // expected-warning 2 {{literal value is already handled by previous pattern; consider removing it}}
588+
default: break
589+
}
590+
591+
switch str {
592+
case "xyz": break // expected-note {{first occurrence of identical literal pattern is here}}
593+
case "ghi": break
594+
case "def": break
595+
case "abc": break
596+
case "xyz": break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
597+
default: break
598+
}
599+
600+
func someStr() -> String { return "sdlkj" }
601+
let otherStr = "ifnvbnwe"
602+
switch str {
603+
case "sdlkj": break
604+
case "ghi": break // expected-note {{first occurrence of identical literal pattern is here}}
605+
case someStr(): break
606+
case "def": break
607+
case otherStr: break
608+
case "xyz": break // expected-note {{first occurrence of identical literal pattern is here}}
609+
case "ifnvbnwe": break
610+
case "ghi": break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
611+
case "xyz": break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
612+
default: break
613+
}
614+
615+
// No Diagnostics
616+
switch int {
617+
case 1: break
618+
case 2: break
619+
case 3: break
620+
default: break
621+
}
622+
623+
switch int {
624+
case 1: break
625+
case 2: break // expected-note {{first occurrence of identical literal pattern is here}}
626+
case 2: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
627+
case 3: break
628+
default: break
629+
}
630+
631+
switch int {
632+
case 1, 2: break // expected-note 3 {{first occurrence of identical literal pattern is here}}
633+
case 2, 3: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
634+
case 1, 2: break // expected-warning 2 {{literal value is already handled by previous pattern; consider removing it}}
635+
case 4, 5: break
636+
case 7, 7: break // expected-note {{first occurrence of identical literal pattern is here}}
637+
// expected-warning@-1 {{literal value is already handled by previous pattern; consider removing it}}
638+
default: break
639+
}
640+
641+
switch int {
642+
case 1: break // expected-note {{first occurrence of identical literal pattern is here}}
643+
case 2: break // expected-note 2 {{first occurrence of identical literal pattern is here}}
644+
case 3: break
645+
case 17: break // expected-note {{first occurrence of identical literal pattern is here}}
646+
case 4: break
647+
case 2: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
648+
case 001: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
649+
case 5: break
650+
case 0x11: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
651+
case 0b10: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
652+
default: break
653+
}
654+
655+
switch int {
656+
case 10: break
657+
case 0b10: break // expected-note {{first occurrence of identical literal pattern is here}}
658+
case 3000: break
659+
case 0x12: break // expected-note {{first occurrence of identical literal pattern is here}}
660+
case 400: break
661+
case 2: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
662+
case 18: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
663+
default: break
664+
}
665+
666+
func someInt() -> Int { return 0x1234 }
667+
let otherInt = 13254
668+
switch int {
669+
case 13254: break
670+
case 3000: break
671+
case 00000002: break // expected-note {{first occurrence of identical literal pattern is here}}
672+
case 0x1234: break
673+
case someInt(): break
674+
case 400: break
675+
case 2: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
676+
case 18: break
677+
case otherInt: break
678+
case 230: break
679+
default: break
680+
}
681+
682+
// No Diagnostics
683+
switch dbl {
684+
case 1.5: break
685+
case 2.5: break
686+
case 3.5: break
687+
default: break
688+
}
689+
690+
switch dbl {
691+
case 1.5: break
692+
case 2.5: break // expected-note {{first occurrence of identical literal pattern is here}}
693+
case 2.5: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
694+
case 3.5: break
695+
default: break
696+
}
697+
698+
switch dbl {
699+
case 1.5, 4.5, 7.5, 6.9: break // expected-note 2 {{first occurrence of identical literal pattern is here}}
700+
case 3.4, 1.5: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
701+
case 7.5, 2.3: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
702+
default: break
703+
}
704+
705+
switch dbl {
706+
case 1: break
707+
case 1.5: break // expected-note 2 {{first occurrence of identical literal pattern is here}}
708+
case 2.5: break
709+
case 3.5: break // expected-note {{first occurrence of identical literal pattern is here}}
710+
case 5.3132: break
711+
case 1.500: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
712+
case 46.2395: break
713+
case 1.5000: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
714+
case 0003.50000: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
715+
case 23452.43: break
716+
default: break
717+
}
718+
719+
func someDouble() -> Double { return 324.4523 }
720+
let otherDouble = 458.2345
721+
switch dbl {
722+
case 1: break // expected-note {{first occurrence of identical literal pattern is here}}
723+
case 1.5: break
724+
case 2.5: break
725+
case 3.5: break // expected-note {{first occurrence of identical literal pattern is here}}
726+
case 5.3132: break
727+
case 46.2395: break
728+
case someDouble(): break
729+
case 0003.50000: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
730+
case otherDouble: break
731+
case 2.50505: break // expected-note {{first occurrence of identical literal pattern is here}}
732+
case 23452.43: break
733+
case 00001: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
734+
case 123453: break
735+
case 2.50505000000: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
736+
default: break
737+
}
738+
}
739+
740+
func checkLiteralTuples() {
741+
let str1 = "abc"
742+
let str2 = "def"
743+
let int1 = 23
744+
let int2 = 7
745+
let dbl1 = 4.23
746+
let dbl2 = 23.45
747+
748+
// No Diagnostics
749+
switch (str1, str2) {
750+
case ("abc", "def"): break
751+
case ("def", "ghi"): break
752+
case ("ghi", "def"): break
753+
case ("abc", "def"): break // We currently don't catch this
754+
default: break
755+
}
756+
757+
// No Diagnostics
758+
switch (int1, int2) {
759+
case (94, 23): break
760+
case (7, 23): break
761+
case (94, 23): break // We currently don't catch this
762+
case (23, 7): break
763+
default: break
764+
}
765+
766+
// No Diagnostics
767+
switch (dbl1, dbl2) {
768+
case (543.21, 123.45): break
769+
case (543.21, 123.45): break // We currently don't catch this
770+
case (23.45, 4.23): break
771+
case (4.23, 23.45): break
772+
default: break
773+
}
774+
}

0 commit comments

Comments
 (0)