Consumed Analysis: Change callable_when so that it 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.
Patch by chriswails@gmail.com.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@191983 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 134 insertions, 107 deletions

diff --git a/lib/Analysis/Consumed.cpp b/lib/Analysis/Consumed.cpp
index 7cd029020a..ee8dd77ff6 100644
--- a/lib/Analysis/Consumed.cpp
+++ b/lib/Analysis/Consumed.cpp
@@ -33,6 +33,8 @@
 
 // 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: Mark variables as Unknown going into while- or for-loops only if they
@@ -66,6 +68,37 @@ static ConsumedState invertConsumedUnconsumed(ConsumedState State) {
   llvm_unreachable("invalid enum");
 }
 
+static bool isCallableInState(const CallableWhenAttr *CWAttr,
+                              ConsumedState State) {
+  
+  CallableWhenAttr::callableState_iterator I = CWAttr->callableState_begin(),
+                                           E = CWAttr->callableState_end();
+  
+  for (; I != E; ++I) {
+    
+    ConsumedState MappedAttrState = CS_None;
+    
+    switch (*I) {
+    case CallableWhenAttr::Unknown:
+      MappedAttrState = CS_Unknown;
+      break;
+      
+    case CallableWhenAttr::Unconsumed:
+      MappedAttrState = CS_Unconsumed;
+      break;
+      
+    case CallableWhenAttr::Consumed:
+      MappedAttrState = CS_Consumed;
+      break;
+    }
+    
+    if (MappedAttrState == State)
+      return true;
+  }
+  
+  return false;
+}
+
 static bool isConsumableType(const QualType &QT) {
   if (const CXXRecordDecl *RD = QT->getAsCXXRecordDecl())
     return RD->hasAttr<ConsumableAttr>();
@@ -174,6 +207,8 @@ class PropagationInfo {
     BinTestTy BinTest;
   };
   
+  QualType TempType;
+  
 public:
   PropagationInfo() : InfoType(IT_None) {}
   
@@ -208,7 +243,9 @@ public:
     BinTest.RTest.TestsFor = RTestsFor;
   }
   
-  PropagationInfo(ConsumedState State) : InfoType(IT_State), State(State) {}
+  PropagationInfo(ConsumedState State, QualType TempType)
+    : InfoType(IT_State), State(State), TempType(TempType) {}
+  
   PropagationInfo(const VarDecl *Var) : InfoType(IT_Var), Var(Var) {}
   
   const ConsumedState & getState() const {
@@ -216,6 +253,11 @@ public:
     return State;
   }
   
+  const QualType & getTempType() const {
+    assert(InfoType == IT_State);
+    return TempType;
+  }
+  
   const VarTestResult & getTest() const {
     assert(InfoType == IT_Test);
     return Test;
@@ -327,51 +369,38 @@ public:
   }
 };
 
-// TODO: When we support CallableWhenConsumed this will have to check for
-//       the different attributes and change the behavior bellow. (Deferred)
 void ConsumedStmtVisitor::checkCallability(const PropagationInfo &PInfo,
                                            const FunctionDecl *FunDecl,
                                            const CallExpr *Call) {
   
-  if (!FunDecl->hasAttr<CallableWhenUnconsumedAttr>()) return;
+  if (!FunDecl->hasAttr<CallableWhenAttr>())
+    return;
+  
+  const CallableWhenAttr *CWAttr = FunDecl->getAttr<CallableWhenAttr>();
   
   if (PInfo.isVar()) {
     const VarDecl *Var = PInfo.getVar();
+    ConsumedState VarState = StateMap->getState(Var);
     
-    switch (StateMap->getState(Var)) {
-    case CS_Consumed:
-      Analyzer.WarningsHandler.warnUseWhileConsumed(
-        FunDecl->getNameAsString(), Var->getNameAsString(),
-        Call->getExprLoc());
-      break;
+    assert(VarState != CS_None && "Invalid state");
     
-    case CS_Unknown:
-      Analyzer.WarningsHandler.warnUseInUnknownState(
-        FunDecl->getNameAsString(), Var->getNameAsString(),
-        Call->getExprLoc());
-      break;
-      
-    case CS_None:
-    case CS_Unconsumed:
-      break;
-    }
+    if (isCallableInState(CWAttr, VarState))
+      return;
     
-  } else {
-    switch (PInfo.getState()) {
-    case CS_Consumed:
-      Analyzer.WarningsHandler.warnUseOfTempWhileConsumed(
-        FunDecl->getNameAsString(), Call->getExprLoc());
-      break;
+    Analyzer.WarningsHandler.warnUseInInvalidState(
+      FunDecl->getNameAsString(), Var->getNameAsString(),
+      stateToString(VarState), Call->getExprLoc());
     
-    case CS_Unknown:
-      Analyzer.WarningsHandler.warnUseOfTempInUnknownState(
-        FunDecl->getNameAsString(), Call->getExprLoc());
-      break;
-      
-    case CS_None:
-    case CS_Unconsumed:
-      break;
-    }
+  } else if (PInfo.isState()) {
+    
+    assert(PInfo.getState() != CS_None && "Invalid state");
+    
+    if (isCallableInState(CWAttr, PInfo.getState()))
+      return;
+    
+    Analyzer.WarningsHandler.warnUseOfTempInInvalidState(
+      FunDecl->getNameAsString(), stateToString(PInfo.getState()),
+      Call->getExprLoc());
   }
 }
 
@@ -421,7 +450,7 @@ void ConsumedStmtVisitor::propagateReturnType(const Stmt *Call,
       ReturnState = mapConsumableAttrState(ReturnType);
     
     PropagationMap.insert(PairType(Call,
-      PropagationInfo(ReturnState)));
+      PropagationInfo(ReturnState, ReturnType)));
   }
 }
 
@@ -522,7 +551,11 @@ void ConsumedStmtVisitor::VisitCallExpr(const CallExpr *Call) {
       }
     }
     
-    propagateReturnType(Call, FunDecl, FunDecl->getCallResultType());
+    QualType RetType = FunDecl->getCallResultType();
+    if (RetType->isReferenceType())
+      RetType = RetType->getPointeeType();
+    
+    propagateReturnType(Call, FunDecl, RetType);
   }
 }
 
@@ -540,7 +573,7 @@ void ConsumedStmtVisitor::VisitCXXConstructExpr(const CXXConstructExpr *Call) {
     if (Constructor->isDefaultConstructor()) {
       
       PropagationMap.insert(PairType(Call,
-        PropagationInfo(consumed::CS_Consumed)));
+        PropagationInfo(consumed::CS_Consumed, ThisType)));
       
     } else if (Constructor->isMoveConstructor()) {
       
@@ -551,7 +584,7 @@ void ConsumedStmtVisitor::VisitCXXConstructExpr(const CXXConstructExpr *Call) {
         const VarDecl* Var = PInfo.getVar();
         
         PropagationMap.insert(PairType(Call,
-          PropagationInfo(StateMap->getState(Var))));
+          PropagationInfo(StateMap->getState(Var), ThisType)));
         
         StateMap->setState(Var, consumed::CS_Consumed);
         
@@ -630,7 +663,8 @@ void ConsumedStmtVisitor::VisitCXXOperatorCallExpr(
         
       } else if (!LPInfo.isVar() && RPInfo.isVar()) {
         PropagationMap.insert(PairType(Call,
-          PropagationInfo(StateMap->getState(RPInfo.getVar()))));
+          PropagationInfo(StateMap->getState(RPInfo.getVar()),
+                          LPInfo.getTempType())));
         
         StateMap->setState(RPInfo.getVar(), consumed::CS_Consumed);
         
@@ -648,27 +682,16 @@ void ConsumedStmtVisitor::VisitCXXOperatorCallExpr(
         
         PropagationMap.insert(PairType(Call, LPInfo));
         
-      } else {
+      } else if (LPInfo.isState()) {
         PropagationMap.insert(PairType(Call,
-          PropagationInfo(consumed::CS_Unknown)));
+          PropagationInfo(consumed::CS_Unknown, LPInfo.getTempType())));
       }
       
     } else if (LEntry == PropagationMap.end() &&
                REntry != PropagationMap.end()) {
       
-      RPInfo = REntry->second;
-      
-      if (RPInfo.isVar()) {
-        const VarDecl *Var = RPInfo.getVar();
-        
-        PropagationMap.insert(PairType(Call,
-          PropagationInfo(StateMap->getState(Var))));
-        
-        StateMap->setState(Var, consumed::CS_Consumed);
-        
-      } else {
-        PropagationMap.insert(PairType(Call, RPInfo));
-      }
+      if (REntry->second.isVar())
+        StateMap->setState(REntry->second.getVar(), consumed::CS_Consumed);
     }
     
   } else {
@@ -776,6 +799,7 @@ void ConsumedStmtVisitor::VisitUnaryOperator(const UnaryOperator *UOp) {
   }
 }
 
+// TODO: See if I need to check for reference types here.
 void ConsumedStmtVisitor::VisitVarDecl(const VarDecl *Var) {
   if (isConsumableType(Var->getType())) {
     if (Var->hasInit()) {
@@ -803,13 +827,12 @@ void splitVarStateForIf(const IfStmt * IfNode, const VarTestResult &Test,
   
   if (VarState == CS_Unknown) {
     ThenStates->setState(Test.Var, Test.TestsFor);
-    if (ElseStates)
-      ElseStates->setState(Test.Var, invertConsumedUnconsumed(Test.TestsFor));
+    ElseStates->setState(Test.Var, invertConsumedUnconsumed(Test.TestsFor));
   
   } else if (VarState == invertConsumedUnconsumed(Test.TestsFor)) {
     ThenStates->markUnreachable();
     
-  } else if (VarState == Test.TestsFor && ElseStates) {
+  } else if (VarState == Test.TestsFor) {
     ElseStates->markUnreachable();
   }
 }
@@ -832,31 +855,27 @@ void splitVarStateForIfBinOp(const PropagationInfo &PInfo,
         ThenStates->markUnreachable();
         
       } else if (LState == LTest.TestsFor && isKnownState(RState)) {
-        if (RState == RTest.TestsFor) {
-          if (ElseStates)
-            ElseStates->markUnreachable();
-        } else {
+        if (RState == RTest.TestsFor)
+          ElseStates->markUnreachable();
+        else
           ThenStates->markUnreachable();
-        }
       }
       
     } else {
-      if (LState == CS_Unknown && ElseStates) {
+      if (LState == CS_Unknown) {
         ElseStates->setState(LTest.Var,
                              invertConsumedUnconsumed(LTest.TestsFor));
       
-      } else if (LState == LTest.TestsFor && ElseStates) {
+      } else if (LState == LTest.TestsFor) {
         ElseStates->markUnreachable();
         
       } else if (LState == invertConsumedUnconsumed(LTest.TestsFor) &&
                  isKnownState(RState)) {
         
-        if (RState == RTest.TestsFor) {
-          if (ElseStates)
-            ElseStates->markUnreachable();
-        } else {
+        if (RState == RTest.TestsFor)
+          ElseStates->markUnreachable();
+        else
           ThenStates->markUnreachable();
-        }
       }
     }
   }
@@ -868,7 +887,7 @@ void splitVarStateForIfBinOp(const PropagationInfo &PInfo,
       else if (RState == invertConsumedUnconsumed(RTest.TestsFor))
         ThenStates->markUnreachable();
       
-    } else if (ElseStates) {
+    } else {
       if (RState == CS_Unknown)
         ElseStates->setState(RTest.Var,
                              invertConsumedUnconsumed(RTest.TestsFor));
@@ -1016,7 +1035,6 @@ bool ConsumedAnalyzer::splitState(const CFGBlock *CurrBlock,
   if (const IfStmt *IfNode =
     dyn_cast_or_null<IfStmt>(CurrBlock->getTerminator().getStmt())) {
     
-    bool HasElse = IfNode->getElse() != NULL;
     const Stmt *Cond = IfNode->getCond();
     
     PInfo = Visitor.getInfo(Cond);
@@ -1026,15 +1044,12 @@ bool ConsumedAnalyzer::splitState(const CFGBlock *CurrBlock,
     if (PInfo.isTest()) {
       CurrStates->setSource(Cond);
       FalseStates->setSource(Cond);
-      
-      splitVarStateForIf(IfNode, PInfo.getTest(), CurrStates,
-                         HasElse ? FalseStates : NULL);
+      splitVarStateForIf(IfNode, PInfo.getTest(), CurrStates, FalseStates);
       
     } else if (PInfo.isBinTest()) {
       CurrStates->setSource(PInfo.testSourceNode());
       FalseStates->setSource(PInfo.testSourceNode());
-      
-      splitVarStateForIfBinOp(PInfo, CurrStates, HasElse ? FalseStates : NULL);
+      splitVarStateForIfBinOp(PInfo, CurrStates, FalseStates);
       
     } else {
       delete FalseStates;
diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index cc1cb0b342..a206acd7f2 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1503,36 +1503,20 @@ public:
     Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
   }
   
-  void warnUseOfTempWhileConsumed(StringRef MethodName, SourceLocation Loc) {
+  void warnUseOfTempInInvalidState(StringRef MethodName, StringRef State,
+                                   SourceLocation Loc) {
                                                     
     PartialDiagnosticAt Warning(Loc, S.PDiag(
-      diag::warn_use_of_temp_while_consumed) << MethodName);
+      diag::warn_use_of_temp_in_invalid_state) << MethodName << State);
     
     Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
   }
   
-  void warnUseOfTempInUnknownState(StringRef MethodName, SourceLocation Loc) {
+  void warnUseInInvalidState(StringRef MethodName, StringRef VariableName,
+                                  StringRef State, SourceLocation Loc) {
   
-    PartialDiagnosticAt Warning(Loc, S.PDiag(
-      diag::warn_use_of_temp_in_unknown_state) << MethodName);
-    
-    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
-  }
-  
-  void warnUseWhileConsumed(StringRef MethodName, StringRef VariableName,
-                            SourceLocation Loc) {
-  
-    PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_use_while_consumed) <<
-                                MethodName << VariableName);
-    
-    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
-  }
-  
-  void warnUseInUnknownState(StringRef MethodName, StringRef VariableName,
-                             SourceLocation Loc) {
-
-    PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_use_in_unknown_state) <<
-                                MethodName << VariableName);
+    PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_use_in_invalid_state) <<
+                                MethodName << VariableName << State);
     
     Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
   }
@@ -1570,7 +1554,7 @@ clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
     (D.getDiagnosticLevel(diag::warn_double_lock, SourceLocation()) !=
      DiagnosticsEngine::Ignored);
   DefaultPolicy.enableConsumedAnalysis = (unsigned)
-    (D.getDiagnosticLevel(diag::warn_use_while_consumed, SourceLocation()) !=
+    (D.getDiagnosticLevel(diag::warn_use_in_invalid_state, SourceLocation()) !=
      DiagnosticsEngine::Ignored);
 }
 
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index c6337a5015..41cc3fd285 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -1041,8 +1041,11 @@ static void handleConsumesAttr(Sema &S, Decl *D, const AttributeList &Attr) {
                           Attr.getAttributeSpellingListIndex()));
 }
 
-static void handleCallableWhenUnconsumedAttr(Sema &S, Decl *D,
-                                             const AttributeList &Attr) {
+static void handleCallableWhenAttr(Sema &S, Decl *D,
+                                   const AttributeList &Attr) {
+  
+  if (!checkAttributeAtLeastNumArgs(S, Attr, 1)) return;
+
   if (!isa<CXXMethodDecl>(D)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
       Attr.getName() << ExpectedMethod;
@@ -1052,9 +1055,34 @@ static void handleCallableWhenUnconsumedAttr(Sema &S, Decl *D,
   if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
     return;
   
+  SmallVector<CallableWhenAttr::ConsumedState, 3> States;
+  for (unsigned ArgIndex = 0; ArgIndex < Attr.getNumArgs(); ++ArgIndex) {
+    CallableWhenAttr::ConsumedState CallableState;
+    
+    if (Attr.isArgExpr(ArgIndex) &&
+        isa<StringLiteral>(Attr.getArgAsExpr(ArgIndex))) {
+      
+      Expr *Arg = Attr.getArgAsExpr(ArgIndex);
+      StringRef StateString = cast<StringLiteral>(Arg)->getString();
+      
+      if (!CallableWhenAttr::ConvertStrToConsumedState(StateString,
+                                                       CallableState)) {
+        S.Diag(Arg->getExprLoc(), diag::warn_attribute_type_not_supported)
+          << Attr.getName() << StateString;
+        return;
+      }
+      
+      States.push_back(CallableState);
+    } else {
+      S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) << Attr.getName()
+        << AANT_ArgumentString;
+      return;
+    }
+  }
+  
   D->addAttr(::new (S.Context)
-             CallableWhenUnconsumedAttr(Attr.getRange(), S.Context,
-                                        Attr.getAttributeSpellingListIndex()));
+             CallableWhenAttr(Attr.getRange(), S.Context, States.data(),
+               States.size(), Attr.getAttributeSpellingListIndex()));
 }
 
 static void handleTestsConsumedAttr(Sema &S, Decl *D,
@@ -4767,8 +4795,8 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case AttributeList::AT_Consumes:
     handleConsumesAttr(S, D, Attr);
     break;
-  case AttributeList::AT_CallableWhenUnconsumed:
-    handleCallableWhenUnconsumedAttr(S, D, Attr);
+  case AttributeList::AT_CallableWhen:
+    handleCallableWhenAttr(S, D, Attr);
     break;
   case AttributeList::AT_TestsConsumed:
     handleTestsConsumedAttr(S, D, Attr);