Refactor switch analysis to make it possible to detect duplicate case values

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@40388 91177308-0d34-0410-b5e6-96231b3b80d8

AST/StmtPrinter.cpp

@@ -218,6 +218,10 @@ void StmtPrinter::VisitSwitchStmt(SwitchStmt *Node) {
   }
 }
 
+void StmtPrinter::VisitSwitchCase(SwitchCase*) {
+  assert(0 && "SwitchCase is an abstract class");
+}
+
 void StmtPrinter::VisitWhileStmt(WhileStmt *Node) {
   Indent() << "while (";
   PrintExpr(Node->getCond());

Parse/ParseStmt.cpp

@@ -475,14 +475,24 @@ Parser::StmtResult Parser::ParseSwitchStatement() {
   // Parse the condition.
   ExprResult Cond = ParseSimpleParenExpression();
   
+  if (Cond.isInvalid) {
+    ExitScope();
+    return true;
+  }
+    
+  StmtResult Switch = Actions.StartSwitchStmt(Cond.Val);
+  
   // Read the body statement.
   StmtResult Body = ParseStatement();
 
+  if (Body.isInvalid) {
+    Body = Actions.ParseNullStmt(Tok.getLocation());
+    // FIXME: Remove the case statement list from the Switch statement.
+  }
+  
   ExitScope();
   
-  if (Cond.isInvalid || Body.isInvalid) return true;
-  
-  return Actions.ParseSwitchStmt(SwitchLoc, Cond.Val, Body.Val);
+  return Actions.FinishSwitchStmt(SwitchLoc, Switch.Val, Body.Val);
 }
 
 /// ParseWhileStatement

Sema/Sema.h

@@ -17,6 +17,7 @@
 
 #include "clang/Parse/Action.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
 #include <vector>
 #include <string>
 
@@ -36,6 +37,7 @@ namespace clang {
   class IntegerLiteral;
   class ArrayType;
   class LabelStmt;
+  class SwitchStmt;
   
 /// Sema - This implements semantic analysis and AST building for C.
 class Sema : public Action {
@@ -57,6 +59,8 @@ class Sema : public Action {
   /// it (which acts like the label decl in some ways).  Forward referenced
   /// labels have a LabelStmt created for them with a null location & SubStmt.
   llvm::DenseMap<IdentifierInfo*, LabelStmt*> LabelMap;
+  
+  llvm::SmallVector<SwitchStmt*, 8> SwitchStack;
 public:
   Sema(Preprocessor &pp, ASTContext &ctxt, std::vector<Decl*> &prevInGroup);
   
@@ -175,8 +179,9 @@ public:
   virtual StmtResult ParseIfStmt(SourceLocation IfLoc, ExprTy *CondVal,
                                  StmtTy *ThenVal, SourceLocation ElseLoc,
                                  StmtTy *ElseVal);
-  virtual StmtResult ParseSwitchStmt(SourceLocation SwitchLoc, ExprTy *Cond,
-                                     StmtTy *Body);
+  virtual StmtResult StartSwitchStmt(ExprTy *Cond);
+  virtual StmtResult FinishSwitchStmt(SourceLocation SwitchLoc, StmtTy *Switch, 
+                                      ExprTy *Body);
   virtual StmtResult ParseWhileStmt(SourceLocation WhileLoc, ExprTy *Cond,
                                     StmtTy *Body);
   virtual StmtResult ParseDoStmt(SourceLocation DoLoc, StmtTy *Body,

Sema/SemaStmt.cpp

@@ -18,6 +18,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Lex/IdentifierTable.h"
+
 using namespace clang;
 
 Sema::StmtResult Sema::ParseExprStmt(ExprTy *expr) {
@@ -56,7 +57,7 @@ Sema::ParseCaseStmt(SourceLocation CaseLoc, ExprTy *lhsval,
   Stmt *SubStmt = static_cast<Stmt*>(subStmt);
   Expr *LHSVal = ((Expr *)lhsval);
   assert((LHSVal != 0) && "missing expression in case statement");
-    
+  
   SourceLocation ExpLoc;
   // C99 6.8.4.2p3: The expression shall be an integer constant.
   if (!LHSVal->isIntegerConstantExpr(Context, &ExpLoc)) {
@@ -73,7 +74,13 @@ Sema::ParseCaseStmt(SourceLocation CaseLoc, ExprTy *lhsval,
     return SubStmt;
   }
 
-  return new CaseStmt(LHSVal, (Expr*)RHSVal, SubStmt);
+  CaseStmt *CS = new CaseStmt(LHSVal, RHSVal, SubStmt);
+  
+  assert(!SwitchStack.empty() && "missing push/pop in switch stack!");
+  SwitchStmt *SS = SwitchStack.back();
+  SS->addSwitchCase(CS);
+ 
+  return CS;
 }
 
 Action::StmtResult
@@ -87,15 +94,12 @@ Sema::ParseDefaultStmt(SourceLocation DefaultLoc, SourceLocation ColonLoc,
     return SubStmt;
   }
   
-  if (S->getDefaultStmt()) {
-    Diag(DefaultLoc, diag::err_multiple_default_labels_defined);
-    Diag(((DefaultStmt *)S->getDefaultStmt())->getDefaultLoc(), 
-         diag::err_first_label);
-    return SubStmt;
-  }
-  
   DefaultStmt *DS = new DefaultStmt(DefaultLoc, SubStmt);
-  S->setDefaultStmt(DS);
+
+  assert(!SwitchStack.empty() && "missing push/pop in switch stack!");
+  SwitchStmt *SS = SwitchStack.back();
+  SS->addSwitchCase(DS);
+
   return DS;
 }
 
@@ -145,16 +149,54 @@ Sema::ParseIfStmt(SourceLocation IfLoc, ExprTy *CondVal,
 }
 
 Action::StmtResult
-Sema::ParseSwitchStmt(SourceLocation SwitchLoc, ExprTy *Cond, StmtTy *Body) {
-  Expr *condExpr = (Expr *)Cond;
+Sema::StartSwitchStmt(ExprTy *Cond) {
+  SwitchStmt *SS = new SwitchStmt((Expr*)Cond);
+  SwitchStack.push_back(SS);
+  return SS;
+}
 
+Action::StmtResult
+Sema::FinishSwitchStmt(SourceLocation SwitchLoc, StmtTy *Switch, ExprTy *Body) {
+  Stmt *BodyStmt = (Stmt*)Body;
+  
+  SwitchStmt *SS = SwitchStack.back();
+  assert(SS == (SwitchStmt*)Switch && "switch stack missing push/pop!");
+    
+  SS->setBody(BodyStmt);
+  SwitchStack.pop_back(); 
+
+  Expr *condExpr = SS->getCond();
   QualType condType = condExpr->getType();
   
-  if (!condType->isIntegerType()) // C99 6.8.4.2p1
-    return Diag(SwitchLoc, diag::err_typecheck_statement_requires_integer,
-                condType.getAsString(), condExpr->getSourceRange());
+  if (!condType->isIntegerType()) { // C99 6.8.4.2p1
+    Diag(SwitchLoc, diag::err_typecheck_statement_requires_integer,
+         condType.getAsString(), condExpr->getSourceRange());
+    return false;
+  }
 
-  return new SwitchStmt((Expr*)Cond, (Stmt*)Body);
+  DefaultStmt *CurDefaultStmt = 0;
+  
+  // FIXME: The list of cases is backwards and needs to be reversed.
+  for (SwitchCase *SC = SS->getSwitchCaseList(); SC; 
+       SC = SC->getNextSwitchCase()) {
+    if (DefaultStmt *DS = dyn_cast<DefaultStmt>(SC)) {
+      if (CurDefaultStmt) {
+        Diag(DS->getDefaultLoc(), 
+            diag::err_multiple_default_labels_defined);
+        Diag(CurDefaultStmt->getDefaultLoc(), 
+            diag::err_first_label);
+            
+        // FIXME: Remove the default statement from the switch block
+        // so that we'll return a valid AST.
+      } else {
+        CurDefaultStmt = DS;
+      }
+      
+      // FIXME: Check that case values are unique here.
+    }    
+  }
+
+  return SS;
 }
 
 Action::StmtResult

include/clang/AST/Stmt.h

@@ -124,15 +124,37 @@ public:
   static bool classof(const CompoundStmt *) { return true; }
 };
 
-class CaseStmt : public Stmt {
+// SwitchCase is the base class for CaseStmt and DefaultStmt,
+class SwitchCase : public Stmt {
+  // A pointer to the following CaseStmt or DefaultStmt class,
+  // used by SwitchStmt.
+  SwitchCase *NextSwitchCase;
+protected:
+  SwitchCase(StmtClass SC) : Stmt(SC), NextSwitchCase(0) {}
+  
+public:
+  const SwitchCase *getNextSwitchCase() const { return NextSwitchCase; }
+
+  SwitchCase *getNextSwitchCase() { return NextSwitchCase; }
+
+  void setNextSwitchCase(SwitchCase *SC) { NextSwitchCase = SC; }
+  
+  static bool classof(const Stmt *T) { 
+    return T->getStmtClass() == CaseStmtClass || 
+    T->getStmtClass() == DefaultStmtClass;
+  }
+  static bool classof(const SwitchCase *) { return true; }
+
+  virtual void visit(StmtVisitor &Visitor) = 0;
+};
+
+class CaseStmt : public SwitchCase {
   Expr *LHSVal;
   Expr *RHSVal;  // Non-null for GNU "case 1 ... 4" extension
   Stmt *SubStmt;
-  SwitchStmt *Switch;
 public:
   CaseStmt(Expr *lhs, Expr *rhs, Stmt *substmt) 
-    : Stmt(CaseStmtClass), LHSVal(lhs), RHSVal(rhs), SubStmt(substmt), 
-    Switch(0) {}
+    : SwitchCase(CaseStmtClass), LHSVal(lhs), RHSVal(rhs), SubStmt(substmt) {}
   
   Expr *getLHS() { return LHSVal; }
   Expr *getRHS() { return RHSVal; }
@@ -145,11 +167,11 @@ public:
   static bool classof(const CaseStmt *) { return true; }
 };
 
-class DefaultStmt : public Stmt {
+class DefaultStmt : public SwitchCase {
   SourceLocation DefaultLoc;
   Stmt *SubStmt;
 public:
-  DefaultStmt(SourceLocation DL, Stmt *substmt) : Stmt(DefaultStmtClass), 
+  DefaultStmt(SourceLocation DL, Stmt *substmt) : SwitchCase(DefaultStmtClass), 
     DefaultLoc(DL), SubStmt(substmt) {}
   
   SourceLocation getDefaultLoc() const { return DefaultLoc; }
@@ -216,12 +238,29 @@ public:
 class SwitchStmt : public Stmt {
   Expr *Cond;
   Stmt *Body;
-public:
-  SwitchStmt(Expr *cond, Stmt *body)
-    : Stmt(SwitchStmtClass), Cond(cond), Body(body) {}
   
+  // This points to a linked list of case and default statements.
+  SwitchCase *FirstCase;
+public:
+  SwitchStmt(Expr *cond)
+    : Stmt(SwitchStmtClass), Cond(cond), Body(0), FirstCase(0) {}
+  
+  const Expr *getCond() const { return Cond; }
+  const Stmt *getBody() const { return Body; }
+  const SwitchCase *getSwitchCaseList() const { return FirstCase; }
+
   Expr *getCond() { return Cond; }
   Stmt *getBody() { return Body; }
+  SwitchCase *getSwitchCaseList() { return FirstCase; }
+
+  void setBody(Stmt *S) { Body = S; }  
+  
+  void addSwitchCase(SwitchCase *SC) {
+    if (FirstCase)
+      SC->setNextSwitchCase(FirstCase);
+
+    FirstCase = SC;
+  }
   
   virtual void visit(StmtVisitor &Visitor);
   static bool classof(const Stmt *T) { 

include/clang/AST/StmtNodes.def

@@ -25,8 +25,8 @@
 FIRST_STMT(1)
 STMT( 1, NullStmt        , Stmt)
 STMT( 2, CompoundStmt    , Stmt)
-STMT( 3, CaseStmt        , Stmt)
-STMT( 4, DefaultStmt     , Stmt)
+STMT( 3, CaseStmt        , SwitchCase)
+STMT( 4, DefaultStmt     , SwitchCase)
 STMT( 5, LabelStmt       , Stmt)
 STMT( 6, IfStmt          , Stmt)
 STMT( 7, SwitchStmt      , Stmt)
@@ -39,7 +39,8 @@ STMT(13, ContinueStmt    , Stmt)
 STMT(14, BreakStmt       , Stmt)
 STMT(15, ReturnStmt      , Stmt)
 STMT(16, DeclStmt        , Stmt)
-LAST_STMT(16)
+STMT(17, SwitchCase      , Stmt)
+LAST_STMT(17)
 
 FIRST_EXPR(31)
 // Expressions.

include/clang/Parse/Action.h

@@ -220,10 +220,15 @@ public:
     return 0; 
   }
   
-  virtual StmtResult ParseSwitchStmt(SourceLocation SwitchLoc, ExprTy *Cond,
-                                     StmtTy *Body) {
+  virtual StmtResult StartSwitchStmt(ExprTy *Cond) {
     return 0;
   }
+  
+  virtual StmtResult FinishSwitchStmt(SourceLocation SwitchLoc, StmtTy *Switch,
+                                      ExprTy *Body) {
+    return 0;
+  }
+
   virtual StmtResult ParseWhileStmt(SourceLocation WhileLoc, ExprTy *Cond,
                                     StmtTy *Body) {
     return 0;

include/clang/Parse/Scope.h

@@ -79,9 +79,6 @@ private:
   typedef llvm::SmallPtrSet<Action::DeclTy*, 32> DeclSetTy;
   DeclSetTy DeclsInScope;
   
-  /// DefaultStmt - when parsing the body of a switch statement, this keeps
-  /// track of the statement with the default label.
-  Action::StmtTy *DefaultStmt;
 public:
   Scope(Scope *Parent, unsigned ScopeFlags) {
     Init(Parent, ScopeFlags);
@@ -118,9 +115,6 @@ public:
     return DeclsInScope.count(D) != 0;
   }
   
-  void setDefaultStmt(Action::StmtTy *S) { DefaultStmt = S; }
-  Action::StmtTy *getDefaultStmt() const { return DefaultStmt; }
-  
   /// Init - This is used by the parser to implement scope caching.
   ///
   void Init(Scope *Parent, unsigned ScopeFlags) {
@@ -138,8 +132,6 @@ public:
       FnParent = BreakParent = ContinueParent = 0;
     }
     
-    DefaultStmt = 0;
-    
     // If this scope is a function or contains breaks/continues, remember it.
     if (Flags & FnScope)       FnParent = this;
     if (Flags & BreakScope)    BreakParent = this;

test/Sema/switch-duplicate-defaults.c

@@ -0,0 +1,10 @@
+// RUN: clang -parse-ast-check %s
+
+void f (int z) { 
+  switch(z) {
+      default:  // expected-error {{first label is here}}
+      default:  // expected-error {{multiple default labels in one switch}}
+        break;
+  }
+} 
+