<div dir="ltr"><div><div>With regards to the string literals and identifiers: I agree that the parser should be able to handle lists of unresolved identifiers and would be willing to work on the feature.  Currently, I don't know how to change the parser to get that working, and this is my last day at Google.  Plus, making that change should probably be a separate patch.<br>
<br></div>- Chris<br><br></div>P.S. Richard was removed from this email chain because his Microsoft Exchange server keeps bouncing my emails.<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Sep 6, 2013 at 11:08 AM, Christian Wailes <span dir="ltr"><<a href="mailto:chriswailes@google.com" target="_blank">chriswailes@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Fri, Sep 6, 2013 at 7:34 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron.ballman@gmail.com" target="_blank">aaron.ballman@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> Index: include/clang/Analysis/Analyses/Consumed.h<br>
> ===================================================================<br>
> --- include/clang/Analysis/Analyses/Consumed.h<br>
> +++ include/clang/Analysis/Analyses/Consumed.h<br>
> @@ -25,6 +25,15 @@<br>
>  namespace clang {<br>
>  namespace consumed {<br>
><br>
> +  enum ConsumedState {<br>
> +    // No state information for the given variable.<br>
> +    CS_None,<br>
> +<br>
> +    CS_Unknown,<br>
> +    CS_Unconsumed,<br>
> +    CS_Consumed<br>
> +  };<br>
> +<br>
>    class ConsumedStmtVisitor;<br>
><br>
>    typedef SmallVector<PartialDiagnosticAt, 1> OptionalNotes;<br>
> @@ -71,52 +80,29 @@<br>
>      /// \param MethodName -- The name of the method that was incorrectly<br>
>      /// invoked.<br>
>      ///<br>
> -    /// \param Loc -- The SourceLocation of the method invocation.<br>
> -    virtual void warnUseOfTempWhileConsumed(StringRef MethodName,<br>
> -                                            SourceLocation Loc) {}<br>
> -<br>
> -    /// \brief Warn about use-in-unknown-state errors.<br>
> -    /// \param MethodName -- The name of the method that was incorrectly<br>
> -    /// invoked.<br>
> +    /// \param State -- The state the object was used in.<br>
>      ///<br>
>      /// \param Loc -- The SourceLocation of the method invocation.<br>
> -    virtual void warnUseOfTempInUnknownState(StringRef MethodName,<br>
> +    virtual void warnUseOfTempInInvalidState(StringRef MethodName,<br>
> +                                             StringRef State,<br>
>                                               SourceLocation Loc) {}<br>
><br>
>      /// \brief Warn about use-while-consumed errors.<br>
>      /// \param MethodName -- The name of the method that was incorrectly<br>
>      /// invoked.<br>
>      ///<br>
> -    /// \param VariableName -- The name of the variable that holds the unique<br>
> -    /// value.<br>
> -    ///<br>
> -    /// \param Loc -- The SourceLocation of the method invocation.<br>
> -    virtual void warnUseWhileConsumed(StringRef MethodName,<br>
> -                                      StringRef VariableName,<br>
> -                                      SourceLocation Loc) {}<br>
> -<br>
> -    /// \brief Warn about use-in-unknown-state errors.<br>
> -    /// \param MethodName -- The name of the method that was incorrectly<br>
> -    /// invoked.<br>
> +    /// \param State -- The state the object was used in.<br>
>      ///<br>
>      /// \param VariableName -- The name of the variable that holds the unique<br>
>      /// value.<br>
>      ///<br>
>      /// \param Loc -- The SourceLocation of the method invocation.<br>
> -    virtual void warnUseInUnknownState(StringRef MethodName,<br>
> +    virtual void warnUseInInvalidState(StringRef MethodName,<br>
>                                         StringRef VariableName,<br>
> +                                       StringRef State,<br>
>                                         SourceLocation Loc) {}<br>
<br>
This is a bit late in coming, but why does ConsumedWarningsHandlerBase<br>
exist at all?  There is only one derivation of it, and that implements<br>
all of the virtual functions.<br>
<br></blockquote><div><br></div></div></div><div>LibAnalysis doesn't link against LibSema and therefore can't call PDiag or any of the other Sema functions necessary to issue warnings.  This gets around it by creating the implementation in Sema and routing calls to it in Analysis via the VTable.  I thought it was a bit weird as well.<br>

</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>    };<br>
><br>
> -  enum ConsumedState {<br>
> -    // No state information for the given variable.<br>
> -    CS_None,<br>
> -<br>
> -    CS_Unknown,<br>
> -    CS_Unconsumed,<br>
> -    CS_Consumed<br>
> -  };<br>
> -<br>
>    class ConsumedStateMap {<br>
><br>
>      typedef llvm::DenseMap<const VarDecl *, ConsumedState> MapType;<br>
> Index: include/clang/Basic/Attr.td<br>
> ===================================================================<br>
> --- include/clang/Basic/Attr.td<br>
> +++ include/clang/Basic/Attr.td<br>
> @@ -55,6 +55,9 @@<br>
>  class TypeArgument<string name> : Argument<name>;<br>
>  class UnsignedArgument<string name> : Argument<name>;<br>
>  class SourceLocArgument<string name> : Argument<name>;<br>
> +<br>
> +// FIXME: There should be a VariadicArgument type that takes any other type<br>
> +//        of argument and generates the appropriate type.<br>
<br>
Why?  Not that I disagree, but what does that have to do with the current patch?<br>
<br>
>  class VariadicUnsignedArgument<string name> : Argument<name>;<br>
>  class VariadicExprArgument<string name> : Argument<name>;<br>
><br>
> @@ -80,6 +83,13 @@<br>
>    list<string> Enums = enums;<br>
>  }<br>
><br>
> +class VariadicEnumArgument<string name, string type, list<string> values,<br>
> +                           list<string> enums> : Argument<name>  {<br>
> +  string Type = type;<br>
> +  list<string> Values = values;<br>
> +  list<string> Enums = enums;<br>
> +}<br>
> +<br>
>  // This handles one spelling of an attribute.<br>
>  class Spelling<string name, string variety> {<br>
>    string Name = name;<br>
> @@ -936,9 +946,12 @@<br>
>                             ["Unknown", "Consumed", "Unconsumed"]>];<br>
>  }<br>
><br>
> -def CallableWhenUnconsumed : InheritableAttr {<br>
> -  let Spellings = [GNU<"callable_when_unconsumed">];<br>
> +def CallableWhen : InheritableAttr {<br>
> +  let Spellings = [GNU<"callable_when">];<br>
>    let Subjects = [CXXMethod];<br>
> +  let Args = [VariadicEnumArgument<"CallableState", "ConsumedState",<br>
> +                                   ["unknown", "consumed", "unconsumed"],<br>
> +                                   ["Unknown", "Consumed", "Unconsumed"]>];<br>
>  }<br>
><br>
>  def TestsUnconsumed : InheritableAttr {<br>
> Index: include/clang/Basic/DiagnosticSemaKinds.td<br>
> ===================================================================<br>
> --- include/clang/Basic/DiagnosticSemaKinds.td<br>
> +++ include/clang/Basic/DiagnosticSemaKinds.td<br>
> @@ -2188,12 +2188,12 @@<br>
>    "Thread safety beta warning.">, InGroup<ThreadSafetyBeta>, DefaultIgnore;<br>
><br>
>  // Consumed warnings<br>
> -def warn_use_while_consumed : Warning<<br>
> -  "invocation of method '%0' on object '%1' while it is in the 'consumed' "<br>
> +def warn_use_in_invalid_state : Warning<<br>
> +  "invalid invocation of method '%0' on object '%1' while it is in the '%2' "<br>
>    "state">, InGroup<Consumed>, DefaultIgnore;<br>
> -def warn_use_of_temp_while_consumed : Warning<<br>
> -  "invocation of method '%0' on a temporary object while it is in the "<br>
> -  "'consumed' state">, InGroup<Consumed>, DefaultIgnore;<br>
> +def warn_use_of_temp_in_invalid_state : Warning<<br>
> +  "invalid invocation of method '%0' on a temporary object while it is in the "<br>
> +  "'%1' state">, InGroup<Consumed>, DefaultIgnore;<br>
>  def warn_attr_on_unconsumable_class : Warning<<br>
>    "consumed analysis attribute is attached to member of class '%0' which isn't "<br>
>    "marked as consumable">, InGroup<Consumed>, DefaultIgnore;<br>
> @@ -2207,12 +2207,6 @@<br>
>    InGroup<Consumed>, DefaultIgnore;<br>
><br>
>  // ConsumedStrict warnings<br>
> -def warn_use_in_unknown_state : Warning<<br>
> -  "invocation of method '%0' on object '%1' while it is in an unknown state">,<br>
> -  InGroup<ConsumedStrict>, DefaultIgnore;<br>
> -def warn_use_of_temp_in_unknown_state : Warning<<br>
> -  "invocation of method '%0' on a temporary object while it is in an unknown "<br>
> -  "state">, InGroup<ConsumedStrict>, DefaultIgnore;<br>
>  def warn_unnecessary_test : Warning<<br>
>    "unnecessary test. Variable '%0' is known to be in the '%1' state">,<br>
>    InGroup<ConsumedStrict>, DefaultIgnore;<br>
> Index: lib/Analysis/Consumed.cpp<br>
> ===================================================================<br>
> --- lib/Analysis/Consumed.cpp<br>
> +++ lib/Analysis/Consumed.cpp<br>
> @@ -33,6 +33,8 @@<br>
><br>
>  // TODO: Add notes about the actual and expected state for<br>
>  // TODO: Correctly identify unreachable blocks when chaining boolean operators.<br>
> +// TODO: Adjust the parser and AttributesList class to support lists of<br>
> +//       identifiers.<br>
>  // TODO: Warn about unreachable code.<br>
>  // TODO: Switch to using a bitmap to track unreachable blocks.<br>
>  // TODO: Mark variables as Unknown going into while- or for-loops only if they<br>
> @@ -66,6 +68,37 @@<br>
>    llvm_unreachable("invalid enum");<br>
>  }<br>
><br>
> +static bool isCallableInState(const CallableWhenAttr *CWAttr,<br>
> +                              ConsumedState State) {<br>
> +<br>
> +  CallableWhenAttr::callableState_iterator I = CWAttr->callableState_begin(),<br>
> +                                           E = CWAttr->callableState_end();<br>
> +<br>
> +  for (; I != E; ++I) {<br>
> +<br>
> +    ConsumedState MappedAttrState = CS_None;<br>
> +<br>
> +    switch (*I) {<br>
> +    case CallableWhenAttr::Unknown:<br>
> +      MappedAttrState = CS_Unknown;<br>
> +      break;<br>
> +<br>
> +    case CallableWhenAttr::Unconsumed:<br>
> +      MappedAttrState = CS_Unconsumed;<br>
> +      break;<br>
> +<br>
> +    case CallableWhenAttr::Consumed:<br>
> +      MappedAttrState = CS_Consumed;<br>
> +      break;<br>
> +    }<br>
> +<br>
> +    if (MappedAttrState == State)<br>
> +      return true;<br>
> +  }<br>
> +<br>
> +  return false;<br>
> +}<br>
<br>
Can't this loop be replaced with a standard algorithm?  It smells an<br>
awful lot like a std::find_if.<br>
<br></blockquote><div><br></div></div></div><div>It could, yes, but I would still have to provide a method to map between the CallableWhenAttr enum and the ConsumedState enum.  I think it would end up being more code due to the second function definition.<br>

</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> +<br>
>  static bool isConsumableType(const QualType &QT) {<br>
>    if (const CXXRecordDecl *RD = QT->getAsCXXRecordDecl())<br>
>      return RD->hasAttr<ConsumableAttr>();<br>
> @@ -174,6 +207,8 @@<br>
>      BinTestTy BinTest;<br>
>    };<br>
><br>
> +  QualType TempType;<br>
> +<br>
>  public:<br>
>    PropagationInfo() : InfoType(IT_None) {}<br>
><br>
> @@ -208,7 +243,9 @@<br>
>      BinTest.RTest.TestsFor = RTestsFor;<br>
>    }<br>
><br>
> -  PropagationInfo(ConsumedState State) : InfoType(IT_State), State(State) {}<br>
> +  PropagationInfo(ConsumedState State, QualType TempType)<br>
> +    : InfoType(IT_State), State(State), TempType(TempType) {}<br>
> +<br>
>    PropagationInfo(const VarDecl *Var) : InfoType(IT_Var), Var(Var) {}<br>
><br>
>    const ConsumedState & getState() const {<br>
> @@ -216,6 +253,11 @@<br>
>      return State;<br>
>    }<br>
><br>
> +  const QualType & getTempType() const {<br>
<br>
Formatting.<br>
<br>
> +    assert(InfoType == IT_State);<br>
> +    return TempType;<br>
> +  }<br>
> +<br>
>    const VarTestResult & getTest() const {<br>
>      assert(InfoType == IT_Test);<br>
>      return Test;<br>
> @@ -327,51 +369,36 @@<br>
>    }<br>
>  };<br>
><br>
> -// TODO: When we support CallableWhenConsumed this will have to check for<br>
> -//       the different attributes and change the behavior bellow. (Deferred)<br>
>  void ConsumedStmtVisitor::checkCallability(const PropagationInfo &PInfo,<br>
>                                             const FunctionDecl *FunDecl,<br>
>                                             const CallExpr *Call) {<br>
><br>
> -  if (!FunDecl->hasAttr<CallableWhenUnconsumedAttr>()) return;<br>
> +  if (!FunDecl->hasAttr<CallableWhenAttr>()) return;<br>
<br>
Formatting (I know the original code had the same formatting, but the<br>
statement should go on a newline).<br>
<br>
> +  const CallableWhenAttr *CWAttr = FunDecl->getAttr<CallableWhenAttr>();<br>
><br>
>    if (PInfo.isVar()) {<br>
>      const VarDecl *Var = PInfo.getVar();<br>
> +    ConsumedState VarState = StateMap->getState(Var);<br>
><br>
> -    switch (StateMap->getState(Var)) {<br>
> -    case CS_Consumed:<br>
> -      Analyzer.WarningsHandler.warnUseWhileConsumed(<br>
> -        FunDecl->getNameAsString(), Var->getNameAsString(),<br>
> -        Call->getExprLoc());<br>
> -      break;<br>
> +    assert(VarState != CS_None && "Invalid state");<br>
><br>
> -    case CS_Unknown:<br>
> -      Analyzer.WarningsHandler.warnUseInUnknownState(<br>
> -        FunDecl->getNameAsString(), Var->getNameAsString(),<br>
> -        Call->getExprLoc());<br>
> -      break;<br>
> -<br>
> -    case CS_None:<br>
> -    case CS_Unconsumed:<br>
> -      break;<br>
> -    }<br>
> +    if (isCallableInState(CWAttr, VarState))<br>
> +      return;<br>
><br>
> -  } else {<br>
> -    switch (PInfo.getState()) {<br>
> -    case CS_Consumed:<br>
> -      Analyzer.WarningsHandler.warnUseOfTempWhileConsumed(<br>
> -        FunDecl->getNameAsString(), Call->getExprLoc());<br>
> -      break;<br>
> +    Analyzer.WarningsHandler.warnUseInInvalidState(<br>
> +      FunDecl->getNameAsString(), Var->getNameAsString(),<br>
> +      stateToString(VarState), Call->getExprLoc());<br>
><br>
> -    case CS_Unknown:<br>
> -      Analyzer.WarningsHandler.warnUseOfTempInUnknownState(<br>
> -        FunDecl->getNameAsString(), Call->getExprLoc());<br>
> -      break;<br>
> -<br>
> -    case CS_None:<br>
> -    case CS_Unconsumed:<br>
> -      break;<br>
> -    }<br>
> +  } else if (PInfo.isState()) {<br>
> +<br>
> +    assert(PInfo.getState() != CS_None && "Invalid state");<br>
> +<br>
> +    if (isCallableInState(CWAttr, PInfo.getState()))<br>
> +      return;<br>
> +<br>
> +    Analyzer.WarningsHandler.warnUseOfTempInInvalidState(<br>
> +      FunDecl->getNameAsString(), stateToString(PInfo.getState()),<br>
> +      Call->getExprLoc());<br>
>    }<br>
>  }<br>
><br>
> @@ -421,7 +448,7 @@<br>
>        ReturnState = mapConsumableAttrState(ReturnType);<br>
><br>
>      PropagationMap.insert(PairType(Call,<br>
> -      PropagationInfo(ReturnState)));<br>
> +      PropagationInfo(ReturnState, ReturnType)));<br>
>    }<br>
>  }<br>
><br>
> @@ -522,7 +549,11 @@<br>
>        }<br>
>      }<br>
><br>
> -    propagateReturnType(Call, FunDecl, FunDecl->getCallResultType());<br>
> +    QualType RetType = FunDecl->getCallResultType();<br>
> +    if (RetType->isReferenceType())<br>
<br>
What about pointer types?<br></blockquote><div><br></div></div></div><div>Pointers to consumable types aren't handled yet.  This will change in the near future.<br></div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<br>
> +      RetType = RetType->getPointeeType();<br>
> +<br>
> +    propagateReturnType(Call, FunDecl, RetType);<br>
>    }<br>
>  }<br>
><br>
> @@ -540,7 +571,7 @@<br>
>      if (Constructor->isDefaultConstructor()) {<br>
><br>
>        PropagationMap.insert(PairType(Call,<br>
> -        PropagationInfo(consumed::CS_Consumed)));<br>
> +        PropagationInfo(consumed::CS_Consumed, ThisType)));<br>
><br>
>      } else if (Constructor->isMoveConstructor()) {<br>
><br>
> @@ -551,7 +582,7 @@<br>
>          const VarDecl* Var = PInfo.getVar();<br>
><br>
>          PropagationMap.insert(PairType(Call,<br>
> -          PropagationInfo(StateMap->getState(Var))));<br>
> +          PropagationInfo(StateMap->getState(Var), ThisType)));<br>
><br>
>          StateMap->setState(Var, consumed::CS_Consumed);<br>
><br>
> @@ -630,7 +661,8 @@<br>
><br>
>        } else if (!LPInfo.isVar() && RPInfo.isVar()) {<br>
>          PropagationMap.insert(PairType(Call,<br>
> -          PropagationInfo(StateMap->getState(RPInfo.getVar()))));<br>
> +          PropagationInfo(StateMap->getState(RPInfo.getVar()),<br>
> +                          LPInfo.getTempType())));<br>
><br>
>          StateMap->setState(RPInfo.getVar(), consumed::CS_Consumed);<br>
><br>
> @@ -648,27 +680,16 @@<br>
><br>
>          PropagationMap.insert(PairType(Call, LPInfo));<br>
><br>
> -      } else {<br>
> +      } else if (LPInfo.isState()) {<br>
>          PropagationMap.insert(PairType(Call,<br>
> -          PropagationInfo(consumed::CS_Unknown)));<br>
> +          PropagationInfo(consumed::CS_Unknown, LPInfo.getTempType())));<br>
>        }<br>
><br>
>      } else if (LEntry == PropagationMap.end() &&<br>
>                 REntry != PropagationMap.end()) {<br>
><br>
> -      RPInfo = REntry->second;<br>
> -<br>
> -      if (RPInfo.isVar()) {<br>
> -        const VarDecl *Var = RPInfo.getVar();<br>
> -<br>
> -        PropagationMap.insert(PairType(Call,<br>
> -          PropagationInfo(StateMap->getState(Var))));<br>
> -<br>
> -        StateMap->setState(Var, consumed::CS_Consumed);<br>
> -<br>
> -      } else {<br>
> -        PropagationMap.insert(PairType(Call, RPInfo));<br>
> -      }<br>
> +      if (REntry->second.isVar())<br>
> +        StateMap->setState(REntry->second.getVar(), consumed::CS_Consumed);<br>
>      }<br>
><br>
>    } else {<br>
> @@ -776,6 +797,7 @@<br>
>    }<br>
>  }<br>
><br>
> +// TODO: See if I need to check for reference types here.<br>
>  void ConsumedStmtVisitor::VisitVarDecl(const VarDecl *Var) {<br>
>    if (isConsumableType(Var->getType())) {<br>
>      if (Var->hasInit()) {<br>
> @@ -803,13 +825,12 @@<br>
><br>
>    if (VarState == CS_Unknown) {<br>
>      ThenStates->setState(Test.Var, Test.TestsFor);<br>
> -    if (ElseStates)<br>
> -      ElseStates->setState(Test.Var, invertConsumedUnconsumed(Test.TestsFor));<br>
> +    ElseStates->setState(Test.Var, invertConsumedUnconsumed(Test.TestsFor));<br>
><br>
>    } else if (VarState == invertConsumedUnconsumed(Test.TestsFor)) {<br>
>      ThenStates->markUnreachable();<br>
><br>
> -  } else if (VarState == Test.TestsFor && ElseStates) {<br>
> +  } else if (VarState == Test.TestsFor) {<br>
>      ElseStates->markUnreachable();<br>
>    }<br>
>  }<br>
> @@ -832,31 +853,27 @@<br>
>          ThenStates->markUnreachable();<br>
><br>
>        } else if (LState == LTest.TestsFor && isKnownState(RState)) {<br>
> -        if (RState == RTest.TestsFor) {<br>
> -          if (ElseStates)<br>
> -            ElseStates->markUnreachable();<br>
> -        } else {<br>
> +        if (RState == RTest.TestsFor)<br>
> +          ElseStates->markUnreachable();<br>
> +        else<br>
>            ThenStates->markUnreachable();<br>
> -        }<br>
>        }<br>
><br>
>      } else {<br>
> -      if (LState == CS_Unknown && ElseStates) {<br>
> +      if (LState == CS_Unknown) {<br>
>          ElseStates->setState(LTest.Var,<br>
>                               invertConsumedUnconsumed(LTest.TestsFor));<br>
><br>
> -      } else if (LState == LTest.TestsFor && ElseStates) {<br>
> +      } else if (LState == LTest.TestsFor) {<br>
>          ElseStates->markUnreachable();<br>
><br>
>        } else if (LState == invertConsumedUnconsumed(LTest.TestsFor) &&<br>
>                   isKnownState(RState)) {<br>
><br>
> -        if (RState == RTest.TestsFor) {<br>
> -          if (ElseStates)<br>
> -            ElseStates->markUnreachable();<br>
> -        } else {<br>
> +        if (RState == RTest.TestsFor)<br>
> +          ElseStates->markUnreachable();<br>
> +        else<br>
>            ThenStates->markUnreachable();<br>
> -        }<br>
>        }<br>
>      }<br>
>    }<br>
> @@ -868,7 +885,7 @@<br>
>        else if (RState == invertConsumedUnconsumed(RTest.TestsFor))<br>
>          ThenStates->markUnreachable();<br>
><br>
> -    } else if (ElseStates) {<br>
> +    } else {<br>
>        if (RState == CS_Unknown)<br>
>          ElseStates->setState(RTest.Var,<br>
>                               invertConsumedUnconsumed(RTest.TestsFor));<br>
> @@ -1016,7 +1033,6 @@<br>
>    if (const IfStmt *IfNode =<br>
>      dyn_cast_or_null<IfStmt>(CurrBlock->getTerminator().getStmt())) {<br>
><br>
> -    bool HasElse = IfNode->getElse() != NULL;<br>
>      const Stmt *Cond = IfNode->getCond();<br>
><br>
>      PInfo = Visitor.getInfo(Cond);<br>
> @@ -1026,15 +1042,12 @@<br>
>      if (PInfo.isTest()) {<br>
>        CurrStates->setSource(Cond);<br>
>        FalseStates->setSource(Cond);<br>
> -<br>
> -      splitVarStateForIf(IfNode, PInfo.getTest(), CurrStates,<br>
> -                         HasElse ? FalseStates : NULL);<br>
> +      splitVarStateForIf(IfNode, PInfo.getTest(), CurrStates, FalseStates);<br>
><br>
>      } else if (PInfo.isBinTest()) {<br>
>        CurrStates->setSource(PInfo.testSourceNode());<br>
>        FalseStates->setSource(PInfo.testSourceNode());<br>
> -<br>
> -      splitVarStateForIfBinOp(PInfo, CurrStates, HasElse ? FalseStates : NULL);<br>
> +      splitVarStateForIfBinOp(PInfo, CurrStates, FalseStates);<br>
><br>
>      } else {<br>
>        delete FalseStates;<br>
> Index: lib/Sema/AnalysisBasedWarnings.cpp<br>
> ===================================================================<br>
> --- lib/Sema/AnalysisBasedWarnings.cpp<br>
> +++ lib/Sema/AnalysisBasedWarnings.cpp<br>
> @@ -1471,36 +1471,20 @@<br>
>      Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));<br>
>    }<br>
><br>
> -  void warnUseOfTempWhileConsumed(StringRef MethodName, SourceLocation Loc) {<br>
> +  void warnUseOfTempInInvalidState(StringRef MethodName, StringRef State,<br>
> +                                   SourceLocation Loc) {<br>
><br>
>      PartialDiagnosticAt Warning(Loc, S.PDiag(<br>
> -      diag::warn_use_of_temp_while_consumed) << MethodName);<br>
> +      diag::warn_use_of_temp_in_invalid_state) << MethodName << State);<br>
><br>
>      Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));<br>
>    }<br>
><br>
> -  void warnUseOfTempInUnknownState(StringRef MethodName, SourceLocation Loc) {<br>
> +  void warnUseInInvalidState(StringRef MethodName, StringRef VariableName,<br>
> +                                  StringRef State, SourceLocation Loc) {<br>
><br>
> -    PartialDiagnosticAt Warning(Loc, S.PDiag(<br>
> -      diag::warn_use_of_temp_in_unknown_state) << MethodName);<br>
> -<br>
> -    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));<br>
> -  }<br>
> -<br>
> -  void warnUseWhileConsumed(StringRef MethodName, StringRef VariableName,<br>
> -                            SourceLocation Loc) {<br>
> -<br>
> -    PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_use_while_consumed) <<<br>
> -                                MethodName << VariableName);<br>
> -<br>
> -    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));<br>
> -  }<br>
> -<br>
> -  void warnUseInUnknownState(StringRef MethodName, StringRef VariableName,<br>
> -                             SourceLocation Loc) {<br>
> -<br>
> -    PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_use_in_unknown_state) <<<br>
> -                                MethodName << VariableName);<br>
> +    PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_use_in_invalid_state) <<<br>
> +                                MethodName << VariableName << State);<br>
><br>
>      Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));<br>
>    }<br>
> @@ -1538,7 +1522,7 @@<br>
>      (D.getDiagnosticLevel(diag::warn_double_lock, SourceLocation()) !=<br>
>       DiagnosticsEngine::Ignored);<br>
>    DefaultPolicy.enableConsumedAnalysis = (unsigned)<br>
> -    (D.getDiagnosticLevel(diag::warn_use_while_consumed, SourceLocation()) !=<br>
> +    (D.getDiagnosticLevel(diag::warn_use_in_invalid_state, SourceLocation()) !=<br>
>       DiagnosticsEngine::Ignored);<br>
>  }<br>
><br>
> Index: lib/Sema/SemaDeclAttr.cpp<br>
> ===================================================================<br>
> --- lib/Sema/SemaDeclAttr.cpp<br>
> +++ lib/Sema/SemaDeclAttr.cpp<br>
> @@ -1038,9 +1038,10 @@<br>
>                            Attr.getAttributeSpellingListIndex()));<br>
>  }<br>
><br>
> -static void handleCallableWhenUnconsumedAttr(Sema &S, Decl *D,<br>
> -                                             const AttributeList &Attr) {<br>
> -  if (!checkAttributeNumArgs(S, Attr, 0)) return;<br>
> +static void handleCallableWhenAttr(Sema &S, Decl *D,<br>
> +                                   const AttributeList &Attr) {<br>
> +<br>
> +  if (!checkAttributeAtLeastNumArgs(S, Attr, 1)) return;<br>
><br>
>    if (!isa<CXXMethodDecl>(D)) {<br>
>      S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<<br>
> @@ -1051,9 +1052,38 @@<br>
>    if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))<br>
>      return;<br>
><br>
> +  SmallVector<CallableWhenAttr::ConsumedState, 3> States;<br>
> +  for (unsigned ArgIndex = 0; ArgIndex < Attr.getNumArgs(); ++ArgIndex) {<br>
> +    CallableWhenAttr::ConsumedState CallableState;<br>
> +<br>
> +    if (Attr.isArgExpr(ArgIndex) &&<br>
> +        isa<StringLiteral>(Attr.getArgAsExpr(ArgIndex))) {<br>
> +<br>
> +      Expr *Arg = Attr.getArgAsExpr(ArgIndex);<br>
> +      StringRef StateString = cast<StringLiteral>(Arg)->getString();<br>
<br>
I don't think it should be taking string literals; that's inconsistent<br>
with other consumable-related attributes.  It should be taking a list<br>
of unresolved identifiers.<br>
<br>
> +<br>
> +      if (StateString == "unknown") {<br>
> +        CallableState = CallableWhenAttr::Unknown;<br>
> +      } else if (StateString == "consumed") {<br>
> +        CallableState = CallableWhenAttr::Consumed;<br>
> +      } else if (StateString == "unconsumed") {<br>
> +        CallableState = CallableWhenAttr::Unconsumed;<br>
> +      } else {<br>
> +        S.Diag(Attr.getLoc(), diag::warn_unknown_consumed_state);<br>
> +        return;<br>
> +      }<br>
<br>
Formatting (the braces should be removed); might make sense to use a<br>
StringSwitch.  Actually, the more I think of it, the more I think we<br>
should have this sort of thing auto generated.  Every attribute<br>
enumeration is going to have the same general notion...<br>
<br></blockquote><div><br></div></div></div><div>A StringSwitch won't work in this case.  If there was an extra value in the CallableWhenAttr::ConsumedState enum the default could return that and I could test CallableState and issue the warning and return appropriately.   Unfortunately there is no extra enum value.  I agree that this is a common case and it would be possible to auto-generate code to handle it.  Additionally, it would be nice to be able to declare an enum in Attr.td, have it emitted along with a string-to-enum mapping function, and then be able to re-use that enum in multiple attributes... but that is code for another patch.<br>

</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> +<br>
> +      States.push_back(CallableState);<br>
> +    } else {<br>
> +      S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) << Attr.getName()<br>
> +        << AANT_ArgumentString;<br>
<br>
When the string literals are fixed, this should be changed to identifier.<br>
<br>
> +      return;<br>
> +    }<br>
> +  }<br>
> +<br>
>    D->addAttr(::new (S.Context)<br>
> -             CallableWhenUnconsumedAttr(Attr.getRange(), S.Context,<br>
> -                                        Attr.getAttributeSpellingListIndex()));<br>
> +             CallableWhenAttr(Attr.getRange(), S.Context, States.data(),<br>
> +               States.size(), Attr.getAttributeSpellingListIndex()));<br>
>  }<br>
><br>
>  static void handleTestsConsumedAttr(Sema &S, Decl *D,<br>
> @@ -5100,8 +5130,8 @@<br>
>    case AttributeList::AT_Consumes:<br>
>      handleConsumesAttr(S, D, Attr);<br>
>      break;<br>
> -  case AttributeList::AT_CallableWhenUnconsumed:<br>
> -    handleCallableWhenUnconsumedAttr(S, D, Attr);<br>
> +  case AttributeList::AT_CallableWhen:<br>
> +    handleCallableWhenAttr(S, D, Attr);<br>
>      break;<br>
>    case AttributeList::AT_TestsConsumed:<br>
>      handleTestsConsumedAttr(S, D, Attr);<br>
> Index: test/SemaCXX/warn-consumed-analysis-strict.cpp<br>
> ===================================================================<br>
> --- test/SemaCXX/warn-consumed-analysis-strict.cpp<br>
> +++ test/SemaCXX/warn-consumed-analysis-strict.cpp<br>
> @@ -1,10 +1,10 @@<br>
>  // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed-strict -std=c++11 %s<br>
><br>
> -#define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed))<br>
> -#define CONSUMABLE(state)        __attribute__ ((consumable(state)))<br>
> -#define CONSUMES                 __attribute__ ((consumes))<br>
> -#define RETURN_TYPESTATE(state)  __attribute__ ((return_typestate(state)))<br>
> -#define TESTS_UNCONSUMED         __attribute__ ((tests_unconsumed))<br>
> +#define CALLABLE_WHEN(...)      __attribute__ ((callable_when(__VA_ARGS__)))<br>
> +#define CONSUMABLE(state)       __attribute__ ((consumable(state)))<br>
> +#define CONSUMES                __attribute__ ((consumes))<br>
> +#define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))<br>
> +#define TESTS_UNCONSUMED        __attribute__ ((tests_unconsumed))<br>
><br>
>  #define TEST_VAR(Var) Var.isValid()<br>
><br>
> @@ -32,8 +32,8 @@<br>
>    ConsumableClass<T>& operator=(ConsumableClass<U> &&other);<br>
><br>
>    void operator()(int a) CONSUMES;<br>
> -  void operator*() const CALLABLE_WHEN_UNCONSUMED;<br>
> -  void unconsumedCall() const CALLABLE_WHEN_UNCONSUMED;<br>
> +  void operator*() const CALLABLE_WHEN("unconsumed");<br>
> +  void unconsumedCall() const CALLABLE_WHEN("unconsumed");<br>
><br>
>    bool isValid() const TESTS_UNCONSUMED;<br>
>    operator bool() const TESTS_UNCONSUMED;<br>
> @@ -45,9 +45,6 @@<br>
>    void consume() CONSUMES;<br>
>  };<br>
><br>
> -void baf0(ConsumableClass<int>  &var);<br>
> -void baf1(ConsumableClass<int>  *var);<br>
> -<br>
>  void testIfStmt() {<br>
>    ConsumableClass<int> var;<br>
><br>
> @@ -60,137 +57,6 @@<br>
>    }<br>
>  }<br>
><br>
> -void testComplexConditionals() {<br>
> -  ConsumableClass<int> var0, var1, var2;<br>
> -<br>
> -  // Coerce all variables into the unknown state.<br>
> -  baf0(var0);<br>
> -  baf0(var1);<br>
> -  baf0(var2);<br>
> -<br>
> -  if (var0 && var1) {<br>
> -    *var0;<br>
> -    *var1;<br>
> -<br>
> -  } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -  }<br>
> -<br>
> -  if (var0 || var1) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -<br>
> -  } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> -  }<br>
> -<br>
> -  if (var0 && !var1) {<br>
> -    *var0;<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> -<br>
> -  } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -  }<br>
> -<br>
> -  if (var0 || !var1) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -<br>
> -  } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1;<br>
> -  }<br>
> -<br>
> -  if (!var0 && !var1) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> -<br>
> -  } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -  }<br>
> -<br>
> -  if (!(var0 || var1)) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> -<br>
> -  } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -  }<br>
> -<br>
> -  if (!var0 || !var1) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -<br>
> -  } else {<br>
> -    *var0;<br>
> -    *var1;<br>
> -  }<br>
> -<br>
> -  if (!(var0 && var1)) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -<br>
> -  } else {<br>
> -    *var0;<br>
> -    *var1;<br>
> -  }<br>
> -<br>
> -  if (var0 && var1 && var2) {<br>
> -    *var0;<br>
> -    *var1;<br>
> -    *var2;<br>
> -<br>
> -  } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -    *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in an unknown state}}<br>
> -  }<br>
> -<br>
> -#if 0<br>
> -  // FIXME: Get this test to pass.<br>
> -  if (var0 || var1 || var2) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in an unknown state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in an unknown state}}<br>
> -    *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in an unknown state}}<br>
> -<br>
> -  } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> -    *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}<br>
> -  }<br>
> -#endif<br>
> -}<br>
> -<br>
> -void testCallingConventions() {<br>
> -  ConsumableClass<int> var(42);<br>
> -<br>
> -  baf0(var);<br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}<br>
> -<br>
> -  var = ConsumableClass<int>(42);<br>
> -  baf1(&var);<br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}<br>
> -}<br>
> -<br>
> -void testConstAndNonConstMemberFunctions() {<br>
> -  ConsumableClass<int> var(42);<br>
> -<br>
> -  var.constCall();<br>
> -  *var;<br>
> -<br>
> -  var.nonconstCall();<br>
> -  *var;<br>
> -}<br>
> -<br>
> -void testFunctionParam0(ConsumableClass<int> &param) {<br>
> -  *param; // expected-warning {{invocation of method 'operator*' on object 'param' while it is in an unknown state}}<br>
> -}<br>
> -<br>
>  void testNoWarnTestFromMacroExpansion() {<br>
>    ConsumableClass<int> var(42);<br>
><br>
> @@ -198,26 +64,3 @@<br>
>      *var;<br>
>    }<br>
>  }<br>
> -<br>
> -void testSimpleForLoop() {<br>
> -  ConsumableClass<int> var;<br>
> -<br>
> -  for (int i = 0; i < 10; ++i) {<br>
> -    *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}<br>
> -  }<br>
> -<br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}<br>
> -}<br>
> -<br>
> -void testSimpleWhileLoop() {<br>
> -  int i = 0;<br>
> -<br>
> -  ConsumableClass<int> var;<br>
> -<br>
> -  while (i < 10) {<br>
> -    *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}<br>
> -    ++i;<br>
> -  }<br>
> -<br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in an unknown state}}<br>
> -}<br>
> Index: test/SemaCXX/warn-consumed-analysis.cpp<br>
> ===================================================================<br>
> --- test/SemaCXX/warn-consumed-analysis.cpp<br>
> +++ test/SemaCXX/warn-consumed-analysis.cpp<br>
> @@ -1,10 +1,12 @@<br>
>  // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s<br>
><br>
> -#define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed))<br>
> -#define CONSUMABLE(state)        __attribute__ ((consumable(state)))<br>
> -#define CONSUMES                 __attribute__ ((consumes))<br>
> -#define RETURN_TYPESTATE(state)  __attribute__ ((return_typestate(state)))<br>
> -#define TESTS_UNCONSUMED         __attribute__ ((tests_unconsumed))<br>
> +// TODO: Switch to using macros for the expected warnings.<br>
> +<br>
> +#define CALLABLE_WHEN(...)      __attribute__ ((callable_when(__VA_ARGS__)))<br>
> +#define CONSUMABLE(state)       __attribute__ ((consumable(state)))<br>
> +#define CONSUMES                __attribute__ ((consumes))<br>
> +#define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))<br>
> +#define TESTS_UNCONSUMED        __attribute__ ((tests_unconsumed))<br>
><br>
>  typedef decltype(nullptr) nullptr_t;<br>
><br>
> @@ -30,8 +32,9 @@<br>
>    ConsumableClass<T>& operator=(ConsumableClass<U> &&other);<br>
><br>
>    void operator()(int a) CONSUMES;<br>
> -  void operator*() const CALLABLE_WHEN_UNCONSUMED;<br>
> -  void unconsumedCall() const CALLABLE_WHEN_UNCONSUMED;<br>
> +  void operator*() const CALLABLE_WHEN("unconsumed");<br>
> +  void unconsumedCall() const CALLABLE_WHEN("unconsumed");<br>
> +  void callableWhenUnknown() const CALLABLE_WHEN("unconsumed", "unknown");<br>
><br>
>    bool isValid() const TESTS_UNCONSUMED;<br>
>    operator bool() const TESTS_UNCONSUMED;<br>
> @@ -47,7 +50,9 @@<br>
>  void baf1(const ConsumableClass<int> &var);<br>
>  void baf2(const ConsumableClass<int> *var);<br>
><br>
> -void baf3(ConsumableClass<int> &&var);<br>
> +void baf3(ConsumableClass<int>  &var);<br>
> +void baf4(ConsumableClass<int>  *var);<br>
> +void baf5(ConsumableClass<int> &&var);<br>
><br>
>  ConsumableClass<int> returnsUnconsumed() {<br>
>    return ConsumableClass<int>(); // expected-warning {{return value not in expected state; expected 'unconsumed', observed 'consumed'}}<br>
> @@ -58,39 +63,41 @@<br>
>    return ConsumableClass<int>();<br>
>  }<br>
><br>
> +ConsumableClass<int> returnsUnknown() RETURN_TYPESTATE(unknown);<br>
> +<br>
>  void testInitialization() {<br>
>    ConsumableClass<int> var0;<br>
>    ConsumableClass<int> var1 = ConsumableClass<int>();<br>
><br>
>    var0 = ConsumableClass<int>();<br>
><br>
> -  *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -  *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +  *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +  *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
><br>
>    if (var0.isValid()) {<br>
>      *var0;<br>
>      *var1;<br>
><br>
>    } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
>    }<br>
>  }<br>
><br>
>  void testTempValue() {<br>
> -  *ConsumableClass<int>(); // expected-warning {{invocation of method 'operator*' on a temporary object while it is in the 'consumed' state}}<br>
> +  *ConsumableClass<int>(); // expected-warning {{invalid invocation of method 'operator*' on a temporary object while it is in the 'consumed' state}}<br>
>  }<br>
><br>
>  void testSimpleRValueRefs() {<br>
>    ConsumableClass<int> var0;<br>
>    ConsumableClass<int> var1(42);<br>
><br>
> -  *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +  *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
>    *var1;<br>
><br>
>    var0 = static_cast<ConsumableClass<int>&&>(var1);<br>
><br>
>    *var0;<br>
> -  *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +  *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
>  }<br>
><br>
>  void testIfStmt() {<br>
> @@ -99,11 +106,11 @@<br>
>    if (var.isValid()) {<br>
>      *var;<br>
>    } else {<br>
> -    *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +    *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
>    }<br>
><br>
>    if (!var.isValid()) {<br>
> -    *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +    *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
>    } else {<br>
>      *var;<br>
>    }<br>
> @@ -111,17 +118,17 @@<br>
>    if (var) {<br>
>      // Empty<br>
>    } else {<br>
> -    *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +    *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
>    }<br>
><br>
>    if (var != nullptr) {<br>
>      // Empty<br>
>    } else {<br>
> -    *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +    *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
>    }<br>
>  }<br>
><br>
> -void testComplexConditionals() {<br>
> +void testComplexConditionals0() {<br>
>    ConsumableClass<int> var0, var1, var2;<br>
><br>
>    if (var0 && var1) {<br>
> @@ -129,8 +136,8 @@<br>
>      *var1;<br>
><br>
>    } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
>    }<br>
><br>
>    if (var0 || var1) {<br>
> @@ -138,8 +145,8 @@<br>
>      *var1;<br>
><br>
>    } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
>    }<br>
><br>
>    if (var0 && !var1) {<br>
> @@ -147,13 +154,13 @@<br>
>      *var1;<br>
><br>
>    } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
>    }<br>
><br>
>    if (var0 || !var1) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
><br>
>    } else {<br>
>      *var0;<br>
> @@ -161,8 +168,8 @@<br>
>    }<br>
><br>
>    if (!var0 && !var1) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
><br>
>    } else {<br>
>      *var0;<br>
> @@ -170,8 +177,8 @@<br>
>    }<br>
><br>
>    if (!var0 || !var1) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
><br>
>    } else {<br>
>      *var0;<br>
> @@ -179,8 +186,8 @@<br>
>    }<br>
><br>
>    if (!(var0 && var1)) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
><br>
>    } else {<br>
>      *var0;<br>
> @@ -188,8 +195,8 @@<br>
>    }<br>
><br>
>    if (!(var0 || var1)) {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
><br>
>    } else {<br>
>      *var0;<br>
> @@ -202,9 +209,9 @@<br>
>      *var2;<br>
><br>
>    } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> -    *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}<br>
>    }<br>
><br>
>  #if 0<br>
> @@ -215,9 +222,115 @@<br>
>      *var2;<br>
><br>
>    } else {<br>
> -    *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> -    *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> -    *var2; // expected-warning {{invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}<br>
> +  }<br>
> +#endif<br>
> +}<br>
> +<br>
> +void testComplexConditionals1() {<br>
> +  ConsumableClass<int> var0, var1, var2;<br>
> +<br>
> +  // Coerce all variables into the unknown state.<br>
> +  baf3(var0);<br>
> +  baf3(var1);<br>
> +  baf3(var2);<br>
> +<br>
> +  if (var0 && var1) {<br>
> +    *var0;<br>
> +    *var1;<br>
> +<br>
> +  } else {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +  }<br>
> +<br>
> +  if (var0 || var1) {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +<br>
> +  } else {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +  }<br>
> +<br>
> +  if (var0 && !var1) {<br>
> +    *var0;<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +<br>
> +  } else {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +  }<br>
> +<br>
> +  if (var0 || !var1) {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +<br>
> +  } else {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1;<br>
> +  }<br>
> +<br>
> +  if (!var0 && !var1) {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +<br>
> +  } else {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +  }<br>
> +<br>
> +  if (!(var0 || var1)) {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +<br>
> +  } else {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +  }<br>
> +<br>
> +  if (!var0 || !var1) {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +<br>
> +  } else {<br>
> +    *var0;<br>
> +    *var1;<br>
> +  }<br>
> +<br>
> +  if (!(var0 && var1)) {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +<br>
> +  } else {<br>
> +    *var0;<br>
> +    *var1;<br>
> +  }<br>
> +<br>
> +  if (var0 && var1 && var2) {<br>
> +    *var0;<br>
> +    *var1;<br>
> +    *var2;<br>
> +<br>
> +  } else {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +    *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'unknown' state}}<br>
> +  }<br>
> +<br>
> +#if 0<br>
> +  // FIXME: Get this test to pass.<br>
> +  if (var0 || var1 || var2) {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'unknown' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'unknown' state}}<br>
> +    *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'unknown' state}}<br>
> +<br>
> +  } else {<br>
> +    *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +    *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +    *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}<br>
>    }<br>
>  #endif<br>
>  }<br>
> @@ -226,7 +339,7 @@<br>
>    ConsumableClass<int> var;<br>
><br>
>    // Make var enter the 'unknown' state.<br>
> -  baf1(var);<br>
> +  baf3(var);<br>
><br>
>    if (!var) {<br>
>      var = ConsumableClass<int>(42);<br>
> @@ -259,8 +372,34 @@<br>
>    baf2(&var);<br>
>    *var;<br>
><br>
> -  baf3(static_cast<ConsumableClass<int>&&>(var));<br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +  baf3(var);<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}<br>
> +<br>
> +  var = ConsumableClass<int>(42);<br>
> +  baf4(&var);<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}<br>
> +<br>
> +  var = ConsumableClass<int>(42);<br>
> +  baf5(static_cast<ConsumableClass<int>&&>(var));<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +}<br>
> +<br>
> +void testConstAndNonConstMemberFunctions() {<br>
> +  ConsumableClass<int> var(42);<br>
> +<br>
> +  var.constCall();<br>
> +  *var;<br>
> +<br>
> +  var.nonconstCall();<br>
> +  *var;<br>
> +}<br>
> +<br>
> +void testFunctionParam0(ConsumableClass<int> param) {<br>
> +  *param;<br>
> +}<br>
> +<br>
> +void testFunctionParam1(ConsumableClass<int> &param) {<br>
> +  *param; // expected-warning {{invalid invocation of method 'operator*' on object 'param' while it is in the 'unknown' state}}<br>
>  }<br>
><br>
>  void testReturnStates() {<br>
> @@ -270,24 +409,34 @@<br>
>    *var;<br>
><br>
>    var = returnsConsumed();<br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +}<br>
> +<br>
> +void testCallableWhen() {<br>
> +  ConsumableClass<int> var(42);<br>
> +<br>
> +  *var;<br>
> +<br>
> +  baf3(var);<br>
> +<br>
> +  var.callableWhenUnknown();<br>
>  }<br>
><br>
>  void testMoveAsignmentish() {<br>
>    ConsumableClass<int>  var0;<br>
>    ConsumableClass<long> var1(42);<br>
><br>
> -  *var0; // expected-warning {{invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
> +  *var0; // expected-warning {{invalid invocation of method 'operator*' on object 'var0' while it is in the 'consumed' state}}<br>
>    *var1;<br>
><br>
>    var0 = static_cast<ConsumableClass<long>&&>(var1);<br>
><br>
>    *var0;<br>
> -  *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +  *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
><br>
>    var1 = ConsumableClass<long>(42);<br>
>    var1 = nullptr;<br>
> -  *var1; // expected-warning {{invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
> +  *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}<br>
>  }<br>
><br>
>  void testConditionalMerge() {<br>
> @@ -297,7 +446,7 @@<br>
>      // Empty<br>
>    }<br>
><br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
><br>
>    if (var.isValid()) {<br>
>      // Empty<br>
> @@ -305,7 +454,7 @@<br>
>      // Empty<br>
>    }<br>
><br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
>  }<br>
><br>
>  void testConsumes0() {<br>
> @@ -315,13 +464,13 @@<br>
><br>
>    var.consume();<br>
><br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
>  }<br>
><br>
>  void testConsumes1() {<br>
>    ConsumableClass<int> var(nullptr);<br>
><br>
> -  *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}<br>
>  }<br>
><br>
>  void testConsumes2() {<br>
> @@ -330,10 +479,10 @@<br>
>    var.unconsumedCall();<br>
>    var(6);<br>
><br>
> -  var.unconsumedCall(); // expected-warning {{invocation of method 'unconsumedCall' on object 'var' while it is in the 'consumed' state}}<br>
> +  var.unconsumedCall(); // expected-warning {{invalid invocation of method 'unconsumedCall' on object 'var' while it is in the 'consumed' state}}<br>
>  }<br>
><br>
> -void testNonsenseState() {<br>
> +void testUnreachableBlock() {<br>
>    ConsumableClass<int> var(42);<br>
><br>
>    if (var) {<br>
> @@ -349,10 +498,10 @@<br>
>    ConsumableClass<int> var;<br>
><br>
>    for (int i = 0; i < 10; ++i) {<br>
> -    *var;<br>
> +    *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}<br>
>    }<br>
><br>
> -  *var;<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}<br>
>  }<br>
><br>
>  void testSimpleWhileLoop() {<br>
> @@ -361,9 +510,9 @@<br>
>    ConsumableClass<int> var;<br>
><br>
>    while (i < 10) {<br>
> -    *var;<br>
> +    *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}<br>
>      ++i;<br>
>    }<br>
><br>
> -  *var;<br>
> +  *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}<br>
>  }<br>
> Index: test/SemaCXX/warn-consumed-parsing.cpp<br>
> ===================================================================<br>
> --- test/SemaCXX/warn-consumed-parsing.cpp<br>
> +++ test/SemaCXX/warn-consumed-parsing.cpp<br>
> @@ -1,6 +1,6 @@<br>
>  // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s<br>
><br>
> -#define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed))<br>
> +#define CALLABLE_WHEN(...)      __attribute__ ((callable_when(__VA_ARGS__)))<br>
>  #define CONSUMABLE(state)        __attribute__ ((consumable(state)))<br>
>  #define CONSUMES                 __attribute__ ((consumes))<br>
>  #define RETURN_TYPESTATE(state)  __attribute__ ((return_typestate(state)))<br>
> @@ -19,33 +19,34 @@<br>
>  class AttrTester0 {<br>
>    void consumes()        __attribute__ ((consumes(42))); // expected-error {{attribute takes no arguments}}<br>
>    bool testsUnconsumed() __attribute__ ((tests_unconsumed(42))); // expected-error {{attribute takes no arguments}}<br>
> -  void callableWhenUnconsumed() __attribute__ ((callable_when_unconsumed(42))); // expected-error {{attribute takes no arguments}}<br>
> +  void callableWhen()    __attribute__ ((callable_when())); // expected-error {{attribute takes at least 1 argument}}<br>
>  };<br>
><br>
>  int var0 CONSUMES; // expected-warning {{'consumes' attribute only applies to methods}}<br>
>  int var1 TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed' attribute only applies to methods}}<br>
> -int var2 CALLABLE_WHEN_UNCONSUMED; // expected-warning {{'callable_when_unconsumed' attribute only applies to methods}}<br>
> +int var2 CALLABLE_WHEN(42); // expected-warning {{'callable_when' attribute only applies to methods}}<br>
>  int var3 CONSUMABLE(consumed); // expected-warning {{'consumable' attribute only applies to classes}}<br>
>  int var4 RETURN_TYPESTATE(consumed); // expected-warning {{'return_typestate' attribute only applies to functions}}<br>
><br>
>  void function0() CONSUMES; // expected-warning {{'consumes' attribute only applies to methods}}<br>
>  void function1() TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed' attribute only applies to methods}}<br>
> -void function2() CALLABLE_WHEN_UNCONSUMED; // expected-warning {{'callable_when_unconsumed' attribute only applies to methods}}<br>
> +void function2() CALLABLE_WHEN(42); // expected-warning {{'callable_when' attribute only applies to methods}}<br>
>  void function3() CONSUMABLE(consumed); // expected-warning {{'consumable' attribute only applies to classes}}<br>
><br>
>  class CONSUMABLE(unknown) AttrTester1 {<br>
> -  void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED;<br>
> -  void consumes()               CONSUMES;<br>
> -  bool testsUnconsumed()        TESTS_UNCONSUMED;<br>
> +  void callableWhen0()   CALLABLE_WHEN("unconsumed");<br>
> +  void callableWhen1()   CALLABLE_WHEN(42); // expected-error {{'callable_when' attribute requires a string}}<br>
> +  void consumes()        CONSUMES;<br>
> +  bool testsUnconsumed() TESTS_UNCONSUMED;<br>
>  };<br>
><br>
>  AttrTester1 returnTypestateTester0() RETURN_TYPESTATE(not_a_state); // expected-warning {{unknown consumed analysis state 'not_a_state'}}<br>
>  AttrTester1 returnTypestateTester1() RETURN_TYPESTATE(42); // expected-error {{'return_typestate' attribute requires an identifier}}<br>
><br>
>  class AttrTester2 {<br>
> -  void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED; // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}<br>
> -  void consumes()               CONSUMES; // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}<br>
> -  bool testsUnconsumed()        TESTS_UNCONSUMED; // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}<br>
> +  void callableWhen()    CALLABLE_WHEN("unconsumed"); // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}<br>
> +  void consumes()        CONSUMES; // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}<br>
> +  bool testsUnconsumed() TESTS_UNCONSUMED; // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}<br>
>  };<br>
><br>
>  class CONSUMABLE(42) AttrTester3; // expected-error {{'consumable' attribute requires an identifier}}<br>
> Index: utils/TableGen/ClangAttrEmitter.cpp<br>
> ===================================================================<br>
> --- utils/TableGen/ClangAttrEmitter.cpp<br>
> +++ utils/TableGen/ClangAttrEmitter.cpp<br>
> @@ -449,15 +449,15 @@<br>
>        OS << "  " << getType() << " *" << getLowerName() << ";";<br>
>      }<br>
>      void writePCHReadDecls(raw_ostream &OS) const {<br>
> -      OS << "  unsigned " << getLowerName() << "Size = Record[Idx++];\n";<br>
> -      OS << "  SmallVector<" << type << ", 4> " << getLowerName()<br>
> +      OS << "    unsigned " << getLowerName() << "Size = Record[Idx++];\n";<br>
> +      OS << "    SmallVector<" << type << ", 4> " << getLowerName()<br>
>           << ";\n";<br>
> -      OS << "  " << getLowerName() << ".reserve(" << getLowerName()<br>
> +      OS << "    " << getLowerName() << ".reserve(" << getLowerName()<br>
>           << "Size);\n";<br>
> -      OS << "  for (unsigned i = " << getLowerName() << "Size; i; --i)\n";<br>
> +      OS << "    for (unsigned i = " << getLowerName() << "Size; i; --i)\n";<br>
><br>
>        std::string read = ReadPCHRecord(type);<br>
> -      OS << "    " << getLowerName() << ".push_back(" << read << ");\n";<br>
> +      OS << "      " << getLowerName() << ".push_back(" << read << ");\n";<br>
>      }<br>
<br>
The formatting changes are good, but should be a separate patch.<br>
<br>
>      void writePCHReadArgs(raw_ostream &OS) const {<br>
>        OS << getLowerName() << ".data(), " << getLowerName() << "Size";<br>
> @@ -563,6 +563,76 @@<br>
>        OS << "    }\n";<br>
>      }<br>
>    };<br>
> +<br>
> +  class VariadicEnumArgument: public VariadicArgument {<br>
> +    std::string type, QualifiedTypeName;<br>
> +    std::vector<StringRef> values, enums, uniques;<br>
<br>
Formatting (the names should follow the style guidelines).<br></blockquote><div><br></div></div></div><div>Which name, and which style guideline?  The QualifiedTypeName follows the Clang LLVM style guidelines, and the rest are coppied from other places in the ClangAttrEmitter.cpp file.<br>

</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +  public:<br>
> +    VariadicEnumArgument(Record &Arg, StringRef Attr)<br>
> +      : VariadicArgument(Arg, Attr, Arg.getValueAsString("Type")),<br>
> +        type(Arg.getValueAsString("Type")),<br>
> +        values(getValueAsListOfStrings(Arg, "Values")),<br>
> +        enums(getValueAsListOfStrings(Arg, "Enums")),<br>
> +        uniques(enums)<br>
> +    {<br>
> +      // Calculate the various enum values<br>
> +      std::sort(uniques.begin(), uniques.end());<br>
> +      uniques.erase(std::unique(uniques.begin(), uniques.end()), uniques.end());<br>
<br>
This strikes me as hiding a bug with the user's written enumerations.<br>
If they say: __attribute__((foo(one, one, one))) we should 1) report<br>
that to Sema because that information may be important to some<br>
attribute, and 2) Sema (in your case) should report the duplication.<br>
Ideally, there would be some helper method that Sema can optionally<br>
call to ensure uniqueness within the list and report a generic error<br>
about duplication.<br> 
<br></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> +<br>
> +      QualifiedTypeName = getAttrName().str() + "Attr::" + type;<br>
> +<br>
> +      // FIXME: Emit a proper error<br>
> +      assert(!uniques.empty());<br>
<br>
Please fix the fixme.<br></blockquote><div><br></div></div><div>This should probably be a seperate patch, as this FIXME is copied from above.  As well, I'm not entirely sure how Tablegen emits warnings concerning problems with the .td files.  This is to warn if the enum declaration array for EnumArgument (or VariadicEnumArgument) contains zero values.<br>

</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +    }<br>
> +<br>
> +    void writeDeclarations(raw_ostream &OS) const {<br>
> +      std::vector<StringRef>::const_iterator i = uniques.begin(),<br>
> +                                             e = uniques.end();<br>
> +      // The last one needs to not have a comma.<br>
> +      --e;<br>
> +<br>
> +      OS << "public:\n";<br>
> +      OS << "  enum " << type << " {\n";<br>
> +      for (; i != e; ++i)<br>
> +        OS << "    " << *i << ",\n";<br>
> +      OS << "    " << *e << "\n";<br>
> +      OS << "  };\n";<br>
> +      OS << "private:\n";<br>
> +<br>
> +      VariadicArgument::writeDeclarations(OS);<br>
> +    }<br>
> +    void writeDump(raw_ostream &OS) const {<br>
> +      OS << "    for (" << getAttrName() << "Attr::" << getLowerName()<br>
> +         << "_iterator I = SA->" << getLowerName() << "_begin(), E = SA->"<br>
> +         << getLowerName() << "_end(); I != E; ++I) {\n";<br>
> +      OS << "      switch(*I) {\n";<br>
> +      for (std::vector<StringRef>::const_iterator UI = uniques.begin(),<br>
> +           UE = uniques.end(); UI != UE; ++UI) {<br>
> +        OS << "    case " << getAttrName() << "Attr::" << *UI << ":\n";<br>
> +        OS << "      OS << \" " << *UI << "\";\n";<br>
> +        OS << "      break;\n";<br>
> +      }<br>
> +      OS << "      }\n";<br>
> +      OS << "    }\n";<br>
> +    }<br>
> +    void writePCHReadDecls(raw_ostream &OS) const {<br>
> +      OS << "    unsigned " << getLowerName() << "Size = Record[Idx++];\n";<br>
> +      OS << "    SmallVector<" << QualifiedTypeName << ", 4> " << getLowerName()<br>
> +         << ";\n";<br>
> +      OS << "    " << getLowerName() << ".reserve(" << getLowerName()<br>
> +         << "Size);\n";<br>
> +      OS << "    for (unsigned i = " << getLowerName() << "Size; i; --i)\n";<br>
> +      OS << "      " << getLowerName() << ".push_back(" << "static_cast<"<br>
> +         << QualifiedTypeName << ">(Record[Idx++]));\n";<br>
> +    }<br>
> +    void writePCHWrite(raw_ostream &OS) const{<br>
> +      OS << "    Record.push_back(SA->" << getLowerName() << "_size());\n";<br>
> +      OS << "    for (" << getAttrName() << "Attr::" << getLowerName()<br>
> +         << "_iterator i = SA->" << getLowerName() << "_begin(), e = SA->"<br>
> +         << getLowerName() << "_end(); i != e; ++i)\n";<br>
> +      OS << "      " << WritePCHRecord(QualifiedTypeName, "(*i)");<br>
> +    }<br>
> +  };<br>
><br>
>    class VersionArgument : public Argument {<br>
>    public:<br>
> @@ -724,6 +794,8 @@<br>
>      Ptr = new SimpleArgument(Arg, Attr, "SourceLocation");<br>
>    else if (ArgName == "VariadicUnsignedArgument")<br>
>      Ptr = new VariadicArgument(Arg, Attr, "unsigned");<br>
> +  else if (ArgName == "VariadicEnumArgument")<br>
> +    Ptr = new VariadicEnumArgument(Arg, Attr);<br>
>    else if (ArgName == "VariadicExprArgument")<br>
>      Ptr = new VariadicExprArgument(Arg, Attr);<br>
>    else if (ArgName == "VersionArgument")<br>
><br>
<span><font color="#888888"><br>
~Aaron<br>
</font></span><div><div><br>
On Thu, Sep 5, 2013 at 9:55 PM, Christian Wailes <<a href="mailto:chriswailes@google.com" target="_blank">chriswailes@google.com</a>> wrote:<br>
> Hi dblaikie, delesley, rsmith, aaron.ballman,<br>
><br>
> The callable_when annotation can take a list of states that a function can be called in.  This reduced the total number of annotations needed and makes writing more complicated behaviour less burdensome.<br>
><br>
> To allow the the callable_when attribute to hold a variable number of states a VariadicEnumArgument Tablegen attribute classes was added.  Future work should replace the various Variadic attribute classes with a single class that takes another attribute as a parameter.  A FIXME has been added to indicate this at the request of Richard Smith.<br>


><br>
> <a href="http://llvm-reviews.chandlerc.com/D1613" target="_blank">http://llvm-reviews.chandlerc.com/D1613</a><br>
><br>
> Files:<br>
>   include/clang/Analysis/Analyses/Consumed.h<br>
>   include/clang/Basic/Attr.td<br>
>   include/clang/Basic/DiagnosticSemaKinds.td<br>
>   lib/Analysis/Consumed.cpp<br>
>   lib/Sema/AnalysisBasedWarnings.cpp<br>
>   lib/Sema/SemaDeclAttr.cpp<br>
>   test/SemaCXX/warn-consumed-analysis-strict.cpp<br>
>   test/SemaCXX/warn-consumed-analysis.cpp<br>
>   test/SemaCXX/warn-consumed-parsing.cpp<br>
>   utils/TableGen/ClangAttrEmitter.cpp<br>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>