Consumed checker: various improvements

[comex]

Summary

• Treat arguments to constructor calls the same way as arguments to other calls (fixes 42856). Previously, arguments to constructors were completely ignored – even if explicitly marked param_typestate or return_typestate – except for one special case to mark the argument to a move constructor as consumed on return. Even that special case was broken, as it wouldn't apply if a move constructor happened to be marked with return_typestate.
  • Constructors are still special-cased when determining the default typestate for the newly constructed object, unless the constructor is marked return_typestate.

• Stop ignoring return_typestate on rvalue reference parameters.

• When a function takes a consumable object by value, treat that as consuming by default, same as with an rvalue reference.
  
  Note that the object being consumed is always a temporary. If you write:
  
  void a(X); a((X &&)x);
  
  ...this first constructs a temporary X by moving from x, then calls a with a pointer to the temporary. Before and after this change, the move copies x's typestate to the temporary and marks x itself as consumed. But before this change, if the temporary started out unconsumed (because x was unconsumed before the move), it would still be unconsumed when its destructor was run after the call. Now it will be considered consumed at that point.
  
  Note that currently, only parameters explicitly marked return_typestate have their state-on-return checked on the callee side. Parameters which are implicitly consuming due to being rvalue references – or, after this patch, by-value – are checked only on the caller side. This discrepancy was very surprising to me as a user. I wrote a fix, but in my codebase it broke some code that was using std::forward. Therefore, I plan to hold off on submitting the fix until a followup patch, which will generalize the current std::move special case into an attribute that can be applied to any 'cast' function like std::move and std::forward.

• Diagnose if a constructor or a static method is marked set_typestate, instead of (respectively) ignoring it or crashing.

• Diagnose if a param_typestate or return_typestate attribute is attached to a parameter of non-consumable type. The wording I came up with is a bit fuzzy; suggestions welcome.

• Fix broken check that was trying to dyn_cast a CXXOperatorCallExpr to a sibling class, CXXMemberCallExpr (that will never work). Instead, we need to check whether the declaration is a CXXMethodDecl.
  • This was harmless in the existing code, for somewhat complicated reasons, but caused problems after I refactored handleCall to take the arguments as a separate parameter.

Some of these changes have the potential to 'break' code (in the sense of creating warnings where there were none before) because they check things that were previously ignored.

One notable case, which showed up in the existing test code (warn-consumed-analysis.cpp), is a copy constructor that takes its argument by non-const reference. Previously, the argument would be ignored and thus implicitly treated as leaving the state alone – just like all other constructor arguments, except for the argument to a move constructor. Now it's given the usual treatment for non-const reference parameters, which is to mark the original object as being in unknown state, unless overridden with return_typestate. I think the new behavior is more correct, and this isn't removing a special case *per se*. However, since it is a "copy" constructor, there might be some argument that the argument should be treated as non-consuming instead.

Also, if anyone wants to test this on their code, note that I have a separate patch pending to make the CFG more accurate with respect to temporary object destructors (D66404).

[dblaikie]

Likely best to separate these changes into separate/standalone patches - easier to review/see what's changing, what the motivation is, etc.

(I'm probably OK with some breakage here - to the best of my knowledge these attributes haven't achieved widespread adoption, so if it looks like more suitable semantics but breaks existing uses a little, that's probably fine)

[comex]

Sounds good; I'll split this into a few separate patches.

include/clang/Analysis/Analyses/Consumed.h

public:
 
     // FIXME: This can be removed when the attr propagation fix for templated
     //        classes lands.
     /// 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 warnParamAttrForUnconsumableType(SourceLocation Loc,
+                                                  StringRef TypeName) {}
+
+    // FIXME: This can be removed when the attr propagation fix for templated
+    //        classes lands.
+    /// 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) {}
 
     /// Warn about return typestate mismatches.
     ///
     /// \param Loc -- The SourceLocation of the return statement.
     ///
     /// \param ExpectedState -- The state the return value was expected to be

include/clang/Basic/DiagnosticSemaKinds.td

def warn_use_in_invalid_state : Warning<
   "invalid invocation of method '%0' on object '%1' while it is in the '%2' "
   "state">, InGroup<Consumed>, DefaultIgnore;
 def warn_use_of_temp_in_invalid_state : Warning<
   "invalid invocation of method '%0' on a temporary object while it is in the "
   "'%1' state">, InGroup<Consumed>, DefaultIgnore;
 def warn_attr_on_unconsumable_class : Warning<
   "consumed analysis attribute is attached to member of class '%0' which isn't "
   "marked as consumable">, InGroup<Consumed>, DefaultIgnore;
+def warn_consumable_attr_on_static : Warning<
+  "consumed analysis attribute is attached to a "
+  "%select{static method|constructor}0 of class '%1'">,
+  InGroup<Consumed>, DefaultIgnore;
+def warn_param_attr_for_unconsumable_type : Warning<
+  "consumed analysis attribute is attached to a parameter of type '%0': "
+  "expected a type marked as consumable passed by value, reference, or "
+  "rvalue reference">,
+  InGroup<Consumed>, DefaultIgnore;
 def warn_return_typestate_for_unconsumable_type : Warning<
   "return state set for an unconsumable type '%0'">, InGroup<Consumed>,
   DefaultIgnore;
 def warn_return_typestate_mismatch : Warning<
   "return value not in expected state; expected '%0', observed '%1'">,
   InGroup<Consumed>, DefaultIgnore;
 def warn_loop_state_mismatch : Warning<
   "state of variable '%0' must match at the entry and exit of loop">,

lib/Analysis/Consumed.cpp

 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
 #include <memory>
 #include <utility>
 
-// TODO: Adjust states of args to constructors in the same way that arguments to
-//       function calls are handled.
 // TODO: Use information from tests in for- and while-loop conditional.
 // TODO: Add notes about the actual and expected state for
 // TODO: Correctly identify unreachable blocks when chaining boolean operators.
 // TODO: Adjust the parser and AttributesList class to support lists of
 //       identifiers.
 // TODO: Warn about unreachable code.
 // TODO: Switch to using a bitmap to track unreachable blocks.
 // TODO: Handle variable definitions, e.g. bool valid = x.isValid();
class ConsumedStmtVisitor : public ConstStmtVisitor<ConsumedStmtVisitor> {
   ConsumedState getInfo(const Expr *From);
   void setInfo(const Expr *To, ConsumedState NS);
   void propagateReturnType(const Expr *Call, const FunctionDecl *Fun);
 
 public:
   void checkCallability(const PropagationInfo &PInfo,
                         const FunctionDecl *FunDecl,
                         SourceLocation BlameLoc);
-  bool handleCall(const CallExpr *Call, const Expr *ObjArg,
-                  const FunctionDecl *FunD);
+  bool handleCall(const Expr *Call, const Expr *ObjArg,
+                  ArrayRef<const Expr *> Args, const FunctionDecl *FunD);
 
   void VisitBinaryOperator(const BinaryOperator *BinOp);
   void VisitCallExpr(const CallExpr *Call);
   void VisitCastExpr(const CastExpr *Cast);
   void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *Temp);
   void VisitCXXConstructExpr(const CXXConstructExpr *Call);
   void VisitCXXMemberCallExpr(const CXXMemberCallExpr *Call);
   void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *Call);
if (PInfo.isVar()) {
     Analyzer.WarningsHandler.warnUseOfTempInInvalidState(
       FunDecl->getNameAsString(), stateToString(TmpState), BlameLoc);
   }
 }
 
 // Factors out common behavior for function, method, and operator calls.
 // Check parameters and set parameter state if necessary.
 // Returns true if the state of ObjArg is set, or false otherwise.
-bool ConsumedStmtVisitor::handleCall(const CallExpr *Call, const Expr *ObjArg,
+bool ConsumedStmtVisitor::handleCall(const Expr *Call,
+                                     const Expr *ObjArg,
+                                     ArrayRef<const Expr *> Args,
                                      const FunctionDecl *FunD) {
-  unsigned Offset = 0;
-  if (isa<CXXOperatorCallExpr>(Call) && isa<CXXMethodDecl>(FunD))
-    Offset = 1;  // first argument is 'this'
-
   // check explicit parameters
-  for (unsigned Index = Offset; Index < Call->getNumArgs(); ++Index) {
+  unsigned Index = 0;
+  for (const Expr *Arg : Args) {
     // Skip variable argument lists.
-    if (Index - Offset >= FunD->getNumParams())
+    if (Index >= FunD->getNumParams())
       break;
 
-    const ParmVarDecl *Param = FunD->getParamDecl(Index - Offset);
+    const ParmVarDecl *Param = FunD->getParamDecl(Index++);
     QualType ParamType = Param->getType();
 
-    InfoEntry Entry = findInfo(Call->getArg(Index));
+    InfoEntry Entry = findInfo(Arg);
 
     if (Entry == PropagationMap.end() || Entry->second.isTest())
       continue;
     PropagationInfo PInfo = Entry->second;
 
     // Check that the parameter is in the correct state.
     if (ParamTypestateAttr *PTA = Param->getAttr<ParamTypestateAttr>()) {
       ConsumedState ParamState = PInfo.getAsState(StateMap);
       ConsumedState ExpectedState = mapParamTypestateAttrState(PTA);
 
       if (ParamState != ExpectedState)
         Analyzer.WarningsHandler.warnParamTypestateMismatch(
-          Call->getArg(Index)->getExprLoc(),
+          Arg->getExprLoc(),
           stateToString(ExpectedState), stateToString(ParamState));
     }
 
     if (!(Entry->second.isVar() || Entry->second.isTmp()))
       continue;
 
     // Adjust state on the caller side.
-    if (isRValueRef(ParamType))
-      setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
-    else if (ReturnTypestateAttr *RT = Param->getAttr<ReturnTypestateAttr>())
+    if (ReturnTypestateAttr *RT = Param->getAttr<ReturnTypestateAttr>())
       setStateForVarOrTmp(StateMap, PInfo, mapReturnTypestateAttrState(RT));
+    else if (isRValueRef(ParamType) || isConsumableType(ParamType))
+      setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed);
     else if (isPointerOrRef(ParamType) &&
              (!ParamType->getPointeeType().isConstQualified() ||
               isSetOnReadPtrType(ParamType)))
       setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Unknown);
   }
 
   if (!ObjArg)
     return false;
void ConsumedStmtVisitor::VisitCallExpr(const CallExpr *Call) {
 
   // Special case for the std::move function.
   // TODO: Make this more specific. (Deferred)
   if (Call->isCallToStdMove()) {
     copyInfo(Call->getArg(0), Call, CS_Consumed);
     return;
   }
 
-  handleCall(Call, nullptr, FunDecl);
+  handleCall(Call, nullptr,
+             llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()),
+             FunDecl);
   propagateReturnType(Call, FunDecl);
 }
 
 void ConsumedStmtVisitor::VisitCastExpr(const CastExpr *Cast) {
   forwardInfo(Cast->getSubExpr(), Cast);
 }
 
 void ConsumedStmtVisitor::VisitCXXBindTemporaryExpr(
   const CXXBindTemporaryExpr *Temp) {
 
   InfoEntry Entry = findInfo(Temp->getSubExpr());
 
   if (Entry != PropagationMap.end() && !Entry->second.isTest()) {
     StateMap->setState(Temp, Entry->second.getAsState(StateMap));
     PropagationMap.insert(PairType(Temp, PropagationInfo(Temp)));
   }
 }
 
 void ConsumedStmtVisitor::VisitCXXConstructExpr(const CXXConstructExpr *Call) {
   CXXConstructorDecl *Constructor = Call->getConstructor();
 
   QualType ThisType = Constructor->getThisType()->getPointeeType();
 
-  if (!isConsumableType(ThisType))
-    return;
-
-  // FIXME: What should happen if someone annotates the move constructor?
-  if (ReturnTypestateAttr *RTA = Constructor->getAttr<ReturnTypestateAttr>()) {
-    // TODO: Adjust state of args appropriately.
+  if (isConsumableType(ThisType)) {
+    if (auto *RTA = Constructor->getAttr<ReturnTypestateAttr>()) {
       ConsumedState RetState = mapReturnTypestateAttrState(RTA);
       PropagationMap.insert(PairType(Call, PropagationInfo(RetState)));
     } else if (Constructor->isDefaultConstructor()) {
       PropagationMap.insert(PairType(Call,
         PropagationInfo(consumed::CS_Consumed)));
-  } else if (Constructor->isMoveConstructor()) {
-    copyInfo(Call->getArg(0), Call, CS_Consumed);
-  } else if (Constructor->isCopyConstructor()) {
-    // Copy state from arg.  If setStateOnRead then set arg to CS_Unknown.
-    ConsumedState NS =
-      isSetOnReadPtrType(Constructor->getThisType()) ?
-      CS_Unknown : CS_None;
-    copyInfo(Call->getArg(0), Call, NS);
+    } else if (Constructor->isMoveConstructor() ||
+               Constructor->isCopyConstructor()) {
+      copyInfo(Call->getArg(0), Call, CS_None);
     } else {
-    // TODO: Adjust state of args appropriately.
       ConsumedState RetState = mapConsumableAttrState(ThisType);
       PropagationMap.insert(PairType(Call, PropagationInfo(RetState)));
     }
   }
 
+  handleCall(Call, nullptr,
+             llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()),
+             Constructor);
+}
+
 void ConsumedStmtVisitor::VisitCXXMemberCallExpr(
     const CXXMemberCallExpr *Call) {
   CXXMethodDecl* MD = Call->getMethodDecl();
   if (!MD)
     return;
 
-  handleCall(Call, Call->getImplicitObjectArgument(), MD);
+  handleCall(Call, Call->getImplicitObjectArgument(),
+    llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()), MD);
   propagateReturnType(Call, MD);
 }
 
 void ConsumedStmtVisitor::VisitCXXOperatorCallExpr(
     const CXXOperatorCallExpr *Call) {
   const auto *FunDecl = dyn_cast_or_null<FunctionDecl>(Call->getDirectCallee());
   if (!FunDecl) return;
+  ArrayRef<const Expr *> Args(Call->getArgs(), Call->getNumArgs());
 
   if (Call->getOperator() == OO_Equal) {
-    ConsumedState CS = getInfo(Call->getArg(1));
-    if (!handleCall(Call, Call->getArg(0), FunDecl))
-      setInfo(Call->getArg(0), CS);
+    ConsumedState CS = getInfo(Args[1]);
+    if (!handleCall(Call, Args[0], Args.drop_front(1), FunDecl))
+      setInfo(Args[0], CS);
     return;
   }
 
-  if (const auto *MCall = dyn_cast<CXXMemberCallExpr>(Call))
-    handleCall(MCall, MCall->getImplicitObjectArgument(), FunDecl);
+  if (isa<CXXMethodDecl>(FunDecl))
+    handleCall(Call, Args[0], Args.drop_front(1), FunDecl);
   else
-    handleCall(Call, Call->getArg(0), FunDecl);
+    handleCall(Call, nullptr, Args, FunDecl);
 
   propagateReturnType(Call, FunDecl);
 }
 
 void ConsumedStmtVisitor::VisitDeclRefExpr(const DeclRefExpr *DeclRef) {
   if (const auto *Var = dyn_cast_or_null<VarDecl>(DeclRef->getDecl()))
     if (StateMap->getState(Var) != consumed::CS_None)
       PropagationMap.insert(PairType(DeclRef, PropagationInfo(Var)));

 void ConsumedStmtVisitor::VisitMemberExpr(const MemberExpr *MExpr) {
   forwardInfo(MExpr->getBase(), MExpr);
 }
 
 void ConsumedStmtVisitor::VisitParmVarDecl(const ParmVarDecl *Param) {
   QualType ParamType = Param->getType();
   ConsumedState ParamState = consumed::CS_None;
 
-  if (const ParamTypestateAttr *PTA = Param->getAttr<ParamTypestateAttr>())
-    ParamState = mapParamTypestateAttrState(PTA);
-  else if (isConsumableType(ParamType))
+  if (isConsumableType(ParamType))
     ParamState = mapConsumableAttrState(ParamType);
   else if (isRValueRef(ParamType) &&
            isConsumableType(ParamType->getPointeeType()))
     ParamState = mapConsumableAttrState(ParamType->getPointeeType());
   else if (ParamType->isReferenceType() &&
            isConsumableType(ParamType->getPointeeType()))
     ParamState = consumed::CS_Unknown;
 
+  if (const ParamTypestateAttr *PTA = Param->getAttr<ParamTypestateAttr>()) {
+    if (ParamState == CS_None) {
+      // 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.
+      Analyzer.WarningsHandler.warnParamAttrForUnconsumableType(
+          PTA->getLocation(), ParamType.getAsString());
+    }
+    ParamState = mapParamTypestateAttrState(PTA);
+  } else if (auto *RTA = Param->getAttr<ReturnTypestateAttr>()) {
+    if (ParamState == CS_None) {
+      Analyzer.WarningsHandler.warnParamAttrForUnconsumableType(
+          RTA->getLocation(), ParamType.getAsString());
+    }
+  }
+
   if (ParamState != CS_None)
     StateMap->setState(Param, ParamState);
 }
 
 void ConsumedStmtVisitor::VisitReturnStmt(const ReturnStmt *Ret) {
   ConsumedState ExpectedState = Analyzer.getExpectedReturnState();
 
   if (ExpectedState != CS_None) {

lib/Sema/AnalysisBasedWarnings.cpp

void warnParamTypestateMismatch(SourceLocation Loc, StringRef ExpectedState,
                                   StringRef ObservedState) override {
 
     PartialDiagnosticAt Warning(Loc, S.PDiag(
       diag::warn_param_typestate_mismatch) << ExpectedState << ObservedState);
 
     Warnings.emplace_back(std::move(Warning), OptionalNotes());
   }
 
+  void warnParamAttrForUnconsumableType(SourceLocation Loc,
+                                        StringRef TypeName) override {
+    PartialDiagnosticAt Warning(Loc, S.PDiag(
+      diag::warn_param_attr_for_unconsumable_type) << TypeName);
+
+    Warnings.emplace_back(std::move(Warning), OptionalNotes());
+  }
+
   void warnReturnTypestateForUnconsumableType(SourceLocation Loc,
                                               StringRef TypeName) override {
     PartialDiagnosticAt Warning(Loc, S.PDiag(
       diag::warn_return_typestate_for_unconsumable_type) << TypeName);
 
     Warnings.emplace_back(std::move(Warning), OptionalNotes());
   }
 

lib/Sema/SemaDeclAttr.cpp

static void handleConsumableAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   D->addAttr(::new (S.Context)
              ConsumableAttr(AL.getRange(), S.Context, DefaultState,
                             AL.getAttributeSpellingListIndex()));
 }
 
 static bool checkForConsumableClass(Sema &S, const CXXMethodDecl *MD,
                                     const ParsedAttr &AL) {
-  QualType ThisType = MD->getThisType()->getPointeeType();
+  const CXXRecordDecl *RD = MD->getParent();
 
-  if (const CXXRecordDecl *RD = ThisType->getAsCXXRecordDecl()) {
-    if (!RD->hasAttr<ConsumableAttr>()) {
-      S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class) <<
-        RD->getNameAsString();
 
-      return false;
-    }
+
+  if (MD->isStatic() || isa<CXXConstructorDecl>(MD)) {
+      S.Diag(AL.getLoc(), diag::warn_consumable_attr_on_static) <<
+        (MD->isStatic() ? 0 : 1) <<
+        RD->getNameAsString();
+    return false;
   }
 
   return true;
 }
 
 static void handleCallableWhenAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   if (!checkAttributeAtLeastNumArgs(S, AL, 1))
     return;

test/SemaCXX/warn-consumed-analysis.cpp

 template <typename T>
 class CONSUMABLE(unconsumed) ConsumableClass {
   T var;
   
 public:
   ConsumableClass();
   ConsumableClass(nullptr_t p) RETURN_TYPESTATE(consumed);
   ConsumableClass(T val) RETURN_TYPESTATE(unconsumed);
-  ConsumableClass(ConsumableClass<T> &other);
+  ConsumableClass(const ConsumableClass<T> &other);
   ConsumableClass(ConsumableClass<T> &&other);
   
   ConsumableClass<T>& operator=(ConsumableClass<T>  &other);
   ConsumableClass<T>& operator=(ConsumableClass<T> &&other);
   ConsumableClass<T>& operator=(nullptr_t) SET_TYPESTATE(consumed);
   
   template <typename U>
   ConsumableClass<T>& operator=(ConsumableClass<U>  &other);
public:
   void consume() SET_TYPESTATE(consumed);
   void unconsume() SET_TYPESTATE(unconsumed);
 };
 
 class CONSUMABLE(unconsumed) DestructorTester {
 public:
   DestructorTester();
   DestructorTester(int);
+  DestructorTester(nullptr_t) RETURN_TYPESTATE(unconsumed);
+  DestructorTester(DestructorTester &&);
   
   void operator*() CALLABLE_WHEN("unconsumed");
   
   ~DestructorTester() CALLABLE_WHEN("consumed");
+
+  static void byVal(DestructorTester);
+  static void byValMarkUnconsumed(DestructorTester RETURN_TYPESTATE(unconsumed));
 };
 
 void baf0(const ConsumableClass<int>  var);
 void baf1(const ConsumableClass<int> &var);
 void baf2(const ConsumableClass<int> *var);
 
 void baf3(ConsumableClass<int>   var);
 void baf4(ConsumableClass<int>  &var);
void testDestruction() {
   *D2; // expected-warning {{invalid invocation of method 'operator*' on object 'D2' while it is in the 'consumed' state}}
   
   D0.~DestructorTester(); // expected-warning {{invalid invocation of method '~DestructorTester' on object 'D0' while it is in the 'unconsumed' state}}
   
   return; // expected-warning {{invalid invocation of method '~DestructorTester' on object 'D0' while it is in the 'unconsumed' state}} \
              expected-warning {{invalid invocation of method '~DestructorTester' on object 'D1' while it is in the 'unconsumed' state}}
 }
 
+void testDestructionByVal() {
+  {
+    // both the var and the temporary are consumed:
+    DestructorTester D0(nullptr);
+    DestructorTester::byVal((DestructorTester &&)D0);
+  }
+  {
+    // the var is consumed but the temporary isn't:
+    DestructorTester D1(nullptr);
+    DestructorTester::byValMarkUnconsumed((DestructorTester &&)D1); // expected-warning {{invalid invocation of method '~DestructorTester' on a temporary object while it is in the 'unconsumed' state}}
+  }
+}
+
 void testTempValue() {
   *ConsumableClass<int>(); // expected-warning {{invalid invocation of method 'operator*' on a temporary object while it is in the 'consumed' state}}
 }
 
 void testSimpleRValueRefs() {
   ConsumableClass<int> var0;
   ConsumableClass<int> var1(42);
   
void testParamReturnTypestateCallee(bool cond, ConsumableClass<int> &Param RETURN_TYPESTATE(unconsumed)) { // expected-warning {{parameter 'Param' not in expected state when the function returns: expected 'unconsumed', observed 'consumed'}}
   if (cond) {
     Param.consume();
     return; // expected-warning {{parameter 'Param' not in expected state when the function returns: expected 'unconsumed', observed 'consumed'}}
   }
   
   Param.consume();
 }
 
+void testRvalueRefParamReturnTypestateCallee(ConsumableClass<int> &&Param RETURN_TYPESTATE(unconsumed)) {
+  Param.unconsume();
+}
+
 void testParamReturnTypestateCaller() {
   ConsumableClass<int> var;
   
   testParamReturnTypestateCallee(true, var);
+  testRvalueRefParamReturnTypestateCallee((ConsumableClass<int> &&)var);
   
   *var;
 }
 
 void testParamTypestateCallee(ConsumableClass<int>  Param0 PARAM_TYPESTATE(consumed),
                               ConsumableClass<int> &Param1 PARAM_TYPESTATE(consumed)) {
   
   *Param0; // expected-warning {{invalid invocation of method 'operator*' on object 'Param0' while it is in the 'consumed' state}}
void testCallingConventions() {
   baf0(var);  
   *var;
   
   baf1(var);  
   *var;
   
   baf2(&var);  
   *var;
-  
+  baf3(var);
+  *var;
+  
   baf4(var);  
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
   
   var = ConsumableClass<int>(42);
   baf5(&var);  
   *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}}
   
   var = ConsumableClass<int>(42);
void test() {
     x.move();
     std::move();
     std::move(x);
     std::__1::move();
     std::__1::move(x);
   }
 } // end namespace PR18260
 
+namespace NonConstCopyConstructor {
+  struct CONSUMABLE(unknown) Foo {
+    Foo() RETURN_TYPESTATE(unconsumed);
+    Foo(Foo &);
+    void unconsumedCall() const CALLABLE_WHEN(unconsumed);
+  };
+  void test() {
+    Foo foo;
+    Foo bar(foo);
+    foo.unconsumedCall(); // expected-warning {{invalid invocation of method 'unconsumedCall' on object 'foo' while it is in the 'unknown' state}}
+  }
+} // end namespace NonConstCopyConstructor
+
+namespace ParamAttributesOnConstructor {
+  struct CONSUMABLE(unknown) Foo {
+    Foo() RETURN_TYPESTATE(consumed);
+    Foo(PARAM_TYPESTATE(consumed) RETURN_TYPESTATE(unconsumed) Foo &);
+    void unconsumedCall() const CALLABLE_WHEN(unconsumed);
+  };
+  void test() {
+    Foo foo;
+    Foo bar(foo);
+    foo.unconsumedCall();
+    Foo baz(foo); // expected-warning {{argument not in expected state; expected 'consumed', observed 'unconsumed'}}
+  }
+} // end namespace NonConstCopyConstructor
+
+namespace CustomConstructors {
+  struct Foo {
+    Foo(int, ConsumableClass<int> &&c);
+    Foo(int *, RETURN_TYPESTATE(unconsumed) ConsumableClass<int> &&c);
+  };
+
+  void test() {
+    ConsumableClass<int> i1(nullptr);
+    Foo j1(42, (ConsumableClass<int> &&)i1);
+    *i1; // expected-warning {{invalid invocation of method 'operator*' on object 'i1' while it is in the 'consumed' state}}
+    ConsumableClass<int> i2(nullptr);
+    Foo j2(nullptr, (ConsumableClass<int> &&)i2);
+    *i2;
+  }
+}
+
+// This needs to be here instead of warn-consumed-parsing.cpp because these
+// warnings come from analysis.
+struct CONSUMABLE(unknown) MoreUselessAttrs {
+  void a([[clang::param_typestate(consumed)]] int) {} // expected-warning {{consumed analysis attribute is attached to a parameter of type 'int': expected a type marked as consumable passed by value, reference, or rvalue reference}}
+  void b([[clang::return_typestate(consumed)]] MoreUselessAttrs *) {} // expected-warning {{consumed analysis attribute is attached to a parameter of type 'struct MoreUselessAttrs *': expected a type marked as consumable passed by value, reference, or rvalue reference}}
+};

test/SemaCXX/warn-consumed-parsing.cpp

 
 
 class CONSUMABLE(unconsumed)
       __attribute__((consumable_auto_cast_state))
       __attribute__((consumable_set_state_on_read))
       Status {
 };
 
-
+struct CONSUMABLE(unknown) UselessAttrs {
+  static void x() SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a static method of class 'UselessAttrs'}}
+  UselessAttrs() SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a constructor of class 'UselessAttrs'}}
+  UselessAttrs(int) CALLABLE_WHEN(consumed); // expected-warning {{consumed analysis attribute is attached to a constructor of class 'UselessAttrs'}}
+  void operator+(UselessAttrs) SET_TYPESTATE(consumed); // OK
+  static void *operator new(unsigned long) SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a static method of class 'UselessAttrs'}}
+};
+void operator-(UselessAttrs, UselessAttrs) SET_TYPESTATE(consumed); // expected-warning {{'set_typestate' attribute only applies to functions}}