[PATCH] Fixes and features for Consumed analysis

Christian Wailes chriswailes at google.com
Tue Aug 20 16:16:10 PDT 2013


Responses are bellow.  Thanks for the review.

- Chris


On Mon, Aug 19, 2013 at 12:28 PM, Aaron Ballman <aaron at aaronballman.com>wrote:

> Observations interspersed below.
>
> On Mon, Aug 19, 2013 at 2:57 PM, Christian Wailes
> <chriswailes at google.com> wrote:
> > chriswailes added you to the CC list for the revision "Fixes and
> features for Consumed analysis".
> >
> > Hi delesley, dblaikie,
> >
> > This patch provides fixes to the code based on feedback, as well as a
> couple of new features:
> >
> > * The same functionality is now supported for both CXXOperatorCallExprs
> and CXXMemberCallExprs.
> > * Factored out some code in the StmtVisitor.
> > * Removed unnecessary asserts from SemaDeclAttr.cpp
> > * Remove variables from the state map when their destructors are
> encountered.
> > * Re-use an existing warning for misplaced attributes.
> > * Started adding documentation for the consumed analysis attributes.
> >
> > http://llvm-reviews.chandlerc.com/D1384
> >
> > Files:
> >   docs/LanguageExtensions.rst
> >   include/clang/Analysis/Analyses/Consumed.h
> >   include/clang/Basic/Attr.td
> >   include/clang/Basic/DiagnosticSemaKinds.td
> >   lib/Analysis/Consumed.cpp
> >   lib/Sema/AnalysisBasedWarnings.cpp
> >   lib/Sema/SemaDeclAttr.cpp
> >   test/SemaCXX/warn-consumed-analysis.cpp
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> > Index: docs/LanguageExtensions.rst
> > ===================================================================
> > --- docs/LanguageExtensions.rst
> > +++ docs/LanguageExtensions.rst
> > @@ -2022,6 +2022,33 @@
> >  locks.  Arguments must be lockable type, and there must be at least one
> >  argument.
> >
> > +Consumed Annotation Checking
> > +============================
> > +
> > +Clang supports additional attributes for checking basic resource
> management
> > +properties, specifically for unique objects that have a single owning
> reference.
> > +The following attributes are currently supported, although **the
> implementation
> > +for these annotations is currently in development and are subject to
> change.**
> > +
> > +``consumes``
> > +------------
> > +
> > +Use ``__attribute__((consumes))`` on a method that transitions an
> object into
> > +the consumed state.
> > +
> > +``callable_when_unconsumed``
> > +----------------------------
> > +
> > +Use ``__attribute__((callable_when_unconsumed))`` to indicate that a
> method may
> > +only be called when the object is not in the consumed state.
> > +
> > +``tests_unconsumed``
> > +--------------------
> > +
> > +Use `__attribute__((tests_unconsumed))`` to indicate that a method
> returns true
> > +if the object is in the unconsumed state.
> > +
> > +
> >  Type Safety Checking
> >  ====================
> >
> > Index: include/clang/Analysis/Analyses/Consumed.h
> > ===================================================================
> > --- include/clang/Analysis/Analyses/Consumed.h
> > +++ include/clang/Analysis/Analyses/Consumed.h
> > @@ -118,6 +118,9 @@
> >
> >      /// \brief Set the consumed state of a given variable.
> >      void setState(const VarDecl *Var, ConsumedState State);
> > +
> > +    /// \brief Remove the variable from our state map.
> > +    void remove(const VarDecl *Var);
> >    };
> >
> >    class ConsumedBlockInfo {
> > @@ -188,7 +191,7 @@
> >    };
> >
> >    /// \brief Check to see if a function tests an object's validity.
> > -  bool isTestingFunction(const CXXMethodDecl *MethodDecl);
> > +  bool isTestingFunction(const FunctionDecl *FunDecl);
> >
> >  }} // end namespace clang::consumed
> >
> > Index: include/clang/Basic/Attr.td
> > ===================================================================
> > --- include/clang/Basic/Attr.td
> > +++ include/clang/Basic/Attr.td
> > @@ -932,7 +932,7 @@
> >
> >  def Consumes : InheritableAttr {
> >    let Spellings = [GNU<"consumes">];
> > -  let Subjects = [CXXMethod];
> > +  let Subjects = [CXXMethod, CXXConstructor];
> >  }
> >
> >  def TestsConsumed : InheritableAttr {
> > Index: include/clang/Basic/DiagnosticSemaKinds.td
> > ===================================================================
> > --- include/clang/Basic/DiagnosticSemaKinds.td
> > +++ include/clang/Basic/DiagnosticSemaKinds.td
> > @@ -2189,9 +2189,6 @@
> >  def warn_use_of_temp_while_consumed : Warning<
> >    "invocation of method '%0' on a temporary object while it is in the "
> >    "'consumed' state">, InGroup<Consumed>, DefaultIgnore;
> > -def warn_uniqueness_attribute_wrong_decl_type : Warning<
> > -  "%0 attribute only applies to methods">,
> > -  InGroup<Consumed>, DefaultIgnore;
> >
> >  // ConsumedStrict warnings
> >  def warn_use_in_unknown_state : Warning<
> > Index: lib/Analysis/Consumed.cpp
> > ===================================================================
> > --- lib/Analysis/Consumed.cpp
> > +++ lib/Analysis/Consumed.cpp
> > @@ -30,7 +30,6 @@
> >  #include "llvm/ADT/SmallVector.h"
> >  #include "llvm/Support/raw_ostream.h"
> >
> > -// TODO: Add support for methods with CallableWhenUnconsumed.
> >  // TODO: Mark variables as Unknown going into while- or for-loops only
> if they
> >  //       are referenced inside that block. (Deferred)
> >  // TODO: Add a method(s) to identify which method calls perform what
> state
> > @@ -91,9 +90,9 @@
> >        StateOrVar.Var = Var;
> >      }
> >
> > -    ConsumedState getState() { return StateOrVar.State; };
> > +    ConsumedState getState() const { return StateOrVar.State; };
>
> There's a spurious semi colon after the curly brace.
>
>
Fixed


> >
> > -    const VarDecl * getVar() { return IsVar ? StateOrVar.Var : NULL; };
> > +    const VarDecl * getVar() const { return IsVar ? StateOrVar.Var :
> NULL; };
>
> Same here.
>
>
Fixed


> >    };
> >
> >    typedef llvm::DenseMap<const Stmt *, PropagationInfo> MapType;
> > @@ -105,6 +104,9 @@
> >    ConsumedStateMap *StateMap;
> >    MapType PropagationMap;
> >
> > +  void checkCallability(const PropagationInfo &PState,
> > +                        const FunctionDecl *FunDecl,
> > +                        const CallExpr *Call);
>
> Formatting issues with the alignment of the last two parameters.
>
>
I don't see this in my editor, but I'll check to make sure the next patch I
submit doesn't have it.


> >    void forwardInfo(const Stmt *From, const Stmt *To);
> >    bool isLikeMoveAssignment(const CXXMethodDecl *MethodDecl);
> >
> > @@ -128,12 +130,58 @@
> >    ConsumedStmtVisitor(AnalysisDeclContext &AC, ConsumedAnalyzer
> &Analyzer,
> >                        ConsumedStateMap *StateMap)
> >        : AC(AC), Analyzer(Analyzer), StateMap(StateMap) {}
> > -
> > +
>
> Whitespace change?
>
>
Fixed

>    void reset() {
> >      PropagationMap.clear();
> >    }
> >  };
> >
> > +// 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
> &PState,
> > +                                           const FunctionDecl *FunDecl,
> > +                                           const CallExpr *Call) {
>
> More formatting.
>
>
Again, not present in my editor, but I'll check the next patch.


> > +
> > +  if (!FunDecl->hasAttr<CallableWhenUnconsumedAttr>()) return;
> > +
> > +  if (PState.IsVar) {
> > +    const VarDecl *Var = PState.getVar();
> > +
> > +    switch (StateMap->getState(Var)) {
> > +    case CS_Consumed:
> > +      Analyzer.WarningsHandler.warnUseWhileConsumed(
> > +        FunDecl->getNameAsString(), Var->getNameAsString(),
> > +        Call->getExprLoc());
> > +      break;
> > +
> > +    case CS_Unknown:
> > +      Analyzer.WarningsHandler.warnUseInUnknownState(
> > +        FunDecl->getNameAsString(), Var->getNameAsString(),
> > +        Call->getExprLoc());
> > +      break;
> > +
> > +    default:
> > +      break;
>
> Should the default be unreachable?
>
>
No, there are other cases besides Consumed and Unknown.  I just don't want
to do anything for those cases.


> > +    }
> > +
> > +  } else {
> > +    switch (PState.getState()) {
> > +    case CS_Consumed:
> > +      Analyzer.WarningsHandler.warnUseOfTempWhileConsumed(
> > +        FunDecl->getNameAsString(), Call->getExprLoc());
> > +      break;
> > +
> > +    case CS_Unknown:
> > +      Analyzer.WarningsHandler.warnUseOfTempInUnknownState(
> > +        FunDecl->getNameAsString(), Call->getExprLoc());
> > +      break;
> > +
> > +    default:
> > +      break;
>
> Same here.
>
>
Same as above.


> > +    }
> > +  }
> > +}
> > +
> >  void ConsumedStmtVisitor::forwardInfo(const Stmt *From, const Stmt *To)
> {
> >    InfoEntry Entry = PropagationMap.find(From);
> >
> > @@ -278,14 +326,16 @@
> >
> >    if (Entry != PropagationMap.end()) {
> >      PropagationInfo PState = Entry->second;
> > -    if (!PState.IsVar) return;
> > +    const CXXMethodDecl *MethodDecl = Call->getMethodDecl();
> >
> > -    const CXXMethodDecl *Method = Call->getMethodDecl();
> > +    checkCallability(PState, MethodDecl, Call);
> >
> > -    if (Method->hasAttr<ConsumesAttr>())
> > -      StateMap->setState(PState.getVar(), consumed::CS_Consumed);
> > -    else if (!Method->isConst())
> > -      StateMap->setState(PState.getVar(), consumed::CS_Unknown);
> > +    if (PState.IsVar) {
> > +      if (MethodDecl->hasAttr<ConsumesAttr>())
> > +        StateMap->setState(PState.getVar(), consumed::CS_Consumed);
> > +      else if (!MethodDecl->isConst())
> > +        StateMap->setState(PState.getVar(), consumed::CS_Unknown);
> > +    }
> >    }
> >  }
> >
> > @@ -374,58 +424,22 @@
> >      InfoEntry Entry = PropagationMap.find(Call->getArg(0));
> >
> >      if (Entry != PropagationMap.end()) {
> > -
> >        PropagationInfo PState = Entry->second;
> >
> > -      // TODO: When we support CallableWhenConsumed this will have to
> check for
> > -      //       the different attributes and change the behavior bellow.
> > -      //       (Deferred)
> > -      if (FunDecl->hasAttr<CallableWhenUnconsumedAttr>()) {
> > -        if (PState.IsVar) {
> > -          const VarDecl *Var = PState.getVar();
> > -
> > -          switch (StateMap->getState(Var)) {
> > -          case CS_Consumed:
> > -            Analyzer.WarningsHandler.warnUseWhileConsumed(
> > -              FunDecl->getNameAsString(), Var->getNameAsString(),
> > -              Call->getExprLoc());
> > -            break;
> > -
> > -          case CS_Unknown:
> > -            Analyzer.WarningsHandler.warnUseInUnknownState(
> > -              FunDecl->getNameAsString(), Var->getNameAsString(),
> > -              Call->getExprLoc());
> > -            break;
> > -
> > -          default:
> > -            break;
> > -          }
> > -
> > -        } else {
> > -          switch (PState.getState()) {
> > -          case CS_Consumed:
> > -            Analyzer.WarningsHandler.warnUseOfTempWhileConsumed(
> > -              FunDecl->getNameAsString(), Call->getExprLoc());
> > -            break;
> > +      checkCallability(PState, FunDecl, Call);
> > +
> > +      if (PState.IsVar) {
> > +        if (FunDecl->hasAttr<ConsumesAttr>()) {
> > +          // Handle consuming operators.
> > +          StateMap->setState(PState.getVar(), consumed::CS_Consumed);
> > +        } else if (const CXXMethodDecl *MethodDecl =
> > +          dyn_cast_or_null<CXXMethodDecl>(FunDecl)) {
> >
> > -          case CS_Unknown:
> > -            Analyzer.WarningsHandler.warnUseOfTempInUnknownState(
> > -              FunDecl->getNameAsString(), Call->getExprLoc());
> > -            break;
> > -
> > -          default:
> > -            break;
> > -          }
> > +          // Handle non-constant member operators.
> > +          if (!MethodDecl->isConst())
> > +            StateMap->setState(PState.getVar(), consumed::CS_Unknown);
> >          }
> >        }
> > -
> > -      // Handle non-constant member operators.
> > -      if (const CXXMethodDecl *MethodDecl =
> > -        dyn_cast_or_null<CXXMethodDecl>(FunDecl)) {
> > -
> > -        if (!MethodDecl->isConst() && PState.IsVar)
> > -          StateMap->setState(PState.getVar(), consumed::CS_Unknown);
> > -      }
> >      }
> >    }
> >  }
> > @@ -505,10 +519,10 @@
> >  };
> >
> >  bool TestedVarsVisitor::VisitCallExpr(CallExpr *Call) {
> > -  if (const CXXMethodDecl *Method =
> > -    dyn_cast_or_null<CXXMethodDecl>(Call->getDirectCallee())) {
> > +  if (const FunctionDecl *FunDecl =
> > +    dyn_cast_or_null<FunctionDecl>(Call->getDirectCallee())) {
> >
> > -    if (isTestingFunction(Method)) {
> > +    if (isTestingFunction(FunDecl)) {
> >        CurrTestLoc = Call->getExprLoc();
> >        IsUsefulConditional = true;
> >        return true;
> > @@ -521,16 +535,11 @@
> >  }
> >
> >  bool TestedVarsVisitor::VisitDeclRefExpr(DeclRefExpr *DeclRef) {
> > -  if (const VarDecl *Var =
> dyn_cast_or_null<VarDecl>(DeclRef->getDecl())) {
> > -    if (StateMap->getState(Var) != consumed::CS_None) {
> > +  if (const VarDecl *Var =
> dyn_cast_or_null<VarDecl>(DeclRef->getDecl()))
> > +    if (StateMap->getState(Var) != consumed::CS_None)
>
> Can these if's be combined for clarity?
>
>
No, if you delcare a variable in an if-statement then you can't have any
other claues.


> >        Test = VarTestResult(Var, CurrTestLoc, !Invert);
> > -    }
> > -
> > -  } else {
> > -    IsUsefulConditional = false;
> > -  }
> >
> > -  return IsUsefulConditional;
> > +  return true;
> >  }
> >
> >  bool TestedVarsVisitor::VisitUnaryOperator(UnaryOperator *UnaryOp) {
> > @@ -637,6 +646,9 @@
> >    Map[Var] = State;
> >  }
> >
> > +void ConsumedStateMap::remove(const VarDecl *Var) {
> > +  Map.erase(Var);
> > +}
> >
> >  bool ConsumedAnalyzer::isConsumableType(QualType Type) {
> >    const CXXRecordDecl *RD =
> > @@ -734,20 +746,24 @@
> >
> >      if (CurrStates == NULL)
> >        CurrStates = BlockInfo.getInfo(CurrBlock);
> > -
> > +
>
> Whitespace only?
>
>
Fixed


> >      ConsumedStmtVisitor Visitor(AC, *this, CurrStates);
> > -
> > +
>
> Whitespace only?
>
>
Fixed


> >      // Visit all of the basic block's statements.
> >      for (CFGBlock::const_iterator BI = CurrBlock->begin(),
> >           BE = CurrBlock->end(); BI != BE; ++BI) {
> >
> > -      if (BI->getKind() == CFGElement::Statement)
> > +      switch (BI->getKind()) {
> > +      case CFGElement::Statement:
> >          Visitor.Visit(BI->castAs<CFGStmt>().getStmt());
> > +        break;
> > +      case CFGElement::AutomaticObjectDtor:
> > +
>  CurrStates->remove(BI->castAs<CFGAutomaticObjDtor>().getVarDecl());
> > +      default:
> > +        break;
>
> Should this default be unreachable?
>
>
Nope.

> +      }
> >      }
> >
> > -    // TODO: Remove any variables that have reached the end of their
> > -    //       lifetimes from the state map. (Deferred)
> > -
> >      if (const IfStmt *Terminator =
> >        dyn_cast_or_null<IfStmt>(CurrBlock->getTerminator().getStmt())) {
> >
> > @@ -786,8 +802,8 @@
> >    WarningsHandler.emitDiagnostics();
> >  }
> >
> > -bool isTestingFunction(const CXXMethodDecl *Method) {
> > -  return Method->hasAttr<TestsUnconsumedAttr>();
> > +bool isTestingFunction(const FunctionDecl *FunDecl) {
> > +  return FunDecl->hasAttr<TestsUnconsumedAttr>();
> >  }
>
> Can this function be static?
>
>
Done.


> >
> >  }} // end namespace clang::consumed
> > Index: lib/Sema/AnalysisBasedWarnings.cpp
> > ===================================================================
> > --- lib/Sema/AnalysisBasedWarnings.cpp
> > +++ lib/Sema/AnalysisBasedWarnings.cpp
> > @@ -1551,10 +1551,9 @@
> >    DefaultPolicy.enableThreadSafetyAnalysis = (unsigned)
> >      (D.getDiagnosticLevel(diag::warn_double_lock, SourceLocation()) !=
> >       DiagnosticsEngine::Ignored);
> > -  DefaultPolicy.enableConsumedAnalysis =
> > -      (unsigned)(D.getDiagnosticLevel(diag::warn_use_while_consumed,
> > -                                      SourceLocation()) !=
> > -                 DiagnosticsEngine::Ignored);
> > +  DefaultPolicy.enableConsumedAnalysis = (unsigned)
> > +    (D.getDiagnosticLevel(diag::warn_use_while_consumed,
> SourceLocation()) !=
> > +     DiagnosticsEngine::Ignored);
> >  }
> >
> >  static void flushDiagnostics(Sema &S, sema::FunctionScopeInfo *fscope) {
> > Index: lib/Sema/SemaDeclAttr.cpp
> > ===================================================================
> > --- lib/Sema/SemaDeclAttr.cpp
> > +++ lib/Sema/SemaDeclAttr.cpp
> > @@ -497,8 +497,6 @@
>
> Removing the assert(!Attr.isInvalid()); is a good change, but should
> be submitted separately (not as part of this patch).
>
>
While this can be done I would have to walk back some patches break this
one out, re-apply patches, and then submit another patch.  I admit I should
have done a seperate patch, but at this point it would be a non-trivial
amount of work for very little benefit.  I would like to just let this one
slide and keep this in mind for future patches.

>
> >  static bool checkGuardedVarAttrCommon(Sema &S, Decl *D,
> >                                        const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeNumArgs(S, Attr, 0))
> >      return false;
> >
> > @@ -537,8 +535,6 @@
> >  static bool checkGuardedByAttrCommon(Sema &S, Decl *D,
> >                                       const AttributeList &Attr,
> >                                       Expr* &Arg) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeNumArgs(S, Attr, 1))
> >      return false;
> >
> > @@ -584,8 +580,6 @@
> >
> >  static bool checkLockableAttrCommon(Sema &S, Decl *D,
> >                                      const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeNumArgs(S, Attr, 0))
> >      return false;
> >
> > @@ -618,8 +612,6 @@
> >
> >  static void handleNoThreadSafetyAnalysis(Sema &S, Decl *D,
> >                                           const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeNumArgs(S, Attr, 0))
> >      return;
> >
> > @@ -635,8 +627,6 @@
> >
> >  static void handleNoSanitizeAddressAttr(Sema &S, Decl *D,
> >                                        const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeNumArgs(S, Attr, 0))
> >      return;
> >
> > @@ -653,8 +643,6 @@
> >
> >  static void handleNoSanitizeMemory(Sema &S, Decl *D,
> >                                     const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeNumArgs(S, Attr, 0))
> >      return;
> >
> > @@ -670,8 +658,6 @@
> >
> >  static void handleNoSanitizeThread(Sema &S, Decl *D,
> >                                     const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeNumArgs(S, Attr, 0))
> >      return;
> >
> > @@ -688,8 +674,6 @@
> >  static bool checkAcquireOrderAttrCommon(Sema &S, Decl *D,
> >                                          const AttributeList &Attr,
> >                                          SmallVectorImpl<Expr *> &Args) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
> >      return false;
> >
> > @@ -749,8 +733,6 @@
> >  static bool checkLockFunAttrCommon(Sema &S, Decl *D,
> >                                     const AttributeList &Attr,
> >                                     SmallVectorImpl<Expr *> &Args) {
> > -  assert(!Attr.isInvalid());
> > -
> >    // zero or more arguments ok
> >
> >    // check that the attribute is applied to a function
> > @@ -824,8 +806,7 @@
> >  static bool checkTryLockFunAttrCommon(Sema &S, Decl *D,
> >                                        const AttributeList &Attr,
> >                                        SmallVectorImpl<Expr *> &Args) {
> > -  assert(!Attr.isInvalid());
> > -
> > +
>
> Whitespace only?
>
> >    if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
> >      return false;
> >
> > @@ -878,8 +859,6 @@
> >  static bool checkLocksRequiredCommon(Sema &S, Decl *D,
> >                                       const AttributeList &Attr,
> >                                       SmallVectorImpl<Expr *> &Args) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
> >      return false;
> >
> > @@ -925,8 +904,6 @@
> >
> >  static void handleUnlockFunAttr(Sema &S, Decl *D,
> >                                  const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> > -
> >    // zero or more arguments ok
> >
> >    if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) {
> > @@ -948,8 +925,6 @@
> >
> >  static void handleLockReturnedAttr(Sema &S, Decl *D,
> >                                     const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeNumArgs(S, Attr, 1))
> >      return;
> >
> > @@ -973,8 +948,6 @@
> >
> >  static void handleLocksExcludedAttr(Sema &S, Decl *D,
> >                                      const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> > -
> >    if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
> >      return;
> >
> > @@ -999,12 +972,11 @@
> >
> >  static void handleConsumesAttr(Sema &S, Decl *D,
> >                                 const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> >    if (!checkAttributeNumArgs(S, Attr, 0)) return;
> >
> >    if (!(isa<CXXMethodDecl>(D) || isa<CXXConstructorDecl>(D))) {
> > -    S.Diag(Attr.getLoc(),
> diag::warn_uniqueness_attribute_wrong_decl_type) <<
> > -      Attr.getName();
> > +    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
> > +      Attr.getName() << "methods";
>
> You should be using a value from AttributeDeclKind (probably
> ExpectedMethod) instead of a string literal.  Should probably have a
> test to ensure this is properly caught.
>
>
Switched to ExpectedMethod.  What kind of tests are you proposing?


> >      return;
> >    }
> >
> > @@ -1015,12 +987,11 @@
> >
> >  static void handleCallableWhenUnconsumedAttr(Sema &S, Decl *D,
> >                                               const AttributeList &Attr)
> {
> > -  assert(!Attr.isInvalid());
> >    if (!checkAttributeNumArgs(S, Attr, 0)) return;
> >
> >    if (!isa<CXXMethodDecl>(D)) {
> > -    S.Diag(Attr.getLoc(),
> diag::warn_uniqueness_attribute_wrong_decl_type) <<
> > -      Attr.getName();
> > +    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
> > +      Attr.getName() << "methods";
>
> Same here.
>
> >      return;
> >    }
> >
> > @@ -1031,12 +1002,11 @@
> >
> >  static void handleTestsConsumedAttr(Sema &S, Decl *D,
> >                                      const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> >    if (!checkAttributeNumArgs(S, Attr, 0)) return;
> >
> >    if (!isa<CXXMethodDecl>(D)) {
> > -    S.Diag(Attr.getLoc(),
> diag::warn_uniqueness_attribute_wrong_decl_type) <<
> > -      Attr.getName();
> > +    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
> > +      Attr.getName() << "methods";
>
> And here.
>
> >      return;
> >    }
> >
> > @@ -1047,12 +1017,11 @@
> >
> >  static void handleTestsUnconsumedAttr(Sema &S, Decl *D,
> >                                        const AttributeList &Attr) {
> > -  assert(!Attr.isInvalid());
> >    if (!checkAttributeNumArgs(S, Attr, 0)) return;
> >
> >    if (!isa<CXXMethodDecl>(D)) {
> > -    S.Diag(Attr.getLoc(),
> diag::warn_uniqueness_attribute_wrong_decl_type) <<
> > -      Attr.getName();
> > +    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
> > +      Attr.getName() << "methods";
>
> And here.
>
> >      return;
> >    }
> >
> > Index: test/SemaCXX/warn-consumed-analysis.cpp
> > ===================================================================
> > --- test/SemaCXX/warn-consumed-analysis.cpp
> > +++ test/SemaCXX/warn-consumed-analysis.cpp
> > @@ -27,10 +27,13 @@
> >    template <typename U>
> >    Bar<T>& operator=(Bar<U> &&other);
> >
> > +  void operator()(int a) CONSUMES;
> >    void operator*(void) const CALLABLE_WHEN_UNCONSUMED;
> > +  void unconsumedCall(void) const CALLABLE_WHEN_UNCONSUMED;
> >
> >    bool isValid(void) const TESTS_UNCONSUMED;
> >    operator bool() const TESTS_UNCONSUMED;
> > +  bool operator!=(nullptr_t) const TESTS_UNCONSUMED;
> >
> >    void constCall(void) const;
> >    void nonconstCall(void);
> > @@ -98,6 +101,13 @@
> >    } else {
> >      *var; // expected-warning {{invocation of method 'operator*' on
> object 'var' while it is in the 'consumed' state}}
> >    }
> > +
> > +  if (var != nullptr) {
> > +    // Empty
> > +
> > +  } else {
> > +    *var; // expected-warning {{invocation of method 'operator*' on
> object 'var' while it is in the 'consumed' state}}
> > +  }
> >  }
> >
> >  void testCallingConventions(void) {
> > @@ -164,6 +174,15 @@
> >    *var; // expected-warning {{invocation of method 'operator*' on
> object 'var' while it is in the 'consumed' state}}
> >  }
> >
> > +void testConsumes2(void) {
> > +  Bar<int> var(42);
> > +
> > +  var.unconsumedCall();
> > +  var(6);
> > +
> > +  var.unconsumedCall(); // expected-warning {{invocation of method
> 'unconsumedCall' on object 'var' while it is in the 'consumed' state}}
> > +}
> > +
> >  void testSimpleForLoop(void) {
> >    Bar<int> var;
>
> ~Aaron
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130820/351a39d1/attachment.html>


More information about the cfe-commits mailing list