Step #1/N of implementing support for __label__: split labels into

LabelDecl and LabelStmt.  There is a 1-1 correspondence between the
two, but this simplifies a bunch of code by itself.  This is because
labels are the only place where we previously had references to random
other statements, causing grief for AST serialization and other stuff.

This does cause one regression (attr(unused) doesn't silence unused
label warnings) which I'll address next.

This does fix some minor bugs:
1. "The only valid attribute " diagnostic was capitalized.
2. Various diagnostics printed as ''labelname'' instead of 'labelname'
3. This reduces duplication of label checking between functions and blocks.

Review appreciated, particularly for the cindex and template bits.

llvm-svn: 125733

6 files changed, 126 insertions, 140 deletions

diff --git a/clang/lib/Sema/JumpDiagnostics.cpp b/clang/lib/Sema/JumpDiagnostics.cpp
index bd6b48a30ff..b73f0e9f145 100644
--- a/clang/lib/Sema/JumpDiagnostics.cpp
+++ b/clang/lib/Sema/JumpDiagnostics.cpp
@@ -62,7 +62,7 @@ class JumpScopeChecker {
   llvm::SmallVector<Stmt*, 16> Jumps;
 
   llvm::SmallVector<IndirectGotoStmt*, 4> IndirectJumps;
-  llvm::SmallVector<LabelStmt*, 4> IndirectJumpTargets;
+  llvm::SmallVector<LabelDecl*, 4> IndirectJumpTargets;
 public:
   JumpScopeChecker(Stmt *Body, Sema &S);
 private:
@@ -71,7 +71,7 @@ private:
   void VerifyJumps();
   void VerifyIndirectJumps();
   void DiagnoseIndirectJump(IndirectGotoStmt *IG, unsigned IGScope,
-                            LabelStmt *Target, unsigned TargetScope);
+                            LabelDecl *Target, unsigned TargetScope);
   void CheckJump(Stmt *From, Stmt *To,
                  SourceLocation DiagLoc, unsigned JumpDiag);
 
