[cfe-commits] r41315 - in /cfe/trunk: Sema/SemaStmt.cpp include/clang/Basic/DiagnosticKinds.def test/Sema/switch.c

Chris Lattner sabre at nondot.org
Wed Aug 22 23:23:56 PDT 2007


Author: lattner
Date: Thu Aug 23 01:23:56 2007
New Revision: 41315

URL: http://llvm.org/viewvc/llvm-project?rev=41315&view=rev
Log:
report duplicate case values.  TODO: report duplicate/overlapping ranges.

Modified:
    cfe/trunk/Sema/SemaStmt.cpp
    cfe/trunk/include/clang/Basic/DiagnosticKinds.def
    cfe/trunk/test/Sema/switch.c

Modified: cfe/trunk/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Sema/SemaStmt.cpp?rev=41315&r1=41314&r2=41315&view=diff

==============================================================================
--- cfe/trunk/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/Sema/SemaStmt.cpp Thu Aug 23 01:23:56 2007
@@ -232,9 +232,11 @@
   
   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<DefaultStmt>(SC)) {
       if (TheDefaultStmt) {
         Diag(DS->getDefaultLoc(), diag::err_multiple_default_labels_defined);
@@ -244,7 +246,7 @@
         // 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 @@
                                          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;
+    }
+  }
   
-#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;
+  // 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<llvm::APSInt> 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.
+    }
+  }
   
-#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;
 }

Modified: cfe/trunk/include/clang/Basic/DiagnosticKinds.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticKinds.def?rev=41315&r1=41314&r2=41315&view=diff

==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticKinds.def (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticKinds.def Thu Aug 23 01:23:56 2007
@@ -715,6 +715,10 @@
      "'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,

Modified: cfe/trunk/test/Sema/switch.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch.c?rev=41315&r1=41314&r2=41315&view=diff

==============================================================================
--- cfe/trunk/test/Sema/switch.c (original)
+++ cfe/trunk/test/Sema/switch.c Thu Aug 23 01:23:56 2007
@@ -8,8 +8,9 @@
 
 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}}
    ;
   }
 }





More information about the cfe-commits mailing list