From b2ec9d6fede9cccc170a202de7bf7f523dea8be4 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Thu, 23 Aug 2007 06:23:56 +0000 Subject: [PATCH] report duplicate case values. TODO: report duplicate/overlapping ranges. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@41315 91177308-0d34-0410-b5e6-96231b3b80d8 --- Sema/SemaStmt.cpp | 85 ++++++++++++++++++------- include/clang/Basic/DiagnosticKinds.def | 4 ++ test/Sema/switch.c | 3 +- 3 files changed, 67 insertions(+), 25 deletions(-) diff --git a/Sema/SemaStmt.cpp b/Sema/SemaStmt.cpp index 3bc1b9cd54..c660b71742 100644 --- a/Sema/SemaStmt.cpp +++ b/Sema/SemaStmt.cpp @@ -232,9 +232,11 @@ Sema::FinishSwitchStmt(SourceLocation SwitchLoc, StmtTy *Switch, ExprTy *Body) { DefaultStmt *TheDefaultStmt = 0; - // FIXME: The list of cases is backwards and needs to be reversed. - for (SwitchCase *SC = SS->getSwitchCaseList(); SC; + bool CaseListIsErroneous = false; + + for (SwitchCase *SC = SS->getSwitchCaseList(); SC; SC = SC->getNextSwitchCase()) { + if (DefaultStmt *DS = dyn_cast(SC)) { if (TheDefaultStmt) { Diag(DS->getDefaultLoc(), diag::err_multiple_default_labels_defined); @@ -244,7 +246,7 @@ Sema::FinishSwitchStmt(SourceLocation SwitchLoc, StmtTy *Switch, ExprTy *Body) { // we'll return a valid AST. This requires recursing down the // AST and finding it, not something we are set up to do right now. For // now, just lop the entire switch stmt out of the AST. - return true; + CaseListIsErroneous = true; } TheDefaultStmt = DS; @@ -261,34 +263,69 @@ Sema::FinishSwitchStmt(SourceLocation SwitchLoc, StmtTy *Switch, ExprTy *Body) { CS->getLHS()->getLocStart(), diag::warn_case_value_overflow); - // Remember the converted value. - CaseVals.push_back(std::make_pair(LoVal, CS)); - - // Just remember where the case range was. + // If this is a case range, remember it in CaseRanges, otherwise CaseVals. if (CS->getRHS()) CaseRanges.push_back(std::make_pair(LoVal, CS)); + else + CaseVals.push_back(std::make_pair(LoVal, CS)); } } + // Sort all the scalar case values so we can easily detect duplicates. + std::stable_sort(CaseVals.begin(), CaseVals.end()); + + for (unsigned i = 0, e = CaseVals.size()-1; i != e; ++i) { + if (CaseVals[i].first == CaseVals[i+1].first) { + // If we have a duplicate, report it. + Diag(CaseVals[i+1].second->getLHS()->getLocStart(), + diag::err_duplicate_case, CaseVals[i].first.toString()); + Diag(CaseVals[i].second->getLHS()->getLocStart(), + diag::err_duplicate_case_prev); + // FIXME: We really want to remove the bogus case stmt from the substmt, + // but we have no way to do this right now. + CaseListIsErroneous = true; + } + } + // Detect duplicate case ranges, which usually don't exist at all in the first + // place. + if (!CaseRanges.empty()) { + // Sort all the case ranges by their low value so we can easily detect + // overlaps between ranges. + std::stable_sort(CaseVals.begin(), CaseVals.end()); + + // Scan the ranges, computing the high values and removing empty ranges. + std::vector HiVals; + for (unsigned i = 0, e = CaseVals.size(); i != e; ++i) { + CaseStmt *CR = CaseRanges[i].second; + llvm::APSInt HiVal(32); + CR->getRHS()->isIntegerConstantExpr(HiVal, Context); + + // Convert the value to the same width/sign as the condition. + ConvertIntegerToTypeWarnOnOverflow(HiVal, CondWidth, CondIsSigned, + CR->getRHS()->getLocStart(), + diag::warn_case_value_overflow); + + // FIXME: if the low value is bigger than the high value, the case is + // empty: emit "empty range specified" warning and drop it. + + HiVals.push_back(HiVal); + } + + // Rescan the ranges, looking for overlap with singleton values and other + // ranges. + for (unsigned i = 0, e = CaseRanges.size(); i != e; ++i) { + //llvm::APSInt &CRLow = CaseRanges[i].first; + //CaseStmt *CR = CaseRanges[i].second; + + // FIXME: TODO. + } + } -#if 0 - assert(CaseRanges.empty() && "FIXME: Check case ranges for overlap etc"); - // TODO: if the low value is bigger than the high value, the case is - // empty: emit "empty range specified" warning and drop the c - - llvm::APSInt HiVal(32); - RHS->isIntegerConstantExpr(HiVal, Context); - - // Convert the value to the same width/sign as the condition. - ConvertIntegerToTypeWarnOnOverflow(HiVal, CondWidth, CondIsSigned); - - // If low value and high value equal, just forget about it as a range - // for the purposes of checking: it identifies a single value. - if (LoVal == HiVal) - continue; - -#endif + // FIXME: If the case list was broken is some way, we don't have a good system + // to patch it up. Instead, just return the whole substmt as broken. + if (CaseListIsErroneous) + return true; return SS; } diff --git a/include/clang/Basic/DiagnosticKinds.def b/include/clang/Basic/DiagnosticKinds.def index 6f4fa2571d..0dd27c4f6c 100644 --- a/include/clang/Basic/DiagnosticKinds.def +++ b/include/clang/Basic/DiagnosticKinds.def @@ -715,6 +715,10 @@ DIAG(err_case_not_in_switch, ERROR, "'case' statement not in switch statement") DIAG(warn_case_value_overflow, WARNING, "overflow converting case value to switch condition type (%0 to %1)") +DIAG(err_duplicate_case, ERROR, + "duplicate case value '%0'") +DIAG(err_duplicate_case_prev, ERROR, + "previous case value occurrence defined here") DIAG(err_typecheck_return_incompatible, ERROR, "incompatible type returning '%1', expected '%0'") DIAG(ext_typecheck_return_pointer_int, WARNING, diff --git a/test/Sema/switch.c b/test/Sema/switch.c index 8559e0dee3..ce195b175f 100644 --- a/test/Sema/switch.c +++ b/test/Sema/switch.c @@ -8,8 +8,9 @@ void f (int z) { void foo(int X) { switch (X) { + case 42: ; // expected-error {{previous case value}} case 5000000000LL: // expected-warning {{overflow}} - case 42: + case 42: // expected-error {{duplicate case value}} ; } } -- 2.50.1