Check for duplicate literal switch cases

include/swift/AST/DiagnosticsSema.def

@@ -3773,6 +3773,11 @@ NOTE(missing_particular_case,none,
      "add missing case: '%0'", (StringRef))
 WARNING(redundant_particular_case,none,
         "case is already handled by previous patterns; consider removing it",())
+WARNING(redundant_particular_literal_case,none,
+        "literal value is already handled by previous pattern; "
+        "consider removing it",())
+NOTE(redundant_particular_literal_case_here,none,
+     "first occurrence of identical literal pattern is here", ())
 
 // HACK: Downgrades the above to warnings if any of the cases is marked
 // @_downgrade_exhaustivity_check.

lib/Sema/TypeCheckSwitchStmt.cpp

@@ -19,19 +19,53 @@
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/Pattern.h"
 
+#include <llvm/ADT/APInt.h>
+#include <llvm/ADT/APFloat.h>
+
 #include <numeric>
 #include <forward_list>
 
 using namespace swift;
 
 #define DEBUG_TYPE "TypeCheckSwitchStmt"
 
+namespace {
+  struct DenseMapAPIntKeyInfo {
+    static inline APInt getEmptyKey() { return APInt(); }
+
+    static inline APInt getTombstoneKey() {
+      return APInt::getAllOnesValue(/*bitwidth*/1);
+    }
+
+    static unsigned getHashValue(const APInt &Key) {
+      return static_cast<unsigned>(hash_value(Key));
+    }
+
+    static bool isEqual(const APInt &LHS, const APInt &RHS) {
+      return LHS.getBitWidth() == RHS.getBitWidth() && LHS == RHS;
+    }
+  };
+
+  struct DenseMapAPFloatKeyInfo {
+    static inline APFloat getEmptyKey() { return APFloat(APFloat::Bogus(), 1); }
+    static inline APFloat getTombstoneKey() { return APFloat(APFloat::Bogus(), 2); }
+
+    static unsigned getHashValue(const APFloat &Key) {
+      return static_cast<unsigned>(hash_value(Key));
+    }
+
+    static bool isEqual(const APFloat &LHS, const APFloat &RHS) {
+      return LHS.bitwiseIsEqual(RHS);
+    }
+  };
+}
+
 namespace {
 
   /// The SpaceEngine encapsulates an algorithm for computing the exhaustiveness
   /// of a switch statement using an algebra of spaces described by Fengyun Liu
   /// and an algorithm for computing warnings for pattern matching by
-  //  Luc Maranget.
+  /// Luc Maranget.
   ///
   /// The main algorithm centers around the computation of the difference and
   /// the intersection of the "Spaces" given in each case, which reduces the
@@ -914,9 +948,62 @@ namespace {
 
     TypeChecker &TC;
     SwitchStmt *Switch;
-
+    llvm::DenseMap<APInt, Expr *, ::DenseMapAPIntKeyInfo> IntLiteralCache;
+    llvm::DenseMap<APFloat, Expr *, ::DenseMapAPFloatKeyInfo> FloatLiteralCache;
+    llvm::DenseMap<StringRef, Expr *> StringLiteralCache;
+
     SpaceEngine(TypeChecker &C, SwitchStmt *SS) : TC(C), Switch(SS) {}
 
+    bool checkRedundantLiteral(const Pattern *Pat, Expr *&PrevPattern) {
+      if (Pat->getKind() != PatternKind::Expr) {
+          return false;
+      }
+      auto *ExprPat = cast<ExprPattern>(Pat);
+      auto *MatchExpr = ExprPat->getSubExpr();
+      if (!MatchExpr || !isa<LiteralExpr>(MatchExpr)) {
+          return false;
+      }
+      auto *EL = cast<LiteralExpr>(MatchExpr);
+      switch (EL->getKind()) {
+      case ExprKind::StringLiteral: {
+        auto *SLE = cast<StringLiteralExpr>(EL);
+        auto cacheVal =
+            StringLiteralCache.insert({SLE->getValue(), SLE});
+        PrevPattern = (cacheVal.first != StringLiteralCache.end())
+                    ? cacheVal.first->getSecond()
+                    : nullptr;
+        return !cacheVal.second;
+      }
+      case ExprKind::IntegerLiteral: {
+        // FIXME: The magic number 128 is bad and we should actually figure out
+        // the bitwidth.  But it's too early in Sema to get it.
+        auto *ILE = cast<IntegerLiteralExpr>(EL);
+        auto cacheVal =
+            IntLiteralCache.insert(
+                {ILE->getValue(ILE->getDigitsText(), 128), ILE});
+        PrevPattern = (cacheVal.first != IntLiteralCache.end())
+                    ? cacheVal.first->getSecond()
+                    : nullptr;
+        return !cacheVal.second;
+      }
+      case ExprKind::FloatLiteral: {
+        // FIXME: Pessimistically using IEEEquad here is bad and we should
+        // actually figure out the bitwidth.  But it's too early in Sema.
+        auto *FLE = cast<FloatLiteralExpr>(EL);
+        auto cacheVal =
+            FloatLiteralCache.insert(
+                   {FLE->getValue(FLE->getDigitsText(),
+                                  APFloat::IEEEquad()), FLE});
+        PrevPattern = (cacheVal.first != FloatLiteralCache.end())
+                    ? cacheVal.first->getSecond()
+                    : nullptr;
+        return !cacheVal.second;
+      }
+      default:
+        return false;
+      }
+    }
+
     void checkExhaustiveness(bool limitedChecking) {
       // If the type of the scrutinee is uninhabited, we're already dead.
       // Allow any well-typed patterns through.
@@ -958,6 +1045,17 @@ namespace {
             TC.diagnose(caseItem.getStartLoc(),
                           diag::redundant_particular_case)
               .highlight(caseItem.getSourceRange());
+          } else {
+            Expr *cachedExpr = nullptr;
+            if (checkRedundantLiteral(caseItem.getPattern(), cachedExpr)) {
+              assert(cachedExpr && "Cache found hit but no expr?");
+              TC.diagnose(caseItem.getStartLoc(),
+                          diag::redundant_particular_literal_case)
+                .highlight(caseItem.getSourceRange());
+              TC.diagnose(cachedExpr->getLoc(),
+                          diag::redundant_particular_literal_case_here)
+                .highlight(cachedExpr->getSourceRange());
+            }
           }
           spaces.push_back(projection);
         }

test/Sema/exhaustive_switch.swift

@@ -558,3 +558,217 @@ func infinitelySized() -> Bool {
   case (.two, .two): return true
   }
 }
+
+// Literal Duplicate checks
+func checkLiterals() {
+  let str = "def"
+  let int = 2
+  let dbl = 2.5
+
+  // No Diagnostics
+  switch str {
+  case "abc": break
+  case "def": break
+  case "ghi": break
+  default: break
+  }
+
+  switch str {
+  case "abc": break
+  case "def": break // expected-note {{first occurrence of identical literal pattern is here}}
+  case "def": break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case "ghi": break
+  default: break
+  }
+
+  switch str {
+  case "abc", "def": break // expected-note 2 {{first occurrence of identical literal pattern is here}}
+  case "ghi", "jkl": break
+  case "abc", "def": break // expected-warning 2 {{literal value is already handled by previous pattern; consider removing it}}
+  default: break
+  }
+
+  switch str {
+  case "xyz": break // expected-note {{first occurrence of identical literal pattern is here}}
+  case "ghi": break
+  case "def": break
+  case "abc": break
+  case "xyz": break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  default: break
+  }
+
+  func someStr() -> String { return "sdlkj" }
+  let otherStr = "ifnvbnwe"
+  switch str {
+  case "sdlkj": break
+  case "ghi": break // expected-note {{first occurrence of identical literal pattern is here}}
+  case someStr(): break
+  case "def": break
+  case otherStr: break
+  case "xyz": break // expected-note {{first occurrence of identical literal pattern is here}}
+  case "ifnvbnwe": break
+  case "ghi": break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case "xyz": break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  default: break
+  }
+
+  // No Diagnostics
+  switch int {
+  case 1: break
+  case 2: break
+  case 3: break
+  default: break
+  }
+
+  switch int {
+  case 1: break
+  case 2: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 2: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 3: break
+  default: break
+  }
+
+  switch int {
+  case 1, 2: break // expected-note 3 {{first occurrence of identical literal pattern is here}}
+  case 2, 3: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 1, 2: break // expected-warning 2 {{literal value is already handled by previous pattern; consider removing it}}
+  case 4, 5: break
+  case 7, 7: break // expected-note {{first occurrence of identical literal pattern is here}}
+                   // expected-warning@-1 {{literal value is already handled by previous pattern; consider removing it}}
+  default: break
+  }
+
+  switch int {
+  case 1: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 2: break // expected-note 2 {{first occurrence of identical literal pattern is here}}
+  case 3: break
+  case 17: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 4: break
+  case 2: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 001: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 5: break
+  case 0x11: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 0b10: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  default: break
+  }
+
+  switch int {
+  case 10: break
+  case 0b10: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 3000: break
+  case 0x12: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 400: break
+  case 2: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 18: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  default: break
+  }
+
+  func someInt() -> Int { return 0x1234 }
+  let otherInt = 13254
+  switch int {
+  case 13254: break
+  case 3000: break
+  case 00000002: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 0x1234: break
+  case someInt(): break
+  case 400: break
+  case 2: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 18: break
+  case otherInt: break
+  case 230: break
+  default: break
+  }
+
+  // No Diagnostics
+  switch dbl {
+  case 1.5: break
+  case 2.5: break
+  case 3.5: break
+  default: break
+  }
+
+  switch dbl {
+  case 1.5: break
+  case 2.5: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 2.5: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 3.5: break
+  default: break
+  }
+
+  switch dbl {
+  case 1.5, 4.5, 7.5, 6.9: break // expected-note 2 {{first occurrence of identical literal pattern is here}}
+  case 3.4, 1.5: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 7.5, 2.3: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  default: break
+  }
+
+  switch dbl {
+  case 1: break
+  case 1.5: break // expected-note 2 {{first occurrence of identical literal pattern is here}}
+  case 2.5: break
+  case 3.5: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 5.3132: break
+  case 1.500: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 46.2395: break
+  case 1.5000: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 0003.50000: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 23452.43: break
+  default: break
+  }
+
+  func someDouble() -> Double { return 324.4523 }
+  let otherDouble = 458.2345
+  switch dbl {
+  case 1: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 1.5: break
+  case 2.5: break
+  case 3.5: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 5.3132: break
+  case 46.2395: break
+  case someDouble(): break
+  case 0003.50000: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case otherDouble: break
+  case 2.50505: break // expected-note {{first occurrence of identical literal pattern is here}}
+  case 23452.43: break
+  case 00001: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  case 123453: break
+  case 2.50505000000: break // expected-warning {{literal value is already handled by previous pattern; consider removing it}}
+  default: break
+  }
+}
+
+func checkLiteralTuples() {
+  let str1 = "abc"
+  let str2 = "def"
+  let int1 = 23
+  let int2 = 7
+  let dbl1 = 4.23
+  let dbl2 = 23.45
+
+  // No Diagnostics
+  switch (str1, str2) {
+  case ("abc", "def"): break
+  case ("def", "ghi"): break
+  case ("ghi", "def"): break
+  case ("abc", "def"): break // We currently don't catch this
+  default: break
+  }
+
+  // No Diagnostics
+  switch (int1, int2) {
+  case (94, 23): break
+  case (7, 23): break
+  case (94, 23): break // We currently don't catch this
+  case (23, 7): break
+  default: break
+  }
+
+  // No Diagnostics
+  switch (dbl1, dbl2) {
+  case (543.21, 123.45): break
+  case (543.21, 123.45): break // We currently don't catch this
+  case (23.45, 4.23): break
+  case (4.23, 23.45): break
+  default: break
+  }
+}