[cfe-commits] r40388 - in /cfe/trunk: AST/StmtPrinter.cpp Parse/ParseStmt.cpp Sema/Sema.h Sema/SemaStmt.cpp include/clang/AST/Stmt.h include/clang/AST/StmtNodes.def include/clang/Parse/Action.h include/clang/Parse/Scope.h test/Sema/switch-duplicate-defaults.c

Anders Carlsson andersca at mac.com
Sun Jul 22 00:07:57 PDT 2007


Author: andersca
Date: Sun Jul 22 02:07:56 2007
New Revision: 40388

URL: http://llvm.org/viewvc/llvm-project?rev=40388&view=rev
Log:
Refactor switch analysis to make it possible to detect duplicate case values

Added:
    cfe/trunk/test/Sema/switch-duplicate-defaults.c
Modified:
    cfe/trunk/AST/StmtPrinter.cpp
    cfe/trunk/Parse/ParseStmt.cpp
    cfe/trunk/Sema/Sema.h
    cfe/trunk/Sema/SemaStmt.cpp
    cfe/trunk/include/clang/AST/Stmt.h
    cfe/trunk/include/clang/AST/StmtNodes.def
    cfe/trunk/include/clang/Parse/Action.h
    cfe/trunk/include/clang/Parse/Scope.h

Modified: cfe/trunk/AST/StmtPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/AST/StmtPrinter.cpp?rev=40388&r1=40387&r2=40388&view=diff

==============================================================================
--- cfe/trunk/AST/StmtPrinter.cpp (original)
+++ cfe/trunk/AST/StmtPrinter.cpp Sun Jul 22 02:07:56 2007
@@ -218,6 +218,10 @@
   }
 }
 
+void StmtPrinter::VisitSwitchCase(SwitchCase*) {
+  assert(0 && "SwitchCase is an abstract class");
+}
+
 void StmtPrinter::VisitWhileStmt(WhileStmt *Node) {
   Indent() << "while (";
   PrintExpr(Node->getCond());

Modified: cfe/trunk/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Parse/ParseStmt.cpp?rev=40388&r1=40387&r2=40388&view=diff

==============================================================================
--- cfe/trunk/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/Parse/ParseStmt.cpp Sun Jul 22 02:07:56 2007
@@ -475,14 +475,24 @@
   // 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();
 
-  ExitScope();
+  if (Body.isInvalid) {
+    Body = Actions.ParseNullStmt(Tok.getLocation());
+    // FIXME: Remove the case statement list from the Switch statement.
+  }
   
-  if (Cond.isInvalid || Body.isInvalid) return true;
+  ExitScope();
   
-  return Actions.ParseSwitchStmt(SwitchLoc, Cond.Val, Body.Val);
+  return Actions.FinishSwitchStmt(SwitchLoc, Switch.Val, Body.Val);
 }
 
 /// ParseWhileStatement

Modified: cfe/trunk/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/Sema/Sema.h?rev=40388&r1=40387&r2=40388&view=diff

==============================================================================
--- cfe/trunk/Sema/Sema.h (original)
+++ cfe/trunk/Sema/Sema.h Sun Jul 22 02:07:56 2007
@@ -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 @@
   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 @@
   /// 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 @@
   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,

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

==============================================================================
--- cfe/trunk/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/Sema/SemaStmt.cpp Sun Jul 22 02:07:56 2007
@@ -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 @@
   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 @@
     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 @@
     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 @@
 }
 
 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;
+  }
+
+  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 new SwitchStmt((Expr*)Cond, (Stmt*)Body);
+  return SS;
 }
 
 Action::StmtResult

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=40388&r1=40387&r2=40388&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Sun Jul 22 02:07:56 2007
@@ -124,15 +124,37 @@
   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 @@
   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 @@
 class SwitchStmt : public Stmt {
   Expr *Cond;
   Stmt *Body;
+  
+  // This points to a linked list of case and default statements.
+  SwitchCase *FirstCase;
 public:
-  SwitchStmt(Expr *cond, Stmt *body)
-    : Stmt(SwitchStmtClass), Cond(cond), Body(body) {}
+  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) { 

Modified: cfe/trunk/include/clang/AST/StmtNodes.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/StmtNodes.def?rev=40388&r1=40387&r2=40388&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/StmtNodes.def (original)
+++ cfe/trunk/include/clang/AST/StmtNodes.def Sun Jul 22 02:07:56 2007
@@ -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(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.

Modified: cfe/trunk/include/clang/Parse/Action.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Action.h?rev=40388&r1=40387&r2=40388&view=diff

==============================================================================
--- cfe/trunk/include/clang/Parse/Action.h (original)
+++ cfe/trunk/include/clang/Parse/Action.h Sun Jul 22 02:07:56 2007
@@ -220,10 +220,15 @@
     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;

Modified: cfe/trunk/include/clang/Parse/Scope.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Scope.h?rev=40388&r1=40387&r2=40388&view=diff

==============================================================================
--- cfe/trunk/include/clang/Parse/Scope.h (original)
+++ cfe/trunk/include/clang/Parse/Scope.h Sun Jul 22 02:07:56 2007
@@ -79,9 +79,6 @@
   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 @@
     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 @@
       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;

Added: cfe/trunk/test/Sema/switch-duplicate-defaults.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch-duplicate-defaults.c?rev=40388&view=auto

==============================================================================
--- cfe/trunk/test/Sema/switch-duplicate-defaults.c (added)
+++ cfe/trunk/test/Sema/switch-duplicate-defaults.c Sun Jul 22 02:07:56 2007
@@ -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;
+  }
+} 
+





More information about the cfe-commits mailing list