diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index cc677b0ca3d92..1df3e67ca4dbc 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/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. diff --git a/lib/Sema/TypeCheckSwitchStmt.cpp b/lib/Sema/TypeCheckSwitchStmt.cpp index 094d3f08bbf24..1a7dff5de24a2 100644 --- a/lib/Sema/TypeCheckSwitchStmt.cpp +++ b/lib/Sema/TypeCheckSwitchStmt.cpp @@ -19,6 +19,9 @@ #include "swift/AST/DiagnosticsSema.h" #include "swift/AST/Pattern.h" +#include +#include + #include #include @@ -26,12 +29,43 @@ 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(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(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 IntLiteralCache; + llvm::DenseMap FloatLiteralCache; + llvm::DenseMap 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(Pat); + auto *MatchExpr = ExprPat->getSubExpr(); + if (!MatchExpr || !isa(MatchExpr)) { + return false; + } + auto *EL = cast(MatchExpr); + switch (EL->getKind()) { + case ExprKind::StringLiteral: { + auto *SLE = cast(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(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(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); } diff --git a/test/Sema/exhaustive_switch.swift b/test/Sema/exhaustive_switch.swift index 65cd55f9ec948..74eed8b77ee69 100644 --- a/test/Sema/exhaustive_switch.swift +++ b/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 + } +}