[PATCH] Add 'return_typestate' function attribute

[Christian Wailes]

On Mon, Sep 2, 2013 at 3:54 PM, Aaron Ballman <aaron.ballman at gmail.com>wrote:

> On Mon, Sep 2, 2013 at 6:45 PM, Christian Wailes <chriswailes at google.com>
> wrote:
> > Notes below.  Updated patch for your refactoring of AttributeList coming
> > soon.
> >
> > - Chris
> >
> >
> > On Sun, Sep 1, 2013 at 7:50 AM, Aaron Ballman <aaron.ballman at gmail.com>
> > wrote:
> >>
> >> No major complaints, but some comments are interspersed below.
> >>
> >> > Index: include/clang/Analysis/Analyses/Consumed.h
> >> > ===================================================================
> >> > --- include/clang/Analysis/Analyses/Consumed.h
> >> > +++ include/clang/Analysis/Analyses/Consumed.h
> >> > @@ -39,11 +39,27 @@
> >> >
> >> >      /// \brief Emit the warnings and notes left by the analysis.
> >> >      virtual void emitDiagnostics() {}
> >> > -
> >> > +
> >> > +    // FIXME: This can be removed when the attr propagation fix for
> >> > templated
> >> > +    //        classes lands.
> >> > +    /// \brief Warn about return typestates set for unconsumable
> types.
> >> > +    ///
> >> > +    /// \param Loc -- The location of the attributes.
> >> > +    ///
> >> > +    /// \param TypeName -- The name of the unconsumable type.
> >> > +    virtual void
> warnReturnTypestateForUnconsumableType(SourceLocation
> >> > Loc,
> >> > +                                                        StringRef
> >> > TypeName) {}
> >> > +
> >> > +    /// \brief Warn about return typestate mismatches.
> >> > +    /// \param Loc -- The SourceLocation of the return statement.
> >> > +    virtual void warnReturnTypestateMismatch(SourceLocation Loc) {}
> >> > +
> >> >      /// \brief Warn about unnecessary-test errors.
> >> >      /// \param VariableName -- The name of the variable that holds
> the
> >> > unique
> >> >      /// value.
> >> >      ///
> >> > +    /// \param VariableState -- The known state of the value.
> >> > +    ///
> >> >      /// \param Loc -- The SourceLocation of the unnecessary test.
> >> >      virtual void warnUnnecessaryTest(StringRef VariableName,
> >> >                                       StringRef VariableState,
> >> > @@ -170,6 +186,8 @@
> >> >      ConsumedBlockInfo BlockInfo;
> >> >      ConsumedStateMap *CurrStates;
> >> >
> >> > +    ConsumedState ExpectedReturnState;
> >> > +
> >> >      bool hasConsumableAttributes(const CXXRecordDecl *RD);
> >> >      bool splitState(const CFGBlock *CurrBlock,
> >> >                      const ConsumedStmtVisitor &Visitor);
> >> > @@ -181,6 +199,8 @@
> >> >      ConsumedAnalyzer(ConsumedWarningsHandlerBase &WarningsHandler)
> >> >          : WarningsHandler(WarningsHandler) {}
> >> >
> >> > +    ConsumedState getExpectedReturnState() { return
> >> > ExpectedReturnState; }
> >>
> >> Method can be const
> >>
> >> > +
> >> >      /// \brief Check a function's CFG for consumed violations.
> >> >      ///
> >> >      /// We traverse the blocks in the CFG, keeping track of the state
> >> > of each
> >> > Index: include/clang/Basic/Attr.td
> >> > ===================================================================
> >> > --- include/clang/Basic/Attr.td
> >> > +++ include/clang/Basic/Attr.td
> >> > @@ -953,6 +953,14 @@
> >> >    let Subjects = [CXXMethod];
> >> >  }
> >> >
> >> > +def ReturnTypestate : InheritableAttr {
> >> > +  let Spellings = [GNU<"return_typestate">];
> >> > +  let Subjects = [Function];
> >> > +  let Args = [EnumArgument<"State", "ConsumedState",
> >> > +                           ["unknown", "consumed", "unconsumed"],
> >> > +                           ["Unknown", "Consumed", "Unconsumed"]>];
> >> > +}
> >> > +
> >> >  // Type safety attributes for `void *' pointers and type tags.
> >> >
> >> >  def ArgumentWithTypeTag : InheritableAttr {
> >> > Index: include/clang/Basic/DiagnosticSemaKinds.td
> >> > ===================================================================
> >> > --- include/clang/Basic/DiagnosticSemaKinds.td
> >> > +++ include/clang/Basic/DiagnosticSemaKinds.td
> >> > @@ -2190,8 +2190,16 @@
> >> >    "invocation of method '%0' on a temporary object while it is in
> the "
> >> >    "'consumed' state">, InGroup<Consumed>, DefaultIgnore;
> >> >  def warn_attr_on_unconsumable_class : Warning<
> >> > -  "consumed analysis attribute is attached to class '%0' which isn't
> >> > marked "
> >> > -  "as consumable">, InGroup<Consumed>, DefaultIgnore;
> >> > +  "consumed analysis attribute is attached to member of class '%0'
> >> > which isn't "
> >> > +  "marked as consumable">, InGroup<Consumed>, DefaultIgnore;
> >> > +def warn_return_typestate_for_unconsumable_type : Warning<
> >> > +  "return state set for an unconsumable type '%0'">,
> InGroup<Consumed>,
> >> > +  DefaultIgnore;
> >> > +def warn_unknown_consumed_state : Warning<
> >> > +  "unknown consumed analysis state">, InGroup<Consumed>,
> DefaultIgnore;
> >>
> >> It would be good to list the state.
> >>
> >> > +def warn_return_typestate_mismatch : Warning<
> >> > +  "return value not in expected state">, InGroup<Consumed>,
> >> > +  DefaultIgnore;
> >>
> >> It might make for a better diagnostics to tell the user what state the
> >> return value is in, and what the expected state was.
> >>
> >
> > I was planning on adding that information in a note as I couldn't think
> of a
> > nice, concise way of expressing it in the warning.  This will also allow
> me
> > to point to where the return typestate is declared.
>
> It could even be as simple as: inconsistent return value state;
> expected %0, got %1
>
> Though I'm sure someone better with words can improve upon that.  ;-)
>

Done.


>
> >>
> >> >
> >> >  // ConsumedStrict warnings
> >> >  def warn_use_in_unknown_state : Warning<
> >> > Index: lib/Analysis/Consumed.cpp
> >> > ===================================================================
> >> > --- lib/Analysis/Consumed.cpp
> >> > +++ lib/Analysis/Consumed.cpp
> >> > @@ -31,6 +31,7 @@
> >> >  #include "llvm/Support/Compiler.h"
> >> >  #include "llvm/Support/raw_ostream.h"
> >> >
> >> > +// TODO: Add notes about the actual and expected state for
> >> >  // TODO: Correctly identify unreachable blocks when chaining boolean
> >> > operators.
> >> >  // TODO: Warn about unreachable code.
> >> >  // TODO: Switch to using a bitmap to track unreachable blocks.
> >> > @@ -256,6 +257,8 @@
> >> >    void forwardInfo(const Stmt *From, const Stmt *To);
> >> >    void handleTestingFunctionCall(const CallExpr *Call, const VarDecl
> >> > *Var);
> >> >    bool isLikeMoveAssignment(const CXXMethodDecl *MethodDecl);
> >> > +  void propagateReturnType(const Stmt *Call, const FunctionDecl *Fun,
> >> > +                           QualType ReturnType);
> >> >
> >> >  public:
> >> >
> >> > @@ -272,6 +275,7 @@
> >> >    void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr
> >> > *Temp);
> >> >    void VisitMemberExpr(const MemberExpr *MExpr);
> >> >    void VisitParmVarDecl(const ParmVarDecl *Param);
> >> > +  void VisitReturnStmt(const ReturnStmt *Ret);
> >> >    void VisitUnaryOperator(const UnaryOperator *UOp);
> >> >    void VisitVarDecl(const VarDecl *Var);
> >> >
> >> > @@ -373,6 +377,34 @@
> >> >
> >> > MethodDecl->getParamDecl(0)->getType()->isRValueReferenceType());
> >> >  }
> >> >
> >> > +void ConsumedStmtVisitor::propagateReturnType(const Stmt *Call,
> >> > +                                              const FunctionDecl
> *Fun,
> >> > +                                              QualType ReturnType) {
> >> > +  if (isConsumableType(ReturnType)) {
> >> > +    if (Fun->hasAttr<ReturnTypestateAttr>()) {
> >> > +      switch (Fun->getAttr<ReturnTypestateAttr>()->getState()) {
> >> > +      case ReturnTypestateAttr::Unconsumed:
> >> > +        PropagationMap.insert(PairType(Call,
> >> > +
> >> > PropagationInfo(consumed::CS_Unconsumed)));
> >> > +        break;
> >> > +
> >> > +      case ReturnTypestateAttr::Consumed:
> >> > +        PropagationMap.insert(PairType(Call,
> >> > +
>  PropagationInfo(consumed::CS_Consumed)));
> >> > +        break;
> >> > +
> >> > +      case ReturnTypestateAttr::Unknown:
> >> > +        PropagationMap.insert(PairType(Call,
> >> > +
>  PropagationInfo(consumed::CS_Unknown)));
> >> > +        break;
> >> > +      }
> >> > +    } else {
> >> > +      PropagationMap.insert(PairType(Call,
> >> > +                            PropagationInfo(consumed::CS_Unknown)));
> >> > +    }
> >> > +  }
> >> > +}
> >>
> >> There's a fair amount of duplication there.  The only variability is
> >> what the PropagationInfo enumerant is, so I think it'd be more clear
> >> to have a single call to insert instead of multiple.
> >>
> >
> > Already done in a future patch.  Backported into the updated patch.
> >
> >>
> >> > +
> >> >  void ConsumedStmtVisitor::Visit(const Stmt *StmtNode) {
> >> >
> >> >    ConstStmtVisitor<ConsumedStmtVisitor>::Visit(StmtNode);
> >> > @@ -469,6 +501,8 @@
> >> >          StateMap->setState(PInfo.getVar(), consumed::CS_Unknown);
> >> >        }
> >> >      }
> >> > +
> >> > +    propagateReturnType(Call, FunDecl, FunDecl->getCallResultType());
> >> >    }
> >> >  }
> >> >
> >> > @@ -483,8 +517,7 @@
> >> >    QualType ThisType =
> >> > Constructor->getThisType(CurrContext)->getPointeeType();
> >> >
> >> >    if (isConsumableType(ThisType)) {
> >> > -    if (Constructor->hasAttr<ConsumesAttr>() ||
> >> > -        Constructor->isDefaultConstructor()) {
> >> > +    if (Constructor->isDefaultConstructor()) {
> >> >
> >> >        PropagationMap.insert(PairType(Call,
> >> >          PropagationInfo(consumed::CS_Consumed)));
> >> > @@ -513,8 +546,7 @@
> >> >          PropagationMap.insert(PairType(Call, Entry->second));
> >> >
> >> >      } else {
> >> > -      PropagationMap.insert(PairType(Call,
> >> > -        PropagationInfo(consumed::CS_Unconsumed)));
> >> > +      propagateReturnType(Call, Constructor, ThisType);
> >> >      }
> >> >    }
> >> >  }
> >> > @@ -677,6 +709,23 @@
> >> >      StateMap->setState(Param, consumed::CS_Unknown);
> >> >  }
> >> >
> >> > +void ConsumedStmtVisitor::VisitReturnStmt(const ReturnStmt *Ret) {
> >> > +  if (Analyzer.getExpectedReturnState()) {
> >> > +    InfoEntry Entry = PropagationMap.find(Ret->getRetValue());
> >> > +
> >> > +    if (Entry != PropagationMap.end()) {
> >> > +      assert(Entry->second.isState() || Entry->second.isVar());
> >> > +
> >> > +      ConsumedState RetState = Entry->second.isState() ?
> >> > +        Entry->second.getState() :
> >> > StateMap->getState(Entry->second.getVar());
> >> > +
> >> > +      if (RetState != Analyzer.getExpectedReturnState())
> >> > +        Analyzer.WarningsHandler.warnReturnTypestateMismatch(
> >> > +          Ret->getReturnLoc());
> >> > +    }
> >> > +  }
> >> > +}
> >> > +
> >> >  void ConsumedStmtVisitor::VisitUnaryOperator(const UnaryOperator
> *UOp)
> >> > {
> >> >    InfoEntry Entry =
> >> > PropagationMap.find(UOp->getSubExpr()->IgnoreParens());
> >> >    if (Entry == PropagationMap.end()) return;
> >> > @@ -997,6 +1046,53 @@
> >> >
> >> >    if (!D) return;
> >> >
> >> > +  // FIXME: This should be removed when template instantiation
> >> > propagates
> >> > +  //        attributes at template specialization definition, not
> >> > declaration.
> >> > +  //        When it is removed the test needs to be enabled in
> >> > SemaDeclAttr.cpp.
> >> > +  QualType ReturnType;
> >> > +  if (const CXXConstructorDecl *Constructor =
> >> > dyn_cast<CXXConstructorDecl>(D)) {
> >> > +    ASTContext &CurrContext = AC.getASTContext();
> >> > +    ReturnType =
> >> > Constructor->getThisType(CurrContext)->getPointeeType();
> >> > +
> >> > +  } else {
> >> > +    ReturnType = cast<FunctionDecl>(D)->getCallResultType();
> >> > +  }
> >> > +
> >> > +  // Determine the expected return value.
> >> > +  if (D->hasAttr<ReturnTypestateAttr>()) {
> >> > +
> >> > +    ReturnTypestateAttr *RTSAttr = D->getAttr<ReturnTypestateAttr>();
> >> > +
> >> > +    const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
> >> > +    if (!RD || !RD->hasAttr<ConsumableAttr>()) {
> >> > +        // FIXME: This branch can be removed with the code above.
> >> > +        WarningsHandler.warnReturnTypestateForUnconsumableType(
> >> > +          RTSAttr->getLocation(), ReturnType.getAsString());
> >> > +        ExpectedReturnState = CS_None;
> >> > +
> >> > +    } else {
> >> > +      switch (RTSAttr->getState()) {
> >> > +      case ReturnTypestateAttr::Unknown:
> >> > +        ExpectedReturnState = CS_Unknown;
> >> > +        break;
> >> > +
> >> > +      case ReturnTypestateAttr::Unconsumed:
> >> > +        ExpectedReturnState = CS_Unconsumed;
> >> > +        break;
> >> > +
> >> > +      case ReturnTypestateAttr::Consumed:
> >> > +        ExpectedReturnState = CS_Consumed;
> >> > +        break;
> >> > +      }
> >> > +    }
> >> > +
> >> > +  } else if (isConsumableType(ReturnType)) {
> >> > +    ExpectedReturnState = CS_Unknown;
> >> > +
> >> > +  } else {
> >> > +    ExpectedReturnState = CS_None;
> >> > +  }
> >> > +
> >> >    BlockInfo = ConsumedBlockInfo(AC.getCFG());
> >> >
> >> >    PostOrderCFGView *SortedGraph = AC.getAnalysis<PostOrderCFGView>();
> >> > Index: lib/Sema/AnalysisBasedWarnings.cpp
> >> > ===================================================================
> >> > --- lib/Sema/AnalysisBasedWarnings.cpp
> >> > +++ lib/Sema/AnalysisBasedWarnings.cpp
> >> > @@ -1445,11 +1445,21 @@
> >> >      }
> >> >    }
> >> >
> >> > -  /// Warn about unnecessary-test errors.
> >> > -  /// \param VariableName -- The name of the variable that holds the
> >> > unique
> >> > -  /// value.
> >> > -  ///
> >> > -  /// \param Loc -- The SourceLocation of the unnecessary test.
> >> > +  void warnReturnTypestateForUnconsumableType(SourceLocation Loc,
> >> > +                                          StringRef TypeName) {
> >> > +    PartialDiagnosticAt Warning(Loc, S.PDiag(
> >> > +      diag::warn_return_typestate_for_unconsumable_type) <<
> TypeName);
> >>
> >> You are missing an enabled test case for this warning.
> >>
> >
> > The test for this is on line 42 of warn-consumed-parsing.cpp.  It is
> > currently commented out because an issue with attributes being
> incorrectly
> > propagated for template specializations is preventing us from checking
> for
> > this problem when parsing the attributes.
>
> Yes, that's why I mentioned an enabled test.  But it seems like you
> could write a test that doesn't use template specialization?
>
>
Can you elaborate on what you mean by an 'enabled test'?  And while we
could test for non-template specialized cases in SemaDeclAttr.cpp now, I
was told to comment out that code and check for the warning in the
ConsumedAnalyzer by DeLesely.


> >  There are some FIXMEs running
> > around telling someone to un-comment the test and move some code around
> once
> > the issue is resolved.  Specifically, this warning function will be
> removed
> > and the warning will be issued from SemaDeclAttr directly.
> >
> >>
> >> > +
> >> > +    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
> >> > +  }
> >> > +
> >> > +  void warnReturnTypestateMismatch(SourceLocation Loc) {
> >> > +    PartialDiagnosticAt Warning(Loc, S.PDiag(
> >> > +      diag::warn_return_typestate_mismatch));
> >> > +
> >> > +    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
> >> > +  }
> >> > +
> >> >    void warnUnnecessaryTest(StringRef VariableName, StringRef
> >> > VariableState,
> >> >                             SourceLocation Loc) {
> >> >
> >> > @@ -1459,11 +1469,6 @@
> >> >      Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
> >> >    }
> >> >
> >> > -  /// Warn about use-while-consumed errors.
> >> > -  /// \param MethodName -- The name of the method that was
> incorrectly
> >> > -  /// invoked.
> >> > -  ///
> >> > -  /// \param Loc -- The SourceLocation of the method invocation.
> >> >    void warnUseOfTempWhileConsumed(StringRef MethodName,
> SourceLocation
> >> > Loc) {
> >> >
> >> >      PartialDiagnosticAt Warning(Loc, S.PDiag(
> >> > @@ -1472,11 +1477,6 @@
> >> >      Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
> >> >    }
> >> >
> >> > -  /// Warn about use-in-unknown-state errors.
> >> > -  /// \param MethodName -- The name of the method that was
> incorrectly
> >> > -  /// invoked.
> >> > -  ///
> >> > -  /// \param Loc -- The SourceLocation of the method invocation.
> >> >    void warnUseOfTempInUnknownState(StringRef MethodName,
> SourceLocation
> >> > Loc) {
> >> >
> >> >      PartialDiagnosticAt Warning(Loc, S.PDiag(
> >> > @@ -1485,14 +1485,6 @@
> >> >      Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
> >> >    }
> >> >
> >> > -  /// Warn about use-while-consumed errors.
> >> > -  /// \param MethodName -- The name of the method that was
> incorrectly
> >> > -  /// invoked.
> >> > -  ///
> >> > -  /// \param VariableName -- The name of the variable that holds the
> >> > unique
> >> > -  /// value.
> >> > -  ///
> >> > -  /// \param Loc -- The SourceLocation of the method invocation.
> >> >    void warnUseWhileConsumed(StringRef MethodName, StringRef
> >> > VariableName,
> >> >                              SourceLocation Loc) {
> >> >
> >> > @@ -1502,14 +1494,6 @@
> >> >      Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
> >> >    }
> >> >
> >> > -  /// Warn about use-in-unknown-state errors.
> >> > -  /// \param MethodName -- The name of the method that was
> incorrectly
> >> > -  /// invoked.
> >> > -  ///
> >> > -  /// \param VariableName -- The name of the variable that holds the
> >> > unique
> >> > -  /// value.
> >> > -  ///
> >> > -  /// \param Loc -- The SourceLocation of the method invocation.
> >> >    void warnUseInUnknownState(StringRef MethodName, StringRef
> >> > VariableName,
> >> >                               SourceLocation Loc) {
> >>
> >> Why are all of the comments being removed?
> >>
> >
> > These comments are redundent, as they are also present on the virtual
> > versions of these functions.  Rather than risk them becoming
> unsynchronized
> > I'm removing the copies here and keeping the ones on the interface.
>
> Sounds good.
>
> >
> >>
> >> >
> >> > Index: lib/Sema/SemaDeclAttr.cpp
> >> > ===================================================================
> >> > --- lib/Sema/SemaDeclAttr.cpp
> >> > +++ lib/Sema/SemaDeclAttr.cpp
> >> > @@ -1071,6 +1071,62 @@
> >> >
> >> > Attr.getAttributeSpellingListIndex()));
> >> >  }
> >> >
> >> > +static void handleReturnTypestateAttr(Sema &S, Decl *D,
> >> > +                                      const AttributeList &Attr) {
> >> > +
> >> > +  ReturnTypestateAttr::ConsumedState ReturnState;
> >> > +
> >> > +  if (Attr.getParameterName()) {
> >> > +    StringRef Param = Attr.getParameterName()->getName();
> >>
> >> You'll have to rebase this since r189711 got rid of the notion of a
> >> distinct parameter for attributes.  It shouldn't be too bad -- the
> >> parameter is now the first argument in the list of args.
> >>
> >
> > Already saw that and am working on it now.  This will also come in handy
> for
> > a future patch I'm about to submit.
> >
> >>
> >> > +
> >> > +    if (Param == "unknown") {
> >> > +      ReturnState = ReturnTypestateAttr::Unknown;
> >> > +    } else if (Param == "consumed") {
> >> > +      ReturnState = ReturnTypestateAttr::Consumed;
> >> > +    } else if (Param == "unconsumed") {
> >> > +      ReturnState = ReturnTypestateAttr::Unconsumed;
> >> > +    } else {
> >> > +      S.Diag(Attr.getLoc(), diag::warn_unknown_consumed_state);
> >>
> >> You are missing test cases for this.
> >>
> >> > +      return;
> >> > +    }
> >> > +
> >> > +  } else {
> >> > +    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments)
> >> > +      << Attr.getName() << 1;
> >>
> >> You should use checkAttributeNumArgs instead.
> >
> >
> > That wouldn't have worked with the previous version of AttributeList as
> if
> > an attribute had a getNumArgs would still return 0.  With your patch I
> can
> > now use checkAttributeNumArgs.
> >
> >>
> >>
> >> > +  }
> >> > +
> >> > +  if (!isa<FunctionDecl>(D)) {
> >> > +    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
> >> > +      Attr.getName() << ExpectedFunction;
> >> > +    return;
> >> > +  }
> >> > +
> >> > +  // FIXME: This check is currently being done in the analysis.  It
> can
> >> > be
> >> > +  //        enabled here only after the parser propagates attributes
> at
> >> > +  //        template specialization definition, not declaration.
> >> > +  //~ QualType ReturnType;
> >> > +  //~
> >> > +  //~ if (const CXXConstructorDecl *Constructor =
> >> > dyn_cast<CXXConstructorDecl>(D)) {
> >> > +    //~ ReturnType =
> >> > Constructor->getThisType(S.getASTContext())->getPointeeType();
> >> > +    //~
> >> > +  //~ } else {
> >> > +    //~
> >> > +    //~ ReturnType = cast<FunctionDecl>(D)->getCallResultType();
> >> > +  //~ }
> >> > +  //~
> >> > +  //~ const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
> >> > +  //~
> >> > +  //~ if (!RD || !RD->hasAttr<ConsumableAttr>()) {
> >> > +      //~ S.Diag(Attr.getLoc(),
> >> > diag::warn_return_state_for_unconsumable_type) <<
> >> > +        //~ ReturnType.getAsString();
> >> > +      //~ return;
> >> > +  //~ }
> >>
> >> What is with the ~ at the start of the comments?
> >>
> >
> > These are from my IDE so it knows which comments it should remove via the
> > keyboard shortcut.  I can remove them, or comment out this block with /*
> */
> > comments.
>
> Thanks!
>
> >
> >>
> >> > +
> >> > +  D->addAttr(::new (S.Context)
> >> > +             ReturnTypestateAttr(Attr.getRange(), S.Context,
> >> > ReturnState,
> >> > +
> >> > Attr.getAttributeSpellingListIndex()));
> >> > +}
> >> > +
> >> >  static void handleExtVectorTypeAttr(Sema &S, Scope *scope, Decl *D,
> >> >                                      const AttributeList &Attr) {
> >> >    TypedefNameDecl *TD = dyn_cast<TypedefNameDecl>(D);
> >> > @@ -5052,6 +5108,9 @@
> >> >    case AttributeList::AT_TestsUnconsumed:
> >> >      handleTestsUnconsumedAttr(S, D, Attr);
> >> >      break;
> >> > +  case AttributeList::AT_ReturnTypestate:
> >> > +    handleReturnTypestateAttr(S, D, Attr);
> >> > +    break;
> >> >
> >> >    // Type safety attributes.
> >> >    case AttributeList::AT_ArgumentWithTypeTag:
> >> > Index: test/SemaCXX/warn-consumed-analysis-strict.cpp
> >> > ===================================================================
> >> > --- test/SemaCXX/warn-consumed-analysis-strict.cpp
> >> > +++ test/SemaCXX/warn-consumed-analysis-strict.cpp
> >> > @@ -3,6 +3,7 @@
> >> >  #define CALLABLE_WHEN_UNCONSUMED __attribute__
> >> > ((callable_when_unconsumed))
> >> >  #define CONSUMABLE               __attribute__ ((consumable))
> >> >  #define CONSUMES                 __attribute__ ((consumes))
> >> > +#define RETURN_TYPESTATE(State)  __attribute__
> >> > ((return_typestate(State)))
> >> >  #define TESTS_UNCONSUMED         __attribute__ ((tests_unconsumed))
> >> >
> >> >  #define TEST_VAR(Var) Var.isValid()
> >> > @@ -15,9 +16,10 @@
> >> >
> >> >    public:
> >> >    ConsumableClass();
> >> > -  ConsumableClass(T val);
> >> > -  ConsumableClass(ConsumableClass<T> &other);
> >> > -  ConsumableClass(ConsumableClass<T> &&other);
> >> > +  ConsumableClass(nullptr_t p) RETURN_TYPESTATE(consumed);
> >> > +  ConsumableClass(T val) RETURN_TYPESTATE(unconsumed);
> >> > +  ConsumableClass(ConsumableClass<T> &other)
> >> > RETURN_TYPESTATE(unconsumed);
> >> > +  ConsumableClass(ConsumableClass<T> &&other)
> >> > RETURN_TYPESTATE(unconsumed);
> >> >
> >> >    ConsumableClass<T>& operator=(ConsumableClass<T>  &other);
> >> >    ConsumableClass<T>& operator=(ConsumableClass<T> &&other);
> >> > Index: test/SemaCXX/warn-consumed-analysis.cpp
> >> > ===================================================================
> >> > --- test/SemaCXX/warn-consumed-analysis.cpp
> >> > +++ test/SemaCXX/warn-consumed-analysis.cpp
> >> > @@ -3,6 +3,7 @@
> >> >  #define CALLABLE_WHEN_UNCONSUMED __attribute__
> >> > ((callable_when_unconsumed))
> >> >  #define CONSUMABLE               __attribute__ ((consumable))
> >> >  #define CONSUMES                 __attribute__ ((consumes))
> >> > +#define RETURN_TYPESTATE(State)  __attribute__
> >> > ((return_typestate(State)))
> >> >  #define TESTS_UNCONSUMED         __attribute__ ((tests_unconsumed))
> >> >
> >> >  typedef decltype(nullptr) nullptr_t;
> >> > @@ -13,10 +14,10 @@
> >> >
> >> >    public:
> >> >    ConsumableClass();
> >> > -  ConsumableClass(nullptr_t p) CONSUMES;
> >> > -  ConsumableClass(T val);
> >> > -  ConsumableClass(ConsumableClass<T> &other);
> >> > -  ConsumableClass(ConsumableClass<T> &&other);
> >> > +  ConsumableClass(nullptr_t p) RETURN_TYPESTATE(consumed);
> >> > +  ConsumableClass(T val) RETURN_TYPESTATE(unconsumed);
> >> > +  ConsumableClass(ConsumableClass<T> &other)
> >> > RETURN_TYPESTATE(unconsumed);
> >> > +  ConsumableClass(ConsumableClass<T> &&other)
> >> > RETURN_TYPESTATE(unconsumed);
> >> >
> >> >    ConsumableClass<T>& operator=(ConsumableClass<T>  &other);
> >> >    ConsumableClass<T>& operator=(ConsumableClass<T> &&other);
> >> > @@ -48,6 +49,16 @@
> >> >
> >> >  void baf3(ConsumableClass<int> &&var);
> >> >
> >> > +ConsumableClass<int> returnsUnconsumed()
> RETURN_TYPESTATE(unconsumed);
> >> > +ConsumableClass<int> returnsUnconsumed() {
> >> > +  return ConsumableClass<int>(); // expected-warning {{return value
> not
> >> > in expected state}}
> >> > +}
> >> > +
> >> > +ConsumableClass<int> returnsConsumed() RETURN_TYPESTATE(consumed);
> >> > +ConsumableClass<int> returnsConsumed() {
> >> > +  return ConsumableClass<int>();
> >> > +}
> >> > +
> >> >  void testInitialization() {
> >> >    ConsumableClass<int> var0;
> >> >    ConsumableClass<int> var1 = ConsumableClass<int>();
> >> > @@ -253,6 +264,16 @@
> >> >    *var; // expected-warning {{invocation of method 'operator*' on
> >> > object 'var' while it is in the 'consumed' state}}
> >> >  }
> >> >
> >> > +void testReturnStates() {
> >> > +  ConsumableClass<int> var;
> >> > +
> >> > +  var = returnsUnconsumed();
> >> > +  *var;
> >> > +
> >> > +  var = returnsConsumed();
> >> > +  *var; // expected-warning {{invocation of method 'operator*' on
> >> > object 'var' while it is in the 'consumed' state}}
> >> > +}
> >> > +
> >> >  void testMoveAsignmentish() {
> >> >    ConsumableClass<int>  var0;
> >> >    ConsumableClass<long> var1(42);
> >> > Index: test/SemaCXX/warn-consumed-parsing.cpp
> >> > ===================================================================
> >> > --- test/SemaCXX/warn-consumed-parsing.cpp
> >> > +++ test/SemaCXX/warn-consumed-parsing.cpp
> >> > @@ -1,9 +1,10 @@
> >> >  // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s
> >> >
> >> > -#define CONSUMABLE                __attribute__ ((consumable))
> >> > -#define CONSUMES                  __attribute__ ((consumes))
> >> > -#define TESTS_UNCONSUMED          __attribute__ ((tests_unconsumed))
> >> > -#define CALLABLE_WHEN_UNCONSUMED  __attribute__
> >> > ((callable_when_unconsumed))
> >> > +#define CALLABLE_WHEN_UNCONSUMED __attribute__
> >> > ((callable_when_unconsumed))
> >> > +#define CONSUMABLE               __attribute__ ((consumable))
> >> > +#define CONSUMES                 __attribute__ ((consumes))
> >> > +#define RETURN_TYPESTATE(State)  __attribute__
> >> > ((return_typestate(State)))
> >> > +#define TESTS_UNCONSUMED         __attribute__ ((tests_unconsumed))
> >> >
> >> >  class AttrTester0 {
> >> >    void Consumes()        __attribute__ ((consumes(42))); //
> >> > expected-error {{attribute takes no arguments}}
> >> > @@ -16,6 +17,7 @@
> >> >  int var1 TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed'
> >> > attribute only applies to methods}}
> >> >  int var2 CALLABLE_WHEN_UNCONSUMED; // expected-warning
> >> > {{'callable_when_unconsumed' attribute only applies to methods}}
> >> >  int var3 CONSUMABLE; // expected-warning {{'consumable' attribute
> only
> >> > applies to classes}}
> >> > +int var4 RETURN_TYPESTATE(consumed); // expected-warning
> >> > {{'return_typestate' attribute only applies to functions}}
> >> >
> >> >  void function0() CONSUMES; // expected-warning {{'consumes' attribute
> >> > only applies to methods}}
> >> >  void function1() TESTS_UNCONSUMED; // expected-warning
> >> > {{'tests_unconsumed' attribute only applies to methods}}
> >> > @@ -29,7 +31,13 @@
> >> >  };
> >> >
> >> >  class AttrTester2 {
> >> > -  void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED; //
> >> > expected-warning {{consumed analysis attribute is attached to class
> >> > 'AttrTester2' which isn't marked as consumable}}
> >> > -  void consumes()               CONSUMES; // expected-warning
> >> > {{consumed analysis attribute is attached to class 'AttrTester2'
> which isn't
> >> > marked as consumable}}
> >> > -  bool testsUnconsumed()        TESTS_UNCONSUMED; // expected-warning
> >> > {{consumed analysis attribute is attached to class 'AttrTester2'
> which isn't
> >> > marked as consumable}}
> >> > +  void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED; //
> >> > expected-warning {{consumed analysis attribute is attached to member
> of
> >> > class 'AttrTester2' which isn't marked as consumable}}
> >> > +  void consumes()               CONSUMES; // expected-warning
> >> > {{consumed analysis attribute is attached to member of class
> 'AttrTester2'
> >> > which isn't marked as consumable}}
> >> > +  bool testsUnconsumed()        TESTS_UNCONSUMED; // expected-warning
> >> > {{consumed analysis attribute is attached to member of class
> 'AttrTester2'
> >> > which isn't marked as consumable}}
> >> >  };
> >> > +
> >> > +// FIXME: This can be re-added once the attribute propagation for
> >> > template
> >> > +//        specializations is fixed.
> >> > +#if 0
> >> > +int returnStateFunction() RETURN_TYPESTATE(consumed); //
> >> > expected-warning {{return state set for an unconsumable type 'int'}}
> >> > +#endif
> >> >
> >>
> >> ~Aaron
> >
> >
>
> ~Aaron
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130902/c517e759/attachment.html>