@@ -235,12 +235,12 @@ void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) {
     // order to avoid blowing out the stack.
     while (true) {
       Stmt *Next;
-      if (isa<CaseStmt>(SubStmt))
-        Next = cast<CaseStmt>(SubStmt)->getSubStmt();
-      else if (isa<DefaultStmt>(SubStmt))
-        Next = cast<DefaultStmt>(SubStmt)->getSubStmt();
-      else if (isa<LabelStmt>(SubStmt))
-        Next = cast<LabelStmt>(SubStmt)->getSubStmt();
+      if (CaseStmt *CS = dyn_cast<CaseStmt>(SubStmt))
+        Next = CS->getSubStmt();
+      else if (DefaultStmt *DS = dyn_cast<DefaultStmt>(SubStmt))
+        Next = DS->getSubStmt();
+      else if (LabelStmt *LS = dyn_cast<LabelStmt>(SubStmt))
+        Next = LS->getSubStmt();
       else
         break;
 
@@ -346,15 +346,15 @@ void JumpScopeChecker::VerifyJumps() {
 
     // With a goto,
     if (GotoStmt *GS = dyn_cast<GotoStmt>(Jump)) {
-      CheckJump(GS, GS->getLabel(), GS->getGotoLoc(),
+      CheckJump(GS, GS->getLabel()->getStmt(), GS->getGotoLoc(),
                 diag::err_goto_into_protected_scope);
       continue;
     }
 
     // We only get indirect gotos here when they have a constant target.
     if (IndirectGotoStmt *IGS = dyn_cast<IndirectGotoStmt>(Jump)) {
-      LabelStmt *Target = IGS->getConstantTarget();
-      CheckJump(IGS, Target, IGS->getGotoLoc(),
+      LabelDecl *Target = IGS->getConstantTarget();
+      CheckJump(IGS, Target->getStmt(), IGS->getGotoLoc(),
                 diag::err_goto_into_protected_scope);
       continue;
     }
@@ -424,15 +424,15 @@ void JumpScopeChecker::VerifyIndirectJumps() {
   // Collect a single representative of every scope containing a
   // label whose address was taken somewhere in the function.
   // For most code bases, there will be only one such scope.
-  llvm::DenseMap<unsigned, LabelStmt*> TargetScopes;
-  for (llvm::SmallVectorImpl<LabelStmt*>::iterator
+  llvm::DenseMap<unsigned, LabelDecl*> TargetScopes;
+  for (llvm::SmallVectorImpl<LabelDecl*>::iterator
          I = IndirectJumpTargets.begin(), E = IndirectJumpTargets.end();
        I != E; ++I) {
-    LabelStmt *TheLabel = *I;
-    assert(LabelAndGotoScopes.count(TheLabel) &&
+    LabelDecl *TheLabel = *I;
+    assert(LabelAndGotoScopes.count(TheLabel->getStmt()) &&
            "Referenced label didn't get added to scopes?");
-    unsigned LabelScope = LabelAndGotoScopes[TheLabel];
-    LabelStmt *&Target = TargetScopes[LabelScope];
+    unsigned LabelScope = LabelAndGotoScopes[TheLabel->getStmt()];
+    LabelDecl *&Target = TargetScopes[LabelScope];
     if (!Target) Target = TheLabel;
   }
 
@@ -445,10 +445,10 @@ void JumpScopeChecker::VerifyIndirectJumps() {
   // entered, then verify that every jump scope can be trivially
   // exitted to reach a scope in S.
   llvm::BitVector Reachable(Scopes.size(), false);
-  for (llvm::DenseMap<unsigned,LabelStmt*>::iterator
+  for (llvm::DenseMap<unsigned,LabelDecl*>::iterator
          TI = TargetScopes.begin(), TE = TargetScopes.end(); TI != TE; ++TI) {
     unsigned TargetScope = TI->first;
-    LabelStmt *TargetLabel = TI->second;
+    LabelDecl *TargetLabel = TI->second;
 
     Reachable.reset();
 
@@ -511,12 +511,12 @@ void JumpScopeChecker::VerifyIndirectJumps() {
 /// Diagnose an indirect jump which is known to cross scopes.
 void JumpScopeChecker::DiagnoseIndirectJump(IndirectGotoStmt *Jump,
                                             unsigned JumpScope,
-                                            LabelStmt *Target,
+                                            LabelDecl *Target,
                                             unsigned TargetScope) {
   assert(JumpScope != TargetScope);
 
   S.Diag(Jump->getGotoLoc(), diag::err_indirect_goto_in_protected_scope);
-  S.Diag(Target->getIdentLoc(), diag::note_indirect_goto_target);
+  S.Diag(Target->getStmt()->getIdentLoc(), diag::note_indirect_goto_target);
 
   unsigned Common = GetDeepestCommonScope(JumpScope, TargetScope);
 
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 8fbbeb85e3e..2e7f72f81c3 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -31,6 +31,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/TargetInfo.h"
@@ -50,6 +51,53 @@ void FunctionScopeInfo::Clear() {
   ErrorTrap.reset();
 }
 
+bool FunctionScopeInfo::checkLabelUse(Stmt *Body, Sema &S) {
+  bool AnyErrors = false;
+  for (llvm::DenseMap<IdentifierInfo*, LabelDecl*>::iterator
+       I = LabelMap.begin(), E = LabelMap.end(); I != E; ++I) {
+    LabelDecl *L = I->second;
+    
+    // Verify that we have no forward references left.  If so, there was a goto
+    // or address of a label taken, but no definition of it.  Label fwd
+    // definitions are indicated with a null substmt.
+    if (L->getStmt() != 0) {
+      if (!L->isUsed())
+        S.Diag(L->getLocation(), diag::warn_unused_label) << L->getDeclName();
+      continue;
+    }
+    
+    AnyErrors = true;
+    
+    // Emit error.
+    S.Diag(L->getLocation(), diag::err_undeclared_label_use) << L->getDeclName();
+    
+    // At this point, we have gotos that use the bogus label.  Stitch it into
+    // the function body so that the AST is well formed.
+    if (Body == 0) {
+      // The whole function wasn't parsed correctly.
+      continue;
+    }
+    
+    // Otherwise, the body is valid: we want to stitch the label decl into the
+    // function somewhere so that it is properly owned and so that the goto
+    // has a valid target.  Do this by creating LabelStmt and adding it to the
+    // end of the outer CompoundStmt.
+    LabelStmt *LS = new (S.Context) LabelStmt(L->getLocation(), L, 
+                                    new (S.Context) NullStmt(L->getLocation()));
+    
+    CompoundStmt *Compound = isa<CXXTryStmt>(Body) ?
+    cast<CXXTryStmt>(Body)->getTryBlock() :
+    cast<CompoundStmt>(Body);
+    llvm::SmallVector<Stmt*, 64> Elements(Compound->body_begin(),
+                                          Compound->body_end());
+    Elements.push_back(LS);
+    Compound->setStmts(S.Context, Elements.data(), Elements.size());
+  }
+  return AnyErrors;
+}
+
+
+
 BlockScopeInfo::~BlockScopeInfo() { }
 
 void Sema::ActOnTranslationUnitScope(Scope *S) {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2807e759b56..bcec9d48393 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -5477,46 +5477,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
 
   // Check goto/label use.
   FunctionScopeInfo *CurFn = getCurFunction();
-  for (llvm::DenseMap<IdentifierInfo*, LabelStmt*>::iterator
-         I = CurFn->LabelMap.begin(), E = CurFn->LabelMap.end(); I != E; ++I) {
-    LabelStmt *L = I->second;
-
-    // Verify that we have no forward references left.  If so, there was a goto
-    // or address of a label taken, but no definition of it.  Label fwd
-    // definitions are indicated with a null substmt.
-    if (L->getSubStmt() != 0) {
-      if (!L->isUsed())
-        Diag(L->getIdentLoc(), diag::warn_unused_label) << L->getName();
-      continue;
-    }
-
-    // Emit error.
-    Diag(L->getIdentLoc(), diag::err_undeclared_label_use) << L->getName();
-
-    // At this point, we have gotos that use the bogus label.  Stitch it into
-    // the function body so that they aren't leaked and that the AST is well
-    // formed.
-    if (Body == 0) {
-      // The whole function wasn't parsed correctly.
-      continue;
-    }
-
-    // Otherwise, the body is valid: we want to stitch the label decl into the
-    // function somewhere so that it is properly owned and so that the goto
-    // has a valid target.  Do this by creating a new compound stmt with the
-    // label in it.
-
-    // Give the label a sub-statement.
-    L->setSubStmt(new (Context) NullStmt(L->getIdentLoc()));
-
-    CompoundStmt *Compound = isa<CXXTryStmt>(Body) ?
-                               cast<CXXTryStmt>(Body)->getTryBlock() :
-                               cast<CompoundStmt>(Body);
-    llvm::SmallVector<Stmt*, 64> Elements(Compound->body_begin(),
-                                          Compound->body_end());
-    Elements.push_back(L);
-    Compound->setStmts(Context, Elements.data(), Elements.size());
-  }
+  CurFn->checkLabelUse(Body, *this);
 
   if (Body) {
     // C++ constructors that have function-try-blocks can't have return
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 0a956079750..94dd27d0c22 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -8321,20 +8321,19 @@ ExprResult Sema::ActOnUnaryOp(Scope *S, SourceLocation OpLoc,
 }
 
 /// ActOnAddrLabel - Parse the GNU address of label extension: "&&foo".
-ExprResult Sema::ActOnAddrLabel(SourceLocation OpLoc,
-                                            SourceLocation LabLoc,
-                                            IdentifierInfo *LabelII) {
+ExprResult Sema::ActOnAddrLabel(SourceLocation OpLoc, SourceLocation LabLoc,
+                                IdentifierInfo *LabelII) {
   // Look up the record for this label identifier.
-  LabelStmt *&LabelDecl = getCurFunction()->LabelMap[LabelII];
+  LabelDecl *&TheDecl = getCurFunction()->LabelMap[LabelII];
 
   // If we haven't seen this label yet, create a forward reference. It
   // will be validated and/or cleaned up in ActOnFinishFunctionBody.
-  if (LabelDecl == 0)
-    LabelDecl = new (Context) LabelStmt(LabLoc, LabelII, 0);
+  if (TheDecl == 0)
+    TheDecl = LabelDecl::Create(Context, CurContext, LabLoc, LabelII);
 
-  LabelDecl->setUsed();
+  TheDecl->setUsed();
   // Create the AST node.  The address of a label always has type 'void*'.
-  return Owned(new (Context) AddrLabelExpr(OpLoc, LabLoc, LabelDecl,
+  return Owned(new (Context) AddrLabelExpr(OpLoc, LabLoc, TheDecl,
                                        Context.getPointerType(Context.VoidTy)));
 }
 
@@ -8833,25 +8832,8 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
 
   BSI->TheDecl->setBody(cast<CompoundStmt>(Body));
 
-  bool Good = true;
   // Check goto/label use.
-  for (llvm::DenseMap<IdentifierInfo*, LabelStmt*>::iterator
-         I = BSI->LabelMap.begin(), E = BSI->LabelMap.end(); I != E; ++I) {
-    LabelStmt *L = I->second;
-
-    // Verify that we have no forward references left.  If so, there was a goto
-    // or address of a label taken, but no definition of it.
-    if (L->getSubStmt() != 0) {
-      if (!L->isUsed())
-        Diag(L->getIdentLoc(), diag::warn_unused_label) << L->getName();
-      continue;
-    }
-
-    // Emit error.
-    Diag(L->getIdentLoc(), diag::err_undeclared_label_use) << L->getName();
-    Good = false;
-  }
-  if (!Good) {
+  if (BSI->checkLabelUse(0, *this)) {
     PopFunctionOrBlockScope();
     return ExprError();
   }
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index e5fd01ab707..2b323ef4b44 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -255,29 +255,31 @@ Sema::ActOnLabelStmt(SourceLocation IdentLoc, IdentifierInfo *II,
                      SourceLocation ColonLoc, Stmt *SubStmt,
                      bool HasUnusedAttr) {
   // Look up the record for this label identifier.
-  LabelStmt *&LabelDecl = getCurFunction()->LabelMap[II];
+  LabelDecl *&TheDecl = getCurFunction()->LabelMap[II];
 
-  // If not forward referenced or defined already, just create a new LabelStmt.
-  if (LabelDecl == 0)
-    return Owned(LabelDecl = new (Context) LabelStmt(IdentLoc, II, SubStmt,
-                                                     HasUnusedAttr));
+  // If not forward referenced or defined already, create the backing decl.
+  if (TheDecl == 0)
+    TheDecl = LabelDecl::Create(Context, CurContext, IdentLoc, II);
 
-  assert(LabelDecl->getID() == II && "Label mismatch!");
+  assert(TheDecl->getIdentifier() == II && "Label mismatch!");
 
-  // Otherwise, this label was either forward reference or multiply defined.  If
-  // multiply defined, reject it now.
-  if (LabelDecl->getSubStmt()) {
-    Diag(IdentLoc, diag::err_redefinition_of_label) << LabelDecl->getID();
-    Diag(LabelDecl->getIdentLoc(), diag::note_previous_definition);
+  // If the label was multiply defined, reject it now.
+  if (TheDecl->getStmt()) {
+    Diag(IdentLoc, diag::err_redefinition_of_label) << TheDecl->getDeclName();
+    Diag(TheDecl->getLocation(), diag::note_previous_definition);
     return Owned(SubStmt);
   }
 
-  // Otherwise, this label was forward declared, and we just found its real
-  // definition.  Fill in the forward definition and return it.
-  LabelDecl->setIdentLoc(IdentLoc);
-  LabelDecl->setSubStmt(SubStmt);
-  LabelDecl->setUnusedAttribute(HasUnusedAttr);
-  return Owned(LabelDecl);
+  // Otherwise, things are good.  Fill in the declaration and return it.
+  TheDecl->setLocation(IdentLoc);
+  
+  // FIXME: Just use Decl ATTRIBUTES!
+  if (HasUnusedAttr)
+    TheDecl->setHasUnusedAttribute();
+  LabelStmt *LS = new (Context) LabelStmt(IdentLoc, TheDecl, SubStmt);
+  TheDecl->setStmt(LS);
+  TheDecl->setLocation(IdentLoc);
+  return Owned(LS);
 }
 
 StmtResult
@@ -1036,17 +1038,17 @@ Sema::ActOnObjCForCollectionStmt(SourceLocation ForLoc,
 StmtResult
 Sema::ActOnGotoStmt(SourceLocation GotoLoc, SourceLocation LabelLoc,
                     IdentifierInfo *LabelII) {
-  // Look up the record for this label identifier.
-  LabelStmt *&LabelDecl = getCurFunction()->LabelMap[LabelII];
-
   getCurFunction()->setHasBranchIntoScope();
+  
+  // Look up the record for this label identifier.
+  LabelDecl *&TheDecl = getCurFunction()->LabelMap[LabelII];
 
   // If we haven't seen this label yet, create a forward reference.
-  if (LabelDecl == 0)
-    LabelDecl = new (Context) LabelStmt(LabelLoc, LabelII, 0);
+  if (TheDecl == 0)
+    TheDecl = LabelDecl::Create(Context, CurContext, LabelLoc, LabelII);
 
-  LabelDecl->setUsed();
-  return Owned(new (Context) GotoStmt(LabelDecl, GotoLoc, LabelLoc));
+  TheDecl->setUsed();
+  return Owned(new (Context) GotoStmt(TheDecl, GotoLoc, LabelLoc));
 }
 
 StmtResult
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index ca5d1c1ea0f..cb0d4250fca 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -975,10 +975,9 @@ public:
   ///
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
-  StmtResult RebuildLabelStmt(SourceLocation IdentLoc,
-                                    IdentifierInfo *Id,
-                                    SourceLocation ColonLoc,
-                                    Stmt *SubStmt, bool HasUnusedAttr) {
+  StmtResult RebuildLabelStmt(SourceLocation IdentLoc, IdentifierInfo *Id,
+                              SourceLocation ColonLoc, Stmt *SubStmt,
+                              bool HasUnusedAttr) {
     return SemaRef.ActOnLabelStmt(IdentLoc, Id, ColonLoc, SubStmt,
                                   HasUnusedAttr);
   }
@@ -1028,10 +1027,8 @@ public:
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
   StmtResult RebuildDoStmt(SourceLocation DoLoc, Stmt *Body,
-                                 SourceLocation WhileLoc,
-                                 SourceLocation LParenLoc,
-                                 Expr *Cond,
-                                 SourceLocation RParenLoc) {
+                           SourceLocation WhileLoc, SourceLocation LParenLoc,
+                           Expr *Cond, SourceLocation RParenLoc) {
     return getSema().ActOnDoStmt(DoLoc, Body, WhileLoc, LParenLoc,
                                  Cond, RParenLoc);
   }
@@ -1040,24 +1037,21 @@ public:
   ///
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
-  StmtResult RebuildForStmt(SourceLocation ForLoc,
-                                  SourceLocation LParenLoc,
-                                  Stmt *Init, Sema::FullExprArg Cond, 
-                                  VarDecl *CondVar, Sema::FullExprArg Inc,
-                                  SourceLocation RParenLoc, Stmt *Body) {
+  StmtResult RebuildForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
+                            Stmt *Init, Sema::FullExprArg Cond, 
+                            VarDecl *CondVar, Sema::FullExprArg Inc,
+                            SourceLocation RParenLoc, Stmt *Body) {
     return getSema().ActOnForStmt(ForLoc, LParenLoc, Init, Cond, 
-                                  CondVar,
-                                  Inc, RParenLoc, Body);
+                                  CondVar, Inc, RParenLoc, Body);
   }
 
   /// \brief Build a new goto statement.
   ///
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
-  StmtResult RebuildGotoStmt(SourceLocation GotoLoc,
-                                   SourceLocation LabelLoc,
-                                   LabelStmt *Label) {
-    return getSema().ActOnGotoStmt(GotoLoc, LabelLoc, Label->getID());
+  StmtResult RebuildGotoStmt(SourceLocation GotoLoc, SourceLocation LabelLoc,
+                             LabelDecl *Label) {
+    return getSema().ActOnGotoStmt(GotoLoc, LabelLoc, Label->getIdentifier());
   }
 
   /// \brief Build a new indirect goto statement.
@@ -1065,8 +1059,8 @@ public:
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
   StmtResult RebuildIndirectGotoStmt(SourceLocation GotoLoc,
-                                           SourceLocation StarLoc,
-                                           Expr *Target) {
+                                     SourceLocation StarLoc,
+                                     Expr *Target) {
     return getSema().ActOnIndirectGotoStmt(GotoLoc, StarLoc, Target);
   }
 
@@ -1074,9 +1068,7 @@ public:
   ///
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
-  StmtResult RebuildReturnStmt(SourceLocation ReturnLoc,
-                                     Expr *Result) {
-
+  StmtResult RebuildReturnStmt(SourceLocation ReturnLoc, Expr *Result) {
     return getSema().ActOnReturnStmt(ReturnLoc, Result);
   }
 
@@ -1550,9 +1542,8 @@ public:
   /// rather than attempting to map the label statement itself.
   /// Subclasses may override this routine to provide different behavior.
   ExprResult RebuildAddrLabelExpr(SourceLocation AmpAmpLoc,
-                                        SourceLocation LabelLoc,
-                                        LabelStmt *Label) {
-    return getSema().ActOnAddrLabel(AmpAmpLoc, LabelLoc, Label->getID());
+                                  SourceLocation LabelLoc, LabelDecl *Label) {
+    return getSema().ActOnAddrLabel(AmpAmpLoc, LabelLoc,Label->getIdentifier());
   }
 
   /// \brief Build a new GNU statement expression.
@@ -4522,8 +4513,10 @@ TreeTransform<Derived>::TransformLabelStmt(LabelStmt *S) {
 
   // FIXME: Pass the real colon location in.
   SourceLocation ColonLoc = SemaRef.PP.getLocForEndOfToken(S->getIdentLoc());
-  return getDerived().RebuildLabelStmt(S->getIdentLoc(), S->getID(), ColonLoc,
-                                       SubStmt.get(), S->HasUnusedAttribute());
+  return getDerived().RebuildLabelStmt(S->getIdentLoc(),
+                                       S->getDecl()->getIdentifier(), ColonLoc,
+                                       SubStmt.get(),
+                                       S->getDecl()->hasUnusedAttribute());
 }
 
 template<typename Derived>