[PATCH] Explicitly keep track of temporaries during the consumed analysis.

[Delesley Hutchins]

LGTM.  Thanks for the patch!

On Wed, May 7, 2014 at 4:21 AM, Manuel Klimek <klimek at google.com> wrote:
> Hi delesley,
>
> This makes the consumed analysis less dependent on the CFG layout and fixes
> a bug where we wouldn't warn on an unconsumed value.
>
> http://reviews.llvm.org/D3643
>
> Files:
>   include/clang/Analysis/Analyses/Consumed.h
>   lib/Analysis/Consumed.cpp
>   test/SemaCXX/warn-consumed-analysis.cpp
>
> Index: include/clang/Analysis/Analyses/Consumed.h
> ===================================================================
> --- include/clang/Analysis/Analyses/Consumed.h
> +++ include/clang/Analysis/Analyses/Consumed.h
> @@ -185,8 +185,8 @@
>      /// \brief Set the consumed state of a given temporary value.
>      void setState(const CXXBindTemporaryExpr *Tmp, ConsumedState State);
>
> -    /// \brief Remove the variable from our state map.
> -    void remove(const VarDecl *Var);
> +    /// \brief Remove the temporary value from our state map.
> +    void remove(const CXXBindTemporaryExpr *Tmp);
>
>      /// \brief Tests to see if there is a mismatch in the states stored in two
>      /// maps.
> Index: lib/Analysis/Consumed.cpp
> ===================================================================
> --- lib/Analysis/Consumed.cpp
> +++ lib/Analysis/Consumed.cpp
> @@ -1233,8 +1233,8 @@
>    TmpMap[Tmp] = State;
>  }
>
> -void ConsumedStateMap::remove(const VarDecl *Var) {
> -  VarMap.erase(Var);
> +void ConsumedStateMap::remove(const CXXBindTemporaryExpr *Tmp) {
> +  TmpMap.erase(Tmp);
>  }
>
>  bool ConsumedStateMap::operator!=(const ConsumedStateMap *Other) const {
> @@ -1413,6 +1413,7 @@
>          Visitor.checkCallability(PropagationInfo(BTE),
>                                   DTor.getDestructorDecl(AC.getASTContext()),
>                                   BTE->getExprLoc());
> +        CurrStates->remove(BTE);
>          break;
>        }
>
> @@ -1432,8 +1433,6 @@
>        }
>      }
>
> -    CurrStates->clearTemporaries();
> -
>      // TODO: Handle other forms of branching with precision, including while-
>      //       and for-loops. (Deferred)
>      if (!splitState(CurrBlock, Visitor)) {
> Index: test/SemaCXX/warn-consumed-analysis.cpp
> ===================================================================
> --- test/SemaCXX/warn-consumed-analysis.cpp
> +++ test/SemaCXX/warn-consumed-analysis.cpp
> @@ -704,6 +704,8 @@
>    void clear() CALLABLE_WHEN("unknown", "consumed") SET_TYPESTATE(consumed);
>
>    ~Status() CALLABLE_WHEN("unknown", "consumed");
> +
> +  operator bool() const; // Will not consume the object.
>  };
>
>
> @@ -735,6 +737,10 @@
>    Status s = doSomething();
>  }  // expected-warning {{invalid invocation of method '~Status' on object 's' while it is in the 'unconsumed' state}}
>
> +void testTemporariesWithControlFlow(bool a) {
> +  bool b = false || doSomething(); // expected-warning {{invalid invocation of method '~Status' on a temporary object while it is in the 'unconsumed' state}}
> +}
> +
>  Status testSimpleTemporariesReturn0() {
>    return doSomething();
>  }



-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315