[cfe-commits] [PATCH] C++ 11 statement attributes + switch labels fall-through detection

Richard Smith richard at metafoo.co.uk
Thu Apr 5 15:23:33 PDT 2012


Hi Alex,

I think the patch is generally looking pretty good. All of your TODOs seem
like reasonable things to defer to later patches. I have some specific
comments below (mostly formatting-related):

> Index: include/clang/Basic/StmtNodes.td
> ===================================================================
> --- include/clang/Basic/StmtNodes.td (revision 154056)
> +++ include/clang/Basic/StmtNodes.td (working copy)
> @@ -12,6 +12,7 @@
>  def NullStmt : Stmt;
>  def CompoundStmt : Stmt;
>  def LabelStmt : Stmt;
> +def AttributedStmt : Stmt;
>  def IfStmt : Stmt;
>  def SwitchStmt : Stmt;
>  def WhileStmt : Stmt;
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h (revision 154056)
> +++ include/clang/Sema/Sema.h (working copy)
> @@ -1973,6 +1973,10 @@
>    bool CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC);
>    bool CheckNoReturnAttr(const AttributeList &attr);
>
> +  // Stmt attributes - this routine is the top level dispatcher.
> +  StmtResult ProcessStmtAttributes(StmtResult Stmt, AttributeList *Attrs,
> +                                   SourceRange Range);

Please pass a Stmt * to this function, not a StmtResult. It's not meaningful
to pass an invalid statement in here.

> +
>    void WarnUndefinedMethod(SourceLocation ImpLoc, ObjCMethodDecl *method,
>                             bool &IncompleteImpl, unsigned DiagID);
>    void WarnConflictingTypedMethods(ObjCMethodDecl *Method,
> @@ -2250,6 +2254,9 @@
>    StmtResult ActOnLabelStmt(SourceLocation IdentLoc, LabelDecl *TheDecl,
>                              SourceLocation ColonLoc, Stmt *SubStmt);
>
> +  StmtResult ActOnAttributedStmt(SourceLocation AttrLoc, const AttrVec
&Attrs,
> +                                 Stmt *SubStmt);
> +
>    StmtResult ActOnIfStmt(SourceLocation IfLoc,
>                           FullExprArg CondVal, Decl *CondVar,
>                           Stmt *ThenVal,
> Index: include/clang/AST/DeclBase.h
> ===================================================================
> --- include/clang/AST/DeclBase.h (revision 154056)
> +++ include/clang/AST/DeclBase.h (working copy)
> @@ -861,7 +861,6 @@
>    void dumpXML(raw_ostream &OS) const;
>
>  private:
> -  const Attr *getAttrsImpl() const;
>    void setAttrsImpl(const AttrVec& Attrs, ASTContext &Ctx);
>    void setDeclContextsImpl(DeclContext *SemaDC, DeclContext *LexicalDC,
>                             ASTContext &Ctx);
> Index: include/clang/AST/Stmt.h
> ===================================================================
> --- include/clang/AST/Stmt.h (revision 154056)
> +++ include/clang/AST/Stmt.h (working copy)
> @@ -20,6 +20,7 @@
>  #include "clang/AST/StmtIterator.h"
>  #include "clang/AST/DeclGroup.h"
>  #include "clang/AST/ASTContext.h"
> +#include "clang/AST/Attr.h"
>  #include "llvm/ADT/SmallVector.h"
>  #include "llvm/Support/Compiler.h"
>  #include "llvm/Support/raw_ostream.h"
> @@ -794,6 +795,43 @@
>  };
>
>
> +/// AttributedStmt - Represents an attribute applied to a statement. For
example:

Please wrap this line to 80 columns.

> +///   [[omp::for(...)]] for (...) { ... }
> +///
> +class AttributedStmt : public Stmt {
> +  Stmt *SubStmt;
> +  SourceLocation AttrLoc;
> +  AttrVec Attrs;
> +  // !!! TODO: It can be done as Attr *Attrs[1]; and variable size array
as in
> +  // StringLiteral

Dealing with this as a separate commit is fine. Drop the "!!!", though.

> +
> +  friend class ASTStmtReader;
> +
> +public:
> +  AttributedStmt(SourceLocation loc, const AttrVec &attrs, Stmt *substmt)
> +    : Stmt(AttributedStmtClass), SubStmt(substmt), AttrLoc(loc),
Attrs(attrs) {
> +  }
> +
> +  // \brief Build an empty attributed statement.
> +  explicit AttributedStmt(EmptyShell Empty) : Stmt(AttributedStmtClass,
Empty) { }

Please wrap this line to 80 columns.

> +
> +  SourceLocation getAttrLoc() const { return AttrLoc; }
> +  const AttrVec &getAttrs() const { return Attrs; }
> +  Stmt *getSubStmt() { return SubStmt; }
> +  const Stmt *getSubStmt() const { return SubStmt; }
> +
> +  SourceRange getSourceRange() const LLVM_READONLY {
> +    return SourceRange(AttrLoc, SubStmt->getLocEnd());
> +  }
> +  child_range children() { return child_range(&SubStmt, &SubStmt+1); }
> +
> +  static bool classof(const Stmt *T) {
> +    return T->getStmtClass() == AttributedStmtClass;
> +  }
> +  static bool classof(const AttributedStmt *) { return true; }
> +};
> +
> +
>  /// IfStmt - This represents an if/then/else.
>  ///
>  class IfStmt : public Stmt {
> Index: include/clang/AST/RecursiveASTVisitor.h
> ===================================================================
> --- include/clang/AST/RecursiveASTVisitor.h (revision 154056)
> +++ include/clang/AST/RecursiveASTVisitor.h (working copy)
> @@ -1870,6 +1870,7 @@
>  DEF_TRAVERSE_STMT(IfStmt, { })
>  DEF_TRAVERSE_STMT(IndirectGotoStmt, { })
>  DEF_TRAVERSE_STMT(LabelStmt, { })
> +DEF_TRAVERSE_STMT(AttributedStmt, { })
>  DEF_TRAVERSE_STMT(NullStmt, { })
>  DEF_TRAVERSE_STMT(ObjCAtCatchStmt, { })
>  DEF_TRAVERSE_STMT(ObjCAtFinallyStmt, { })
> Index: include/clang/Parse/Parser.h
> ===================================================================
> --- include/clang/Parse/Parser.h (revision 154056)
> +++ include/clang/Parse/Parser.h (working copy)
> @@ -1510,37 +1510,34 @@
>      return ParseStatementOrDeclaration(Stmts, true, TrailingElseLoc);
>    }
>    StmtResult ParseStatementOrDeclaration(StmtVector& Stmts,
> -                                         bool OnlyStatement,
> +                                        bool OnlyStatement,
>                                          SourceLocation *TrailingElseLoc
= NULL);

"= 0" is much more common in this file than "= NULL". Please change the "&
" to
" &", too.

> -  StmtResult ParseExprStatement(ParsedAttributes &Attrs);
> +  StmtResult ParseStatementOrDeclarationAfterAttributes(StmtVector&
Stmts,
> +                                        bool OnlyStatement,
> +                                        SourceLocation *TrailingElseLoc,
> +                                        ParsedAttributesWithRange
&Attrs);

Please insert a newline after the '(' and line up all the parameters, and
change
the "& " to " &".

> +  StmtResult ParseExprStatement();
>    StmtResult ParseLabeledStatement(ParsedAttributes &Attr);
> -  StmtResult ParseCaseStatement(ParsedAttributes &Attr,
> -                                bool MissingCase = false,
> +  StmtResult ParseCaseStatement(bool MissingCase = false,
>                                  ExprResult Expr = ExprResult());
> -  StmtResult ParseDefaultStatement(ParsedAttributes &Attr);
> -  StmtResult ParseCompoundStatement(ParsedAttributes &Attr,
> -                                    bool isStmtExpr = false);
> -  StmtResult ParseCompoundStatement(ParsedAttributes &Attr,
> -                                    bool isStmtExpr,
> +  StmtResult ParseDefaultStatement();
> +  StmtResult ParseCompoundStatement(bool isStmtExpr = false);
> +  StmtResult ParseCompoundStatement(bool isStmtExpr,
>                                      unsigned ScopeFlags);
>    StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
>    bool ParseParenExprOrCondition(ExprResult &ExprResult,
>                                   Decl *&DeclResult,
>                                   SourceLocation Loc,
>                                   bool ConvertToBoolean);
> -  StmtResult ParseIfStatement(ParsedAttributes &Attr,
> -                              SourceLocation *TrailingElseLoc);
> -  StmtResult ParseSwitchStatement(ParsedAttributes &Attr,
> -                                  SourceLocation *TrailingElseLoc);
> -  StmtResult ParseWhileStatement(ParsedAttributes &Attr,
> -                                 SourceLocation *TrailingElseLoc);
> -  StmtResult ParseDoStatement(ParsedAttributes &Attr);
> -  StmtResult ParseForStatement(ParsedAttributes &Attr,
> -                               SourceLocation *TrailingElseLoc);
> -  StmtResult ParseGotoStatement(ParsedAttributes &Attr);
> -  StmtResult ParseContinueStatement(ParsedAttributes &Attr);
> -  StmtResult ParseBreakStatement(ParsedAttributes &Attr);
> -  StmtResult ParseReturnStatement(ParsedAttributes &Attr);
> +  StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc);
> +  StmtResult ParseSwitchStatement(SourceLocation *TrailingElseLoc);
> +  StmtResult ParseWhileStatement(SourceLocation *TrailingElseLoc);
> +  StmtResult ParseDoStatement();
> +  StmtResult ParseForStatement(SourceLocation *TrailingElseLoc);
> +  StmtResult ParseGotoStatement();
> +  StmtResult ParseContinueStatement();
> +  StmtResult ParseBreakStatement();
> +  StmtResult ParseReturnStatement();
>    StmtResult ParseAsmStatement(bool &msAsm);
>    StmtResult ParseMicrosoftAsmStatement(SourceLocation AsmLoc);
>
> @@ -1590,14 +1587,14 @@
>
 //===--------------------------------------------------------------------===//
>    // C++ 6: Statements and Blocks
>
> -  StmtResult ParseCXXTryBlock(ParsedAttributes &Attr);
> +  StmtResult ParseCXXTryBlock();
>    StmtResult ParseCXXTryBlockCommon(SourceLocation TryLoc);
>    StmtResult ParseCXXCatchBlock();
>
>
 //===--------------------------------------------------------------------===//
>    // MS: SEH Statements and Blocks
>
> -  StmtResult ParseSEHTryBlock(ParsedAttributes &Attr);
> +  StmtResult ParseSEHTryBlock();
>    StmtResult ParseSEHTryBlockCommon(SourceLocation Loc);
>    StmtResult ParseSEHExceptBlock(SourceLocation Loc);
>    StmtResult ParseSEHFinallyBlock(SourceLocation Loc);
> @@ -1942,7 +1939,7 @@
>
>    void ParseTypeofSpecifier(DeclSpec &DS);
>    SourceLocation ParseDecltypeSpecifier(DeclSpec &DS);
> -  void AnnotateExistingDecltypeSpecifier(const DeclSpec &DS,
> +  void AnnotateExistingDecltypeSpecifier(const DeclSpec &DS,
>                                           SourceLocation StartLoc,
>                                           SourceLocation EndLoc);
>    void ParseUnderlyingTypeSpecifier(DeclSpec &DS);
> Index: include/clang/Serialization/ASTBitCodes.h
> ===================================================================
> --- include/clang/Serialization/ASTBitCodes.h (revision 154056)
> +++ include/clang/Serialization/ASTBitCodes.h (working copy)
> @@ -964,6 +964,8 @@
>        STMT_DEFAULT,
>        /// \brief A LabelStmt record.
>        STMT_LABEL,
> +      /// \brief An AttributedStmt record.
> +      STMT_ATTRIBUTED,
>        /// \brief An IfStmt record.
>        STMT_IF,
>        /// \brief A SwitchStmt record.
> Index: include/clang/Serialization/ASTWriter.h
> ===================================================================
> --- include/clang/Serialization/ASTWriter.h (revision 154056)
> +++ include/clang/Serialization/ASTWriter.h (working copy)
> @@ -76,6 +76,7 @@
>    typedef SmallVectorImpl<uint64_t> RecordDataImpl;
>
>    friend class ASTDeclWriter;
> +  friend class ASTStmtWriter;
>  private:
>    /// \brief Map that provides the ID numbers of each type within the
>    /// output stream, plus those deserialized from a chained PCH.
> Index: tools/libclang/CXCursor.cpp
> ===================================================================
> --- tools/libclang/CXCursor.cpp (revision 154056)
> +++ tools/libclang/CXCursor.cpp (working copy)
> @@ -247,19 +247,23 @@
>    case Stmt::CompoundStmtClass:
>      K = CXCursor_CompoundStmt;
>      break;
> -
> +
>    case Stmt::NullStmtClass:
>      K = CXCursor_NullStmt;
>      break;
> -
> +
>    case Stmt::LabelStmtClass:
>      K = CXCursor_LabelStmt;
>      break;
> -
> +
> +  case Stmt::AttributedStmtClass:
> +    K = CXCursor_UnexposedStmt;
> +    break;
> +
>    case Stmt::DeclStmtClass:
>      K = CXCursor_DeclStmt;
>      break;
> -
> +
>    case Stmt::IntegerLiteralClass:
>      K = CXCursor_IntegerLiteral;
>      break;
> @@ -287,7 +291,7 @@
>    case Stmt::UnaryOperatorClass:
>      K = CXCursor_UnaryOperator;
>      break;
> -
> +
>    case Stmt::CXXNoexceptExprClass:
>      K = CXCursor_UnaryExpr;
>      break;
> Index: lib/Analysis/CFG.cpp
> ===================================================================
> --- lib/Analysis/CFG.cpp (revision 154056)
> +++ lib/Analysis/CFG.cpp (working copy)
> @@ -18,6 +18,7 @@
>  #include "clang/AST/StmtVisitor.h"
>  #include "clang/AST/PrettyPrinter.h"
>  #include "clang/AST/CharUnits.h"
> +#include "clang/Basic/AttrKinds.h"
>  #include "llvm/Support/GraphWriter.h"
>  #include "llvm/Support/Allocator.h"
>  #include "llvm/Support/Format.h"
> @@ -342,6 +343,7 @@
>    CFGBlock *VisitImplicitCastExpr(ImplicitCastExpr *E, AddStmtChoice
asc);
>    CFGBlock *VisitIndirectGotoStmt(IndirectGotoStmt *I);
>    CFGBlock *VisitLabelStmt(LabelStmt *L);
> +  CFGBlock *VisitAttributedStmt(AttributedStmt *L);
>    CFGBlock *VisitMemberExpr(MemberExpr *M, AddStmtChoice asc);
>    CFGBlock *VisitObjCAtCatchStmt(ObjCAtCatchStmt *S);
>    CFGBlock *VisitObjCAutoreleasePoolStmt(ObjCAutoreleasePoolStmt *S);
> @@ -1064,6 +1066,9 @@
>      case Stmt::LabelStmtClass:
>        return VisitLabelStmt(cast<LabelStmt>(S));
>
> +    case Stmt::AttributedStmtClass:
> +      return VisitAttributedStmt(cast<AttributedStmt>(S));

Can you replace these changes with:
         return Visit(cast<AttributedStmt>(S)->getSubStmt(), ast);
? (That is, make AttributedStmt invisible to CFG building.)

> +
>      case Stmt::MemberExprClass:
>        return VisitMemberExpr(cast<MemberExpr>(S), asc);
>
> @@ -1126,7 +1131,7 @@
>
>  /// VisitChildren - Visit the children of a Stmt.
>  CFGBlock *CFGBuilder::VisitChildren(Stmt *Terminator) {
> -  CFGBlock *lastBlock = Block;
> +  CFGBlock *lastBlock = Block;
>    for (Stmt::child_range I = Terminator->children(); I; ++I)
>      if (Stmt *child = *I)
>        if (CFGBlock *b = Visit(child))
> @@ -1716,6 +1721,29 @@
>    return LabelBlock;
>  }
>
> +CFGBlock *CFGBuilder::VisitAttributedStmt(AttributedStmt *L) {
> +  // Get the block of the labeled statement.  Add it to our map.
> +  addStmt(L->getSubStmt());
> +  CFGBlock *InnerBlock = Block;
> +
> +  if (!InnerBlock)              // This can happen when the body is
empty, i.e.
> +    InnerBlock = createBlock(); // scopes that only contains NullStmts.
> +
> +  // we could add AttributedStmt to CFGBlock as a label or as an element
> +  // currently we don't do this as it is not necessary
> +  // InnerBlock->setLabel(L);
> +  if (badCFG)
> +    return 0;
> +
> +  // We set Block to NULL to allow lazy creation of a new block (if
necessary);
> +  Block = NULL;
> +
> +  // This block is now the implicit successor of other blocks.
> +  Succ = InnerBlock;
> +
> +  return InnerBlock;
> +}
> +
>  CFGBlock *CFGBuilder::VisitGotoStmt(GotoStmt *G) {
>    // Goto is a control-flow statement.  Thus we stop processing the
current
>    // block and create a new one.
> Index: lib/Sema/TreeTransform.h
> ===================================================================
> --- lib/Sema/TreeTransform.h (revision 154056)
> +++ lib/Sema/TreeTransform.h (working copy)
> @@ -1044,6 +1044,15 @@
>      return SemaRef.ActOnLabelStmt(IdentLoc, L, ColonLoc, SubStmt);
>    }
>
> +  /// \brief Build a new label statement.
> +  ///
> +  /// By default, performs semantic analysis to build the new statement.
> +  /// Subclasses may override this routine to provide different behavior.
> +  StmtResult RebuildAttributedStmt(SourceLocation AttrLoc, const AttrVec
&Attrs,
> +                              Stmt *SubStmt) {
> +    return SemaRef.ActOnAttributedStmt(AttrLoc, Attrs, SubStmt);
> +  }
> +
>    /// \brief Build a new "if" statement.
>    ///
>    /// By default, performs semantic analysis to build the new statement.
> @@ -5154,8 +5163,8 @@
>                                          S->getDecl());
>    if (!LD)
>      return StmtError();
> -
> -
> +
> +
>    // FIXME: Pass the real colon location in.
>    return getDerived().RebuildLabelStmt(S->getIdentLoc(),
>                                         cast<LabelDecl>(LD),
SourceLocation(),
> @@ -5164,6 +5173,22 @@
>
>  template<typename Derived>
>  StmtResult
> +TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S) {
> +  StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt());
> +  if (SubStmt.isInvalid())
> +    return StmtError();
> +
> +  // !!! TODO: transform attributes?

We'll need to do this eventually (but it need not be done in the initial
commit
for statement attributes).

> +  if (SubStmt.get() == S->getSubStmt() /* && attrs are the same */)
> +    return S;
> +
> +  return getDerived().RebuildAttributedStmt(S->getAttrLoc(),
> +                                            S->getAttrs(), //
SourceLocation(),

Remove the commented-out code here.

> +                                            SubStmt.get());
> +}
> +
> +template<typename Derived>
> +StmtResult
>  TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
>    // Transform the condition
>    ExprResult Cond;
> Index: lib/Sema/SemaStmtAttr.cpp
> ===================================================================
> --- lib/Sema/SemaStmtAttr.cpp (revision 0)
> +++ lib/Sema/SemaStmtAttr.cpp (revision 0)
> @@ -0,0 +1,44 @@
> +//===--- SemaStmtAttr.cpp - Statement Attribute Handling
------------------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
+//===----------------------------------------------------------------------===//
> +//
> +//  This file implements stmt-related attribute processing.
> +//
>
+//===----------------------------------------------------------------------===//
> +
> +#include "clang/Sema/SemaInternal.h"
> +#include "TargetAttributesSema.h"
> +#include "clang/AST/ASTContext.h"
> +#include "clang/Basic/SourceManager.h"
> +#include "clang/Sema/DelayedDiagnostic.h"
> +#include "clang/Sema/Lookup.h"
> +#include "llvm/ADT/StringExtras.h"
> +using namespace clang;
> +using namespace sema;
> +
> +
> +static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const AttributeList
&A) {
> +  switch (A.getKind()) {
> +    default:

case/default labels in switches are usually not indented in clang.

> +      return 0;

You should issue a diag::warn_unknown_attribute_ignored diagnostic in this
case
for a GNU attribute, and an error for a C++11 attribute.

> +  }
> +}
> +
> +StmtResult Sema::ProcessStmtAttributes(StmtResult S, AttributeList
*AttrList,
> +                                  SourceRange Range) {

Please line up this parameter declaration.

> +  AttrVec Attrs;
> +  for (const AttributeList* l = AttrList; l; l = l->getNext()) {
> +    if (Attr *a = ProcessStmtAttribute(*this, S.get(), *l))
> +      Attrs.push_back(a);
> +  }
> +
> +  if (Attrs.empty())
> +    return S;
> +
> +  return ActOnAttributedStmt(Range.getBegin(), Attrs, S.get());
> +}
> Index: lib/Sema/CMakeLists.txt
> ===================================================================
> --- lib/Sema/CMakeLists.txt (revision 154056)
> +++ lib/Sema/CMakeLists.txt (working copy)
> @@ -40,6 +40,7 @@
>    SemaOverload.cpp
>    SemaPseudoObject.cpp
>    SemaStmt.cpp
> +  SemaStmtAttr.cpp
>    SemaTemplate.cpp
>    SemaTemplateDeduction.cpp
>    SemaTemplateInstantiate.cpp
> Index: lib/Sema/SemaStmt.cpp
> ===================================================================
> --- lib/Sema/SemaStmt.cpp (revision 154056)
> +++ lib/Sema/SemaStmt.cpp (working copy)
> @@ -345,7 +345,6 @@
>  StmtResult
>  Sema::ActOnLabelStmt(SourceLocation IdentLoc, LabelDecl *TheDecl,
>                       SourceLocation ColonLoc, Stmt *SubStmt) {
> -
>    // If the label was multiply defined, reject it now.
>    if (TheDecl->getStmt()) {
>      Diag(IdentLoc, diag::err_redefinition_of_label) <<
TheDecl->getDeclName();
> @@ -362,6 +361,15 @@
>  }
>
>  StmtResult
> +Sema::ActOnAttributedStmt(SourceLocation AttrLoc, const AttrVec &Attrs
/* !!! TODO: replace with ArrayRef<Attr*> Attrs */,

Please remove the comment prior to checkin. Switching this over to ArrayRef
can
wait until you start using a variable-length allocation for AttributedStmt
(at
which point TreeTransform will need it).

> +                          Stmt *SubStmt) {
> +  // Fill in the declaration and return it. Variable length will require
to
> +  // change this to AttributedStmt::Create(Context, ....);
> +  AttributedStmt *LS = new (Context) AttributedStmt(AttrLoc, Attrs,
SubStmt);
> +  return Owned(LS);
> +}
> +
> +StmtResult
>  Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl
*CondVar,
>                    Stmt *thenStmt, SourceLocation ElseLoc,
>                    Stmt *elseStmt) {
> Index: lib/AST/Stmt.cpp
> ===================================================================
> --- lib/AST/Stmt.cpp (revision 154056)
> +++ lib/AST/Stmt.cpp (working copy)
> @@ -106,6 +106,8 @@
>        S = LS->getSubStmt();
>      else if (const SwitchCase *SC = dyn_cast<SwitchCase>(S))
>        S = SC->getSubStmt();
> +    else if (const AttributedStmt *AS = dyn_cast<AttributedStmt>(S))
> +      S = AS->getSubStmt();

Please update the doxygen comment for this function.

>      else
>        return S;
>    }
> Index: lib/AST/StmtPrinter.cpp
> ===================================================================
> --- lib/AST/StmtPrinter.cpp (revision 154056)
> +++ lib/AST/StmtPrinter.cpp (working copy)
> @@ -169,6 +169,23 @@
>    PrintStmt(Node->getSubStmt(), 0);
>  }
>
> +void StmtPrinter::VisitAttributedStmt(AttributedStmt *Node) {
> +  OS << "[[";
> +  bool first = true;
> +  for (AttrVec::const_iterator it = Node->getAttrs().begin(),
> +                               end = Node->getAttrs().end();
> +                               it != end; ++it) {
> +    if (!first) {
> +      OS << ", ";
> +      first = false;
> +    }
> +    // !!! TODO: check this
> +    (*it)->printPretty(OS, Context);
> +  }
> +  OS << "]] ";
> +  PrintStmt(Node->getSubStmt(), 0);
> +}
> +
>  void StmtPrinter::PrintRawIfStmt(IfStmt *If) {
>    OS << "if (";
>    PrintExpr(If->getCond());
> Index: lib/AST/StmtProfile.cpp
> ===================================================================
> --- lib/AST/StmtProfile.cpp (revision 154056)
> +++ lib/AST/StmtProfile.cpp (working copy)
> @@ -109,6 +109,11 @@
>    VisitDecl(S->getDecl());
>  }
>
> +void StmtProfiler::VisitAttributedStmt(const AttributedStmt *S) {
> +  VisitStmt(S);
> +  // !!!! TODO: maybe visit attributes?

Hmm, tricky. So long as we don't have any statement attributes which affect
the semantics of the program, I think this should be OK. Eventually, we
should
probably generate profiling code for attributes.

> +}
> +
>  void StmtProfiler::VisitIfStmt(const IfStmt *S) {
>    VisitStmt(S);
>    VisitDecl(S->getConditionVariable());
> Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Core/ExprEngine.cpp (revision 154056)
> +++ lib/StaticAnalyzer/Core/ExprEngine.cpp (working copy)
> @@ -536,6 +536,7 @@
>      case Stmt::IfStmtClass:
>      case Stmt::IndirectGotoStmtClass:
>      case Stmt::LabelStmtClass:
> +    case Stmt::AttributedStmtClass:
>      case Stmt::NoStmtClass:
>      case Stmt::NullStmtClass:
>      case Stmt::SwitchStmtClass:
> Index: lib/CodeGen/CGStmt.cpp
> ===================================================================
> --- lib/CodeGen/CGStmt.cpp (revision 154056)
> +++ lib/CodeGen/CGStmt.cpp (working copy)
> @@ -79,6 +79,7 @@
>    case Stmt::CompoundStmtClass:
>    case Stmt::DeclStmtClass:
>    case Stmt::LabelStmtClass:
> +  case Stmt::AttributedStmtClass:
>    case Stmt::GotoStmtClass:
>    case Stmt::BreakStmtClass:
>    case Stmt::ContinueStmtClass:
> @@ -173,6 +174,8 @@
>    case Stmt::CompoundStmtClass:
EmitCompoundStmt(cast<CompoundStmt>(*S)); break;
>    case Stmt::DeclStmtClass:     EmitDeclStmt(cast<DeclStmt>(*S));
  break;
>    case Stmt::LabelStmtClass:    EmitLabelStmt(cast<LabelStmt>(*S));
  break;
> +  case Stmt::AttributedStmtClass:
> +
 EmitAttributedStmt(cast<AttributedStmt>(*S)); break;
>    case Stmt::GotoStmtClass:     EmitGotoStmt(cast<GotoStmt>(*S));
  break;
>    case Stmt::BreakStmtClass:    EmitBreakStmt(cast<BreakStmt>(*S));
  break;
>    case Stmt::ContinueStmtClass:
EmitContinueStmt(cast<ContinueStmt>(*S)); break;
> @@ -332,6 +335,10 @@
>    EmitStmt(S.getSubStmt());
>  }
>
> +void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) {
> +  EmitStmt(S.getSubStmt());
> +}
> +
>  void CodeGenFunction::EmitGotoStmt(const GotoStmt &S) {
>    // If this code is reachable then emit a stop point (if generating
>    // debug info). We have to do this ourselves because we are on the
> Index: lib/CodeGen/CodeGenFunction.h
> ===================================================================
> --- lib/CodeGen/CodeGenFunction.h (revision 154056)
> +++ lib/CodeGen/CodeGenFunction.h (working copy)
> @@ -1948,6 +1948,7 @@
>    void EmitLabel(const LabelDecl *D); // helper for EmitLabelStmt.
>
>    void EmitLabelStmt(const LabelStmt &S);
> +  void EmitAttributedStmt(const AttributedStmt &S);
>    void EmitGotoStmt(const GotoStmt &S);
>    void EmitIndirectGotoStmt(const IndirectGotoStmt &S);
>    void EmitIfStmt(const IfStmt &S);
> Index: lib/Parse/ParseExpr.cpp
> ===================================================================
> --- lib/Parse/ParseExpr.cpp (revision 154056)
> +++ lib/Parse/ParseExpr.cpp (working copy)
> @@ -1918,11 +1918,9 @@
>    // unless they've already reported an error.
>    if (ExprType >= CompoundStmt && Tok.is(tok::l_brace)) {
>      Diag(Tok, diag::ext_gnu_statement_expr);
> -
>      Actions.ActOnStartStmtExpr();
>
> -    ParsedAttributes attrs(AttrFactory);
> -    StmtResult Stmt(ParseCompoundStatement(attrs, true));
> +    StmtResult Stmt(ParseCompoundStatement(true));
>      ExprType = CompoundStmt;
>
>      // If the substmt parsed correctly, build the AST node.
> Index: lib/Parse/ParseStmt.cpp
> ===================================================================
> --- lib/Parse/ParseStmt.cpp (revision 154056)
> +++ lib/Parse/ParseStmt.cpp (working copy)
> @@ -78,14 +78,28 @@
>  StmtResult
>  Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool
OnlyStatement,
>                                      SourceLocation *TrailingElseLoc) {
> -  const char *SemiError = 0;
> -  StmtResult Res;
>
>    ParenBraceBracketBalancer BalancerRAIIObj(*this);
>
> -  ParsedAttributesWithRange attrs(AttrFactory);
> -  MaybeParseCXX0XAttributes(attrs);
> +  ParsedAttributesWithRange Attrs(AttrFactory);
> +  MaybeParseCXX0XAttributes(Attrs);
>
> +  StmtResult Res = ParseStatementOrDeclarationAfterAttributes(Stmts,
> +                                 OnlyStatement, TrailingElseLoc, Attrs);
> +
> +  if (Attrs.empty() || Res.isInvalid() || !Res.isUsable())

Res.isInvalid() is covered by !Res.isUsable(); you don't need both.

If Res is null but valid, and we have attributes, then something has gone
wrong somewhere. Can you add an assert for that case? (But see below...)

> +    return Res;
> +
> +  return Actions.ProcessStmtAttributes(Res, Attrs.getList(),
Attrs.Range);
> +}
> +
> +StmtResult
> +Parser::ParseStatementOrDeclarationAfterAttributes(StmtVector &Stmts,
> +          bool OnlyStatement, SourceLocation *TrailingElseLoc,
> +          ParsedAttributesWithRange &Attrs) {
> +  const char *SemiError = 0;
> +  StmtResult Res;
> +
>    // Cases in this switch statement should fall through if the parser
expects
>    // the token to end in a semicolon (in which case SemiError should be
set),
>    // or they directly 'return;' if not.
> @@ -95,6 +109,7 @@
>    switch (Kind) {
>    case tok::at: // May be a @try or @throw statement
>      {
> +      ProhibitAttributes(Attrs); // TODO: is it correct?
>        AtLoc = ConsumeToken();  // consume @
>        return ParseObjCAtStatement(AtLoc);
>      }
> @@ -108,7 +123,7 @@
>      Token Next = NextToken();
>      if (Next.is(tok::colon)) { // C99 6.8.1: labeled-statement
>        // identifier ':' statement
> -      return ParseLabeledStatement(attrs);
> +      return ParseLabeledStatement(Attrs);

We should be clearing Attrs somewhere (either here or in
ParseLabeledStatement)
so we don't attach them to both the label declaration and the label
statement.

I would prefer to see them cleared in ParseLabeledStatement, since that's
where
they're consumed.

>      }
>
>      if (Next.isNot(tok::coloncolon)) {
> @@ -208,9 +223,10 @@
>
>    default: {
>      if ((getLangOpts().CPlusPlus || !OnlyStatement) &&
isDeclarationStatement()) {
> +      ProhibitAttributes(Attrs);
>        SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
>        DeclGroupPtrTy Decl = ParseDeclaration(Stmts,
Declarator::BlockContext,
> -                                             DeclEnd, attrs);
> +                                             DeclEnd, Attrs);

Likewise here, we don't want to attach the attributes to both the
declarations
and the declaration statement.

>        return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
>      }
>
> @@ -219,54 +235,54 @@
>        return StmtError();
>      }
>
> -    return ParseExprStatement(attrs);
> +    return ParseExprStatement();
>    }
>
>    case tok::kw_case:                // C99 6.8.1: labeled-statement
> -    return ParseCaseStatement(attrs);
> +    return ParseCaseStatement();
>    case tok::kw_default:             // C99 6.8.1: labeled-statement
> -    return ParseDefaultStatement(attrs);
> +    return ParseDefaultStatement();
>
>    case tok::l_brace:                // C99 6.8.2: compound-statement
> -    return ParseCompoundStatement(attrs);
> +    return ParseCompoundStatement();
>    case tok::semi: {                 // C99 6.8.3p3: expression[opt] ';'
>      bool HasLeadingEmptyMacro = Tok.hasLeadingEmptyMacro();
>      return Actions.ActOnNullStmt(ConsumeToken(), HasLeadingEmptyMacro);
>    }
>
>    case tok::kw_if:                  // C99 6.8.4.1: if-statement
> -    return ParseIfStatement(attrs, TrailingElseLoc);
> +    return ParseIfStatement(TrailingElseLoc);
>    case tok::kw_switch:              // C99 6.8.4.2: switch-statement
> -    return ParseSwitchStatement(attrs, TrailingElseLoc);
> +    return ParseSwitchStatement(TrailingElseLoc);
>
>    case tok::kw_while:               // C99 6.8.5.1: while-statement
> -    return ParseWhileStatement(attrs, TrailingElseLoc);
> +    return ParseWhileStatement(TrailingElseLoc);
>    case tok::kw_do:                  // C99 6.8.5.2: do-statement
> -    Res = ParseDoStatement(attrs);
> +    Res = ParseDoStatement();
>      SemiError = "do/while";
>      break;
>    case tok::kw_for:                 // C99 6.8.5.3: for-statement
> -    return ParseForStatement(attrs, TrailingElseLoc);
> +    return ParseForStatement(TrailingElseLoc);
>
>    case tok::kw_goto:                // C99 6.8.6.1: goto-statement
> -    Res = ParseGotoStatement(attrs);
> +    Res = ParseGotoStatement();
>      SemiError = "goto";
>      break;
>    case tok::kw_continue:            // C99 6.8.6.2: continue-statement
> -    Res = ParseContinueStatement(attrs);
> +    Res = ParseContinueStatement();
>      SemiError = "continue";
>      break;
>    case tok::kw_break:               // C99 6.8.6.3: break-statement
> -    Res = ParseBreakStatement(attrs);
> +    Res = ParseBreakStatement();
>      SemiError = "break";
>      break;
>    case tok::kw_return:              // C99 6.8.6.4: return-statement
> -    Res = ParseReturnStatement(attrs);
> +    Res = ParseReturnStatement();
>      SemiError = "return";
>      break;
>
>    case tok::kw_asm: {
> -    ProhibitAttributes(attrs);
> +    ProhibitAttributes(Attrs);
>      bool msAsm = false;
>      Res = ParseAsmStatement(msAsm);
>      Res = Actions.ActOnFinishFullStmt(Res.get());
> @@ -276,16 +292,19 @@
>    }
>
>    case tok::kw_try:                 // C++ 15: try-block
> -    return ParseCXXTryBlock(attrs);
> +    return ParseCXXTryBlock();
>
>    case tok::kw___try:
> -    return ParseSEHTryBlock(attrs);
> +    ProhibitAttributes(Attrs); // TODO: is it correct?
> +    return ParseSEHTryBlock();
>
>    case tok::annot_pragma_vis:
> +    ProhibitAttributes(Attrs); // TODO: is it correct?
>      HandlePragmaVisibility();
>      return StmtEmpty();
>
>    case tok::annot_pragma_pack:
> +    ProhibitAttributes(Attrs); // TODO: is it correct?

These last two, at least, are correct to prohibit attributes. But they
should
also be clearing Attrs, since we can't build an AttributedStmt for these
annotations.

>      HandlePragmaPack();
>      return StmtEmpty();
>    }
> @@ -306,11 +325,10 @@
>  }
>
>  /// \brief Parse an expression statement.
> -StmtResult Parser::ParseExprStatement(ParsedAttributes &Attrs) {
> +StmtResult Parser::ParseExprStatement() {
>    // If a case keyword is missing, this is where it should be inserted.
>    Token OldToken = Tok;
>
> -  // FIXME: Use the attributes
>    // expression[opt] ';'
>    ExprResult Expr(ParseExpression());
>    if (Expr.isInvalid()) {
> @@ -331,7 +349,7 @@
>        << FixItHint::CreateInsertion(OldToken.getLocation(), "case ");
>
>      // Recover parsing as a case statement.
> -    return ParseCaseStatement(Attrs, /*MissingCase=*/true, Expr);
> +    return ParseCaseStatement(/*MissingCase=*/true, Expr);
>    }
>
>    // Otherwise, eat the semicolon.
> @@ -339,7 +357,7 @@
>    return Actions.ActOnExprStmt(Actions.MakeFullExpr(Expr.get()));
>  }
>
> -StmtResult Parser::ParseSEHTryBlock(ParsedAttributes & Attrs) {
> +StmtResult Parser::ParseSEHTryBlock() {
>    assert(Tok.is(tok::kw___try) && "Expected '__try'");
>    SourceLocation Loc = ConsumeToken();
>    return ParseSEHTryBlockCommon(Loc);
> @@ -358,13 +376,12 @@
>    if(Tok.isNot(tok::l_brace))
>      return StmtError(Diag(Tok,diag::err_expected_lbrace));
>
> -  ParsedAttributesWithRange attrs(AttrFactory);
> -  StmtResult TryBlock(ParseCompoundStatement(attrs));
> +  StmtResult TryBlock(ParseCompoundStatement());
>    if(TryBlock.isInvalid())
>      return move(TryBlock);
>
>    StmtResult Handler;
> -  if (Tok.is(tok::identifier) &&
> +  if (Tok.is(tok::identifier) &&
>        Tok.getIdentifierInfo() == getSEHExceptKeyword()) {
>      SourceLocation Loc = ConsumeToken();
>      Handler = ParseSEHExceptBlock(Loc);
> @@ -418,8 +435,7 @@
>    if(ExpectAndConsume(tok::r_paren,diag::err_expected_rparen))
>      return StmtError();
>
> -  ParsedAttributesWithRange attrs(AttrFactory);
> -  StmtResult Block(ParseCompoundStatement(attrs));
> +  StmtResult Block(ParseCompoundStatement());
>
>    if(Block.isInvalid())
>      return move(Block);
> @@ -437,8 +453,7 @@
>      raii2(Ident___abnormal_termination, false),
>      raii3(Ident_AbnormalTermination, false);
>
> -  ParsedAttributesWithRange attrs(AttrFactory);
> -  StmtResult Block(ParseCompoundStatement(attrs));
> +  StmtResult Block(ParseCompoundStatement());
>    if(Block.isInvalid())
>      return move(Block);
>
> @@ -486,10 +501,8 @@
>  ///         'case' constant-expression ':' statement
>  /// [GNU]   'case' constant-expression '...' constant-expression ':'
statement
>  ///
> -StmtResult Parser::ParseCaseStatement(ParsedAttributes &attrs, bool
MissingCase,
> -                                      ExprResult Expr) {
> +StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr)
{
>    assert((MissingCase || Tok.is(tok::kw_case)) && "Not a case stmt!");
> -  // FIXME: Use attributes?
>
>    // It is very very common for code to contain many case statements
recursively
>    // nested, as in (but usually without indentation):
> @@ -625,9 +638,7 @@
>  ///         'default' ':' statement
>  /// Note that this does not parse the 'statement' at the end.
>  ///
> -StmtResult Parser::ParseDefaultStatement(ParsedAttributes &attrs) {
> -  //FIXME: Use attributes?
> -
> +StmtResult Parser::ParseDefaultStatement() {
>    assert(Tok.is(tok::kw_default) && "Not a default stmt!");
>    SourceLocation DefaultLoc = ConsumeToken();  // eat the 'default'.
>
> @@ -668,9 +679,8 @@
>                                    SubStmt.get(), getCurScope());
>  }
>
> -StmtResult Parser::ParseCompoundStatement(ParsedAttributes &Attr,
> -                                          bool isStmtExpr) {
> -  return ParseCompoundStatement(Attr, isStmtExpr, Scope::DeclScope);
> +StmtResult Parser::ParseCompoundStatement(bool isStmtExpr) {
> +  return ParseCompoundStatement(isStmtExpr, Scope::DeclScope);
>  }
>
>  /// ParseCompoundStatement - Parse a "{}" block.
> @@ -700,11 +710,8 @@
>  /// [OMP]   barrier-directive
>  /// [OMP]   flush-directive
>  ///
> -StmtResult Parser::ParseCompoundStatement(ParsedAttributes &attrs,
> -                                          bool isStmtExpr,
> +StmtResult Parser::ParseCompoundStatement(bool isStmtExpr,
>                                            unsigned ScopeFlags) {
> -  //FIXME: Use attributes?
> -
>    assert(Tok.is(tok::l_brace) && "Not a compount stmt!");
>
>    // Enter a scope to hold everything within the compound stmt.  Compound
> @@ -894,10 +901,7 @@
>  /// [C++]   'if' '(' condition ')' statement
>  /// [C++]   'if' '(' condition ')' statement 'else' statement
>  ///
> -StmtResult Parser::ParseIfStatement(ParsedAttributes &attrs,
> -                                    SourceLocation *TrailingElseLoc) {
> -  // FIXME: Use attributes?
> -
> +StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
>    assert(Tok.is(tok::kw_if) && "Not an if stmt!");
>    SourceLocation IfLoc = ConsumeToken();  // eat the 'if'.
>
> @@ -1028,10 +1032,7 @@
>  ///       switch-statement:
>  ///         'switch' '(' expression ')' statement
>  /// [C++]   'switch' '(' condition ')' statement
> -StmtResult Parser::ParseSwitchStatement(ParsedAttributes &attrs,
> -                                        SourceLocation *TrailingElseLoc)
{
> -  // FIXME: Use attributes?
> -
> +StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc)
{
>    assert(Tok.is(tok::kw_switch) && "Not a switch stmt!");
>    SourceLocation SwitchLoc = ConsumeToken();  // eat the 'switch'.
>
> @@ -1119,10 +1120,7 @@
>  ///       while-statement: [C99 6.8.5.1]
>  ///         'while' '(' expression ')' statement
>  /// [C++]   'while' '(' condition ')' statement
> -StmtResult Parser::ParseWhileStatement(ParsedAttributes &attrs,
> -                                       SourceLocation *TrailingElseLoc) {
> -  // FIXME: Use attributes?
> -
> +StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) {
>    assert(Tok.is(tok::kw_while) && "Not a while stmt!");
>    SourceLocation WhileLoc = Tok.getLocation();
>    ConsumeToken();  // eat the 'while'.
> @@ -1194,9 +1192,7 @@
>  ///       do-statement: [C99 6.8.5.2]
>  ///         'do' statement 'while' '(' expression ')' ';'
>  /// Note: this lets the caller parse the end ';'.
> -StmtResult Parser::ParseDoStatement(ParsedAttributes &attrs) {
> -  // FIXME: Use attributes?
> -
> +StmtResult Parser::ParseDoStatement() {
>    assert(Tok.is(tok::kw_do) && "Not a do stmt!");
>    SourceLocation DoLoc = ConsumeToken();  // eat the 'do'.
>
> @@ -1277,10 +1273,7 @@
>  /// [C++0x] for-range-initializer:
>  /// [C++0x]   expression
>  /// [C++0x]   braced-init-list            [TODO]
> -StmtResult Parser::ParseForStatement(ParsedAttributes &attrs,
> -                                     SourceLocation *TrailingElseLoc) {
> -  // FIXME: Use attributes?
> -
> +StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
>    assert(Tok.is(tok::kw_for) && "Not a for stmt!");
>    SourceLocation ForLoc = ConsumeToken();  // eat the 'for'.
>
> @@ -1535,9 +1528,7 @@
>  ///
>  /// Note: this lets the caller parse the end ';'.
>  ///
> -StmtResult Parser::ParseGotoStatement(ParsedAttributes &attrs) {
> -  // FIXME: Use attributes?
> -
> +StmtResult Parser::ParseGotoStatement() {
>    assert(Tok.is(tok::kw_goto) && "Not a goto stmt!");
>    SourceLocation GotoLoc = ConsumeToken();  // eat the 'goto'.
>
> @@ -1571,9 +1562,7 @@
>  ///
>  /// Note: this lets the caller parse the end ';'.
>  ///
> -StmtResult Parser::ParseContinueStatement(ParsedAttributes &attrs) {
> -  // FIXME: Use attributes?
> -
> +StmtResult Parser::ParseContinueStatement() {
>    SourceLocation ContinueLoc = ConsumeToken();  // eat the 'continue'.
>    return Actions.ActOnContinueStmt(ContinueLoc, getCurScope());
>  }
> @@ -1584,9 +1573,7 @@
>  ///
>  /// Note: this lets the caller parse the end ';'.
>  ///
> -StmtResult Parser::ParseBreakStatement(ParsedAttributes &attrs) {
> -  // FIXME: Use attributes?
> -
> +StmtResult Parser::ParseBreakStatement() {
>    SourceLocation BreakLoc = ConsumeToken();  // eat the 'break'.
>    return Actions.ActOnBreakStmt(BreakLoc, getCurScope());
>  }
> @@ -1594,9 +1581,7 @@
>  /// ParseReturnStatement
>  ///       jump-statement:
>  ///         'return' expression[opt] ';'
> -StmtResult Parser::ParseReturnStatement(ParsedAttributes &attrs) {
> -  // FIXME: Use attributes?
> -
> +StmtResult Parser::ParseReturnStatement() {
>    assert(Tok.is(tok::kw_return) && "Not a return stmt!");
>    SourceLocation ReturnLoc = ConsumeToken();  // eat the 'return'.
>
> @@ -2047,9 +2032,7 @@
>  ///       try-block:
>  ///         'try' compound-statement handler-seq
>  ///
> -StmtResult Parser::ParseCXXTryBlock(ParsedAttributes &attrs) {
> -  // FIXME: Add attributes?
> -
> +StmtResult Parser::ParseCXXTryBlock() {
>    assert(Tok.is(tok::kw_try) && "Expected 'try'");
>
>    SourceLocation TryLoc = ConsumeToken();
> @@ -2075,17 +2058,16 @@
>  StmtResult Parser::ParseCXXTryBlockCommon(SourceLocation TryLoc) {
>    if (Tok.isNot(tok::l_brace))
>      return StmtError(Diag(Tok, diag::err_expected_lbrace));
> -  // FIXME: Possible draft standard bug: attribute-specifier should be
allowed?

Please keep this comment: it's weird that we aren't allowed an
attribute-specifier on the compound-statement in a try block.

> -  ParsedAttributesWithRange attrs(AttrFactory);
> -  StmtResult TryBlock(ParseCompoundStatement(attrs, /*isStmtExpr=*/false,
> +
> +  StmtResult TryBlock(ParseCompoundStatement(/*isStmtExpr=*/false,
>
Scope::DeclScope|Scope::TryScope));
>    if (TryBlock.isInvalid())
>      return move(TryBlock);
>
>    // Borland allows SEH-handlers with 'try'
>
> -  if((Tok.is(tok::identifier) &&
> -      Tok.getIdentifierInfo() == getSEHExceptKeyword()) ||
> +  if((Tok.is(tok::identifier) &&
> +      Tok.getIdentifierInfo() == getSEHExceptKeyword()) ||
>       Tok.is(tok::kw___finally)) {
>      // TODO: Factor into common return ParseSEHHandlerCommon(...)
>      StmtResult Handler;
> @@ -2107,6 +2089,7 @@
>    }
>    else {
>      StmtVector Handlers(Actions);
> +    ParsedAttributesWithRange attrs(AttrFactory);
>      MaybeParseCXX0XAttributes(attrs);
>      ProhibitAttributes(attrs);
>
> @@ -2171,9 +2154,7 @@
>    if (Tok.isNot(tok::l_brace))
>      return StmtError(Diag(Tok, diag::err_expected_lbrace));
>
> -  // FIXME: Possible draft standard bug: attribute-specifier should be
allowed?

Please keep this comment too.

> -  ParsedAttributes attrs(AttrFactory);
> -  StmtResult Block(ParseCompoundStatement(attrs));
> +  StmtResult Block(ParseCompoundStatement());
>    if (Block.isInvalid())
>      return move(Block);
>
> @@ -2192,24 +2173,23 @@
>    if (Result.Behavior == IEB_Dependent) {
>      if (!Tok.is(tok::l_brace)) {
>        Diag(Tok, diag::err_expected_lbrace);
> -      return;
> +      return;
>      }
> -
> -    ParsedAttributes Attrs(AttrFactory);
> -    StmtResult Compound = ParseCompoundStatement(Attrs);
> +
> +    StmtResult Compound = ParseCompoundStatement();
>      if (Compound.isInvalid())
>        return;
> -
> +
>      StmtResult DepResult =
Actions.ActOnMSDependentExistsStmt(Result.KeywordLoc,
>
 Result.IsIfExists,
> -                                                              Result.SS,
> +                                                              Result.SS,
>
 Result.Name,
>
 Compound.get());
>      if (DepResult.isUsable())
>        Stmts.push_back(DepResult.get());
>      return;
>    }
> -
> +
>    BalancedDelimiterTracker Braces(*this, tok::l_brace);
>    if (Braces.consumeOpen()) {
>      Diag(Tok, diag::err_expected_lbrace);
> Index: lib/Serialization/ASTReaderStmt.cpp
> ===================================================================
> --- lib/Serialization/ASTReaderStmt.cpp (revision 154056)
> +++ lib/Serialization/ASTReaderStmt.cpp (working copy)
> @@ -159,9 +159,18 @@
>    S->setIdentLoc(ReadSourceLocation(Record, Idx));
>  }
>
> +void ASTStmtReader::VisitAttributedStmt(AttributedStmt *S) {
> +  VisitStmt(S);
> +  AttrVec Attrs;
> +  Reader.ReadAttributes(F, Attrs, Record, Idx);
> +  S->Attrs = Attrs;
> +  S->SubStmt = Reader.ReadSubStmt();
> +  S->AttrLoc = ReadSourceLocation(Record, Idx);
> +}
> +
>  void ASTStmtReader::VisitIfStmt(IfStmt *S) {
>    VisitStmt(S);
> -  S->setConditionVariable(Reader.getContext(),
> +  S->setConditionVariable(Reader.getContext(),
>                            ReadDeclAs<VarDecl>(Record, Idx));
>    S->setCond(Reader.ReadSubExpr());
>    S->setThen(Reader.ReadSubStmt());
> @@ -1639,6 +1648,10 @@
>        S = new (Context) LabelStmt(Empty);
>        break;
>
> +    case STMT_ATTRIBUTED:
> +      S = new (Context) AttributedStmt(Empty);
> +      break;
> +
>      case STMT_IF:
>        S = new (Context) IfStmt(Empty);
>        break;
> Index: lib/Serialization/ASTWriter.cpp
> ===================================================================
> --- lib/Serialization/ASTWriter.cpp (revision 154056)
> +++ lib/Serialization/ASTWriter.cpp (working copy)
> @@ -651,6 +651,7 @@
>    RECORD(STMT_CASE);
>    RECORD(STMT_DEFAULT);
>    RECORD(STMT_LABEL);
> +  RECORD(STMT_ATTRIBUTED);
>    RECORD(STMT_IF);
>    RECORD(STMT_SWITCH);
>    RECORD(STMT_WHILE);
> Index: lib/Serialization/ASTWriterStmt.cpp
> ===================================================================
> --- lib/Serialization/ASTWriterStmt.cpp (revision 154056)
> +++ lib/Serialization/ASTWriterStmt.cpp (working copy)
> @@ -106,6 +106,14 @@
>    Code = serialization::STMT_LABEL;
>  }
>
> +void ASTStmtWriter::VisitAttributedStmt(AttributedStmt *S) {
> +  VisitStmt(S);
> +  Writer.WriteAttributes(S->getAttrs(), Record);
> +  Writer.AddStmt(S->getSubStmt());
> +  Writer.AddSourceLocation(S->getAttrLoc(), Record);
> +  Code = serialization::STMT_ATTRIBUTED;
> +}
> +
>  void ASTStmtWriter::VisitIfStmt(IfStmt *S) {
>    VisitStmt(S);
>    Writer.AddDeclRef(S->getConditionVariable(), Record);
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120405/03eff51d/attachment.html>


More information about the cfe-commits mailing list