<div dir="ltr"><div><div>All of the ConsumedStateMap objects are freed either in the second version of `addInfo` or in the `run` function. You are correct about the conditional at the end of `splitState` however. That was added to handle null successor blocks after the ConsumedStateMap propagation was implemented. I'll fix that in an updated patch.<br>
<br></div>More comments bellow.<br></div><div><br></div><div>Thanks for the feedback,<br></div>Chris<br><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 26, 2013 at 1:43 PM, 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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im">On Mon, Aug 26, 2013 at 2:23 PM, Christian Wailes<br>
<<a href="mailto:chriswailes@google.com">chriswailes@google.com</a>> wrote:<br>
> If you are referring to the handling of the ConsumedStateMap objects, these<br>
> are guaranteed to be freed. A map can either be passed to another block<br>
> (and re-used) via CurrStates, passed to another block using the BlockInfo<br>
> manager, or merged into an existing block. The last case is the one where<br>
> we need to free a map. This occurs in the `run` method and the end of<br>
> `splitState` via a call to overloaded `addInfo`. The last existing map is<br>
> always deleted as the second to last statement in `run`.<br>
<br>
</div>I must still be missing something. ConsumedBlockInfo::addInfo either<br>
takes ownership of a pointer, or creates a new pointer. Yet it has no<br>
destructor, so everything in StateMapsArray is being leaked as far as<br>
I can tell. What's more, there are cases where you create a new<br>
object, but it may not be added to the BlockInfo manager (such as not<br>
an IfStmt or BinOp in splitState, or *++SI evaluates to false, also in<br>
splitState).<br>
<br>
I agree with David that this patch is not gated on fixing these<br>
issues, but I think a subsequent patch removing the naked pointers<br>
would be a Very Good Thing. The ownership strategy here is rather<br>
hard to follow. For instance, ConsumedBlockInfo::addInfo will<br>
sometimes (but not always) delete the argument passed into it. That<br>
means the caller has to understand that sometimes the pointer they<br>
passed in will be invalidated on return. But that depends on which<br>
addInfo gets called. One addInfo deletes the state map if it's<br>
already set, the other does not.<br>
<div><div class="h5"><br>
> There are responses to your comments bellow, and I am submitting a patch<br>
> with fixes now.<br>
><br>
> - Chris<br>
><br>
><br>
> On Sat, Aug 24, 2013 at 8:03 AM, Aaron Ballman <<a href="mailto:aaron.ballman@gmail.com">aaron.ballman@gmail.com</a>><br>
> wrote:<br>
>><br>
>> Mostly nits about formatting and whitespace, but my biggest concern<br>
>> has to do with memory leaks involving naked pointers (not really<br>
>> introduced in this patch). Also, there is an conversion to bool<br>
>> operator that kind of scares me (we don't have access to explicit<br>
>> operator conversions in C++11 yet, otherwise I would simply suggest<br>
>> marking that as explicit).<br>
>><br>
>> ~Aaron<br>
>><br>
>> > Index: include/clang/Analysis/Analyses/Consumed.h<br>
>> > ===================================================================<br>
>> > --- include/clang/Analysis/Analyses/Consumed.h<br>
>> > +++ include/clang/Analysis/Analyses/Consumed.h<br>
>> > @@ -24,7 +24,9 @@<br>
>> ><br>
>> > namespace clang {<br>
>> > namespace consumed {<br>
>> > -<br>
>> > +<br>
>> > + class ConsumedStmtVisitor;<br>
>> > +<br>
>> > typedef SmallVector<PartialDiagnosticAt, 1> OptionalNotes;<br>
>> > typedef std::pair<PartialDiagnosticAt, OptionalNotes> DelayedDiag;<br>
>> > typedef std::list<DelayedDiag> DiagList;<br>
>> > @@ -38,7 +40,7 @@<br>
>> > /// \brief Emit the warnings and notes left by the analysis.<br>
>> > virtual void emitDiagnostics() {}<br>
>> ><br>
>> > - /// Warn about unnecessary-test errors.<br>
>> > + /// \brief Warn about unnecessary-test errors.<br>
>> > /// \param VariableName -- The name of the variable that holds the<br>
>> > unique<br>
>> > /// value.<br>
>> > ///<br>
>> > @@ -47,7 +49,7 @@<br>
>> > StringRef VariableState,<br>
>> > SourceLocation Loc) {}<br>
>> ><br>
>> > - /// Warn about use-while-consumed errors.<br>
>> > + /// \brief Warn about use-while-consumed errors.<br>
>> > /// \param MethodName -- The name of the method that was<br>
>> > incorrectly<br>
>> > /// invoked.<br>
>> > ///<br>
>> > @@ -55,7 +57,7 @@<br>
>> > virtual void warnUseOfTempWhileConsumed(StringRef MethodName,<br>
>> > SourceLocation Loc) {}<br>
>> ><br>
>> > - /// Warn about use-in-unknown-state errors.<br>
>> > + /// \brief Warn about use-in-unknown-state errors.<br>
>> > /// \param MethodName -- The name of the method that was<br>
>> > incorrectly<br>
>> > /// invoked.<br>
>> > ///<br>
>> > @@ -63,7 +65,7 @@<br>
>> > virtual void warnUseOfTempInUnknownState(StringRef MethodName,<br>
>> > SourceLocation Loc) {}<br>
>> ><br>
>> > - /// Warn about use-while-consumed errors.<br>
>> > + /// \brief Warn about use-while-consumed errors.<br>
>> > /// \param MethodName -- The name of the method that was<br>
>> > incorrectly<br>
>> > /// invoked.<br>
>> > ///<br>
>> > @@ -75,7 +77,7 @@<br>
>> > StringRef VariableName,<br>
>> > SourceLocation Loc) {}<br>
>> ><br>
>> > - /// Warn about use-in-unknown-state errors.<br>
>> > + /// \brief Warn about use-in-unknown-state errors.<br>
>> > /// \param MethodName -- The name of the method that was<br>
>> > incorrectly<br>
>> > /// invoked.<br>
>> > ///<br>
>> > @@ -90,7 +92,7 @@<br>
>> ><br>
>> > enum ConsumedState {<br>
>> > // No state information for the given variable.<br>
>> > - CS_None,<br>
>> > + CS_None = 0,<br>
>><br>
>> Any particular reason for this change?<br>
><br>
><br>
> So that CS_None evaluates to false if a ConsumedState value is used in a<br>
> conditional.<br>
<br>
</div></div>It would still evaluate to false; it's value when unspecified will be zero.<br>
<div><div class="h5"><br></div></div></blockquote><div><br></div><div>Sure, but wouldn't that change if someone added a new enum value in the wrong place?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5">
><br>
>><br>
>><br>
>> ><br>
>> > CS_Unknown,<br>
>> > CS_Unconsumed,<br>
>> > @@ -104,18 +106,35 @@<br>
>> ><br>
>> > protected:<br>
>> ><br>
>> > + bool Reachable;<br>
>> > + const Stmt *From;<br>
>> > MapType Map;<br>
>> ><br>
>> > public:<br>
>> > + ConsumedStateMap() : Reachable(true), From(NULL) {}<br>
>> > + ConsumedStateMap(const ConsumedStateMap &Other)<br>
>> > + : Reachable(Other.Reachable), From(Other.From), Map(Other.Map) {}<br>
>> > +<br>
>> > /// \brief Get the consumed state of a given variable.<br>
>> > ConsumedState getState(const VarDecl *Var);<br>
>> ><br>
>> > /// \brief Merge this state map with another map.<br>
>> > void intersect(const ConsumedStateMap *Other);<br>
>> ><br>
>> > + /// \brief Return true if this block is reachable.<br>
>> > + bool isReachable();<br>
>><br>
>> This method should be const and can be inlined.<br>
>><br>
>> > +<br>
>> > /// \brief Mark all variables as unknown.<br>
>> > void makeUnknown();<br>
>> ><br>
>> > + /// \brief Mark the block as unreachable.<br>
>> > + void markUnreachable();<br>
>> > +<br>
>> > + /// \brief Set the source for a decision about the branching of<br>
>> > states.<br>
>> > + /// \param Source -- The statement that was the origin of a<br>
>> > branching<br>
>> > + /// decision.<br>
>> > + void setSource(const Stmt *Source);<br>
>><br>
>> This one can be inlined as well.<br>
>><br>
>> > +<br>
>> > /// \brief Set the consumed state of a given variable.<br>
>> > void setState(const VarDecl *Var, ConsumedState State);<br>
>> ><br>
>> > @@ -144,18 +163,6 @@<br>
>> ><br>
>> > void markVisited(const CFGBlock *Block);<br>
>> > };<br>
>> > -<br>
>> > - struct VarTestResult {<br>
>> > - const VarDecl *Var;<br>
>> > - SourceLocation Loc;<br>
>> > - bool UnconsumedInTrueBranch;<br>
>> > -<br>
>> > - VarTestResult() : Var(NULL), Loc(), UnconsumedInTrueBranch(true) {}<br>
>> > -<br>
>> > - VarTestResult(const VarDecl *Var, SourceLocation Loc,<br>
>> > - bool UnconsumedInTrueBranch)<br>
>> > - : Var(Var), Loc(Loc),<br>
>> > UnconsumedInTrueBranch(UnconsumedInTrueBranch) {}<br>
>> > - };<br>
>> ><br>
>> > /// A class that handles the analysis of uniqueness violations.<br>
>> > class ConsumedAnalyzer {<br>
>> > @@ -169,7 +176,8 @@<br>
>> > CacheMapType ConsumableTypeCache;<br>
>> ><br>
>> > bool hasConsumableAttributes(const CXXRecordDecl *RD);<br>
>> > - void splitState(const CFGBlock *CurrBlock, const IfStmt<br>
>> > *Terminator);<br>
>> > + bool splitState(const CFGBlock *CurrBlock,<br>
>> > + const ConsumedStmtVisitor &Visitor);<br>
>><br>
>> Formatting for the Visitor.<br>
><br>
><br>
> Again, this isn't showing up in my patch or my editor. Are you viewing the<br>
> patch with non-fixed-width fonts?<br>
<br>
</div></div>Yes, this is my mail client being tricky. Sorry for that.<br>
<div><div class="h5"><br>
><br>
>><br>
>><br>
>> ><br>
>> > public:<br>
>> ><br>
>> > Index: lib/Analysis/Consumed.cpp<br>
>> > ===================================================================<br>
>> > --- lib/Analysis/Consumed.cpp<br>
>> > +++ lib/Analysis/Consumed.cpp<br>
>> > @@ -30,8 +30,13 @@<br>
>> > #include "llvm/ADT/SmallVector.h"<br>
>> > #include "llvm/Support/raw_ostream.h"<br>
>> ><br>
>> > +// TODO: Correctly identify unreachable blocks when chaining boolean<br>
>> > operators.<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<br>
>> > if they<br>
>> > // are referenced inside that block. (Deferred)<br>
>> > +// TODO: Handle variable definitions, e.g. bool valid = x.isValid();<br>
>> > +// if (valid) ...; (Deferred)<br>
>> > // TODO: Add a method(s) to identify which method calls perform what<br>
>> > state<br>
>> > // transitions. (Deferred)<br>
>> > // TODO: Take notes on state transitions to provide better warning<br>
>> > messages.<br>
>> > @@ -47,6 +52,29 @@<br>
>> > // Key method definition<br>
>> > ConsumedWarningsHandlerBase::~ConsumedWarningsHandlerBase() {}<br>
>> ><br>
>> > +static ConsumedState invertConsumedUnconsumed(ConsumedState State) {<br>
>> > + switch (State) {<br>
>> > + case CS_Unconsumed:<br>
>> > + return CS_Consumed;<br>
>> > + case CS_Consumed:<br>
>> > + return CS_Unconsumed;<br>
>> > + case CS_None:<br>
>> > + case CS_Unknown:<br>
>> > + llvm_unreachable("invalid enum");<br>
>> > + }<br>
>> > +}<br>
>> > +<br>
>> > +static bool isKnownState(ConsumedState State) {<br>
>> > + switch (State) {<br>
>> > + case CS_Unconsumed:<br>
>> > + case CS_Consumed:<br>
>> > + return true;<br>
>> > + case CS_None:<br>
>> > + case CS_Unknown:<br>
>> > + return false;<br>
>> > + }<br>
>> > +}<br>
>> > +<br>
>> > static bool isTestingFunction(const FunctionDecl *FunDecl) {<br>
>> > return FunDecl->hasAttr<TestsUnconsumedAttr>();<br>
>> > }<br>
>> > @@ -69,40 +97,143 @@<br>
>> > }<br>
>> ><br>
>> > namespace {<br>
>> > -class ConsumedStmtVisitor : public<br>
>> > ConstStmtVisitor<ConsumedStmtVisitor> {<br>
>> > +struct VarTestResult {<br>
>> > + const VarDecl *Var;<br>
>> > + ConsumedState TestsFor;<br>
>> > +};<br>
>> > +} // end anonymous::VarTestResult<br>
>> > +<br>
>> > +namespace clang {<br>
>> > +namespace consumed {<br>
>> > +<br>
>> > +enum EffectiveOp {<br>
>> > + EO_And,<br>
>> > + EO_Or<br>
>> > +};<br>
>> > +<br>
>> > +class PropagationInfo {<br>
>> > + enum {<br>
>> > + IT_None,<br>
>> > + IT_State,<br>
>> > + IT_Test,<br>
>> > + IT_BinTest,<br>
>> > + IT_Var<br>
>> > + } InfoType;<br>
>> ><br>
>> > - union PropagationUnion {<br>
>> > + union {<br>
>> > ConsumedState State;<br>
>> > + VarTestResult Test;<br>
>> > const VarDecl *Var;<br>
>> > + struct {<br>
>> > + const BinaryOperator *Source;<br>
>> > + EffectiveOp EOp;<br>
>> > + VarTestResult LTest;<br>
>> > + VarTestResult RTest;<br>
>> > + } BinTest;<br>
>> > };<br>
>> ><br>
>> > - class PropagationInfo {<br>
>> > - PropagationUnion StateOrVar;<br>
>> > +public:<br>
>> > + PropagationInfo() : InfoType(IT_None) {}<br>
>> ><br>
>> > - public:<br>
>> > - bool IsVar;<br>
>> > + PropagationInfo(const VarTestResult &Test) : InfoType(IT_Test),<br>
>> > Test(Test) {}<br>
>> > + PropagationInfo(const VarDecl *Var, ConsumedState TestsFor)<br>
>> > + : InfoType(IT_Test) {<br>
>> ><br>
>> > - PropagationInfo() : IsVar(false) {<br>
>> > - StateOrVar.State = consumed::CS_None;<br>
>> > - }<br>
>> > + Test.Var = Var;<br>
>><br>
>> Whitespace<br>
>><br>
>> > + Test.TestsFor = TestsFor;<br>
>> > + }<br>
>> > +<br>
>> > + PropagationInfo(const BinaryOperator *Source, EffectiveOp EOp,<br>
>> > + const VarTestResult <est, const VarTestResult<br>
>> > &RTest)<br>
>> > + : InfoType(IT_BinTest) {<br>
>> ><br>
>> > - PropagationInfo(ConsumedState State) : IsVar(false) {<br>
>> > - StateOrVar.State = State;<br>
>> > - }<br>
>> > + BinTest.Source = Source;<br>
>> > + BinTest.EOp = EOp;<br>
>> > + BinTest.LTest = LTest;<br>
>> > + BinTest.RTest = RTest;<br>
>> > + }<br>
>> > +<br>
>> > + PropagationInfo(const BinaryOperator *Source, EffectiveOp EOp,<br>
>> > + const VarDecl *LVar, ConsumedState LTestsFor,<br>
>> > + const VarDecl *RVar, ConsumedState RTestsFor)<br>
>><br>
>> Code formatting<br>
><br>
><br>
> Same comment as above concerning fonts.<br>
><br>
>><br>
>><br>
>> > + : InfoType(IT_BinTest) {<br>
>> ><br>
>> > - PropagationInfo(const VarDecl *Var) : IsVar(true) {<br>
>> > - StateOrVar.Var = Var;<br>
>> > - }<br>
>> > + BinTest.Source = Source;<br>
>> > + BinTest.EOp = EOp;<br>
>> > + BinTest.LTest.Var = LVar;<br>
>> > + BinTest.LTest.TestsFor = LTestsFor;<br>
>> > + BinTest.RTest.Var = RVar;<br>
>> > + BinTest.RTest.TestsFor = RTestsFor;<br>
>><br>
>> Hopefully this looks better in the editor than it does in my mail<br>
>> client. ;-) I don't think there's a rule against it, but I don't<br>
>> think many places in the code base attempt to line up assignments.<br>
>> The only place I've ever really seen this is with case statement<br>
>> bodies, and even there it's not consistent.<br>
>><br>
>> > + }<br>
>> > +<br>
>> > + PropagationInfo(ConsumedState State) : InfoType(IT_State),<br>
>> > State(State) {}<br>
>> > + PropagationInfo(const VarDecl *Var) : InfoType(IT_Var), Var(Var) {}<br>
>> > +<br>
>> > + const ConsumedState & getState() const {<br>
>> > + assert(InfoType == IT_State);<br>
>> > + return State;<br>
>> > + }<br>
>> > +<br>
>> > + const VarTestResult & getTest() const {<br>
>> > + assert(InfoType == IT_Test);<br>
>> > + return Test;<br>
>> > + }<br>
>> > +<br>
>> > + const VarTestResult & getLTest() const {<br>
>> > + assert(InfoType == IT_BinTest);<br>
>> > + return BinTest.LTest;<br>
>> > + }<br>
>> > +<br>
>> > + const VarTestResult & getRTest() const {<br>
>> > + assert(InfoType == IT_BinTest);<br>
>> > + return BinTest.RTest;<br>
>> > + }<br>
>> > +<br>
>> > + const VarDecl * getVar() const {<br>
>> > + assert(InfoType == IT_Var);<br>
>> > + return Var;<br>
>> > + }<br>
>> > +<br>
>> > + EffectiveOp testEffectiveOp() const {<br>
>> > + assert(InfoType == IT_BinTest);<br>
>> > + return BinTest.EOp;<br>
>> > + }<br>
>> > +<br>
>> > + const BinaryOperator * testSourceNode() const {<br>
>> > + assert(InfoType == IT_BinTest);<br>
>> > + return BinTest.Source;<br>
>> > + }<br>
>> > +<br>
>> > + operator bool() const { return InfoType != IT_None; }<br>
>><br>
>> Implicit conversions to bool are kind of terrifying. This makes the<br>
>> class now assignable as an integer, which can lead to obscure bugs.<br>
>> I'd prefer to avoid this.<br>
>><br>
><br>
> I think "is there information or not" is a pretty obvious test, so I would<br>
> like to go with David's suggestion and use LLVM_EXPLICIT if there aren't any<br>
> objections.<br>
<br>
</div></div>My gut feeling is that there's basically no harm in naming the<br>
operation, especially given that there's only one usage of it in the<br>
entire source file.<br>
<div><div class="h5"><br></div></div></blockquote><div><br>I personally find the Boolean conversion operator clearer and easier to
use when programming than a named function, and from what I understand
the LLVM_EXPLICIT macro fixes the issue with PropagationInfo objects
being assigned to integers. I'm also not sure what name would be
clearer; `isValid()` doesn't convey any more information in my mind.
That being said, I can change it if people want.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5">
><br>
>><br>
>> > + bool isState() const { return InfoType == IT_State; }<br>
>> > + bool isTest() const { return InfoType == IT_Test; }<br>
>> > + bool isBinTest() const { return InfoType == IT_BinTest; }<br>
>> > + bool isVar() const { return InfoType == IT_Var; }<br>
>><br>
>> Whitespace between the end of the parameter list and the const.<br>
>><br>
>> > +<br>
>> > + PropagationInfo invertTest() const {<br>
>> > + assert(InfoType == IT_Test || InfoType == IT_BinTest);<br>
>> ><br>
>> > - ConsumedState getState() const { return StateOrVar.State; }<br>
>> > + if (InfoType == IT_Test) {<br>
>> > + return PropagationInfo(Test.Var,<br>
>> > invertConsumedUnconsumed(Test.TestsFor));<br>
>> ><br>
>> > - const VarDecl * getVar() const { return IsVar ? StateOrVar.Var :<br>
>> > NULL; }<br>
>> > - };<br>
>> > + } else if (InfoType == IT_BinTest) {<br>
>> > + return PropagationInfo(BinTest.Source,<br>
>> > + BinTest.EOp == EO_And ? EO_Or : EO_And,<br>
>> > + BinTest.LTest.Var,<br>
>> > invertConsumedUnconsumed(BinTest.LTest.TestsFor),<br>
>> > + BinTest.RTest.Var,<br>
>> > invertConsumedUnconsumed(BinTest.RTest.TestsFor));<br>
>> > + } else {<br>
>> > + return PropagationInfo();<br>
>> > + }<br>
>> > + }<br>
>> > +};<br>
>> > +<br>
>> > +class ConsumedStmtVisitor : public<br>
>> > ConstStmtVisitor<ConsumedStmtVisitor> {<br>
>> ><br>
>> > typedef llvm::DenseMap<const Stmt *, PropagationInfo> MapType;<br>
>> > typedef std::pair<const Stmt *, PropagationInfo> PairType;<br>
>> > typedef MapType::iterator InfoEntry;<br>
>> > -<br>
>> > + typedef MapType::const_iterator ConstInfoEntry;<br>
>> > +<br>
>> > AnalysisDeclContext &AC;<br>
>> > ConsumedAnalyzer &Analyzer;<br>
>> > ConsumedStateMap *StateMap;<br>
>> > @@ -112,10 +243,11 @@<br>
>> > const FunctionDecl *FunDecl,<br>
>> > const CallExpr *Call);<br>
>> > void forwardInfo(const Stmt *From, const Stmt *To);<br>
>> > + void handleTestingFunctionCall(const CallExpr *Call, const VarDecl<br>
>> > *Var);<br>
>> > bool isLikeMoveAssignment(const CXXMethodDecl *MethodDecl);<br>
>> ><br>
>> > public:<br>
>> > -<br>
>> > +<br>
>> > void Visit(const Stmt *StmtNode);<br>
>> ><br>
>> > void VisitBinaryOperator(const BinaryOperator *BinOp);<br>
>> > @@ -131,12 +263,20 @@<br>
>> > void VisitUnaryOperator(const UnaryOperator *UOp);<br>
>> > void VisitVarDecl(const VarDecl *Var);<br>
>> ><br>
>> > - ConsumedStmtVisitor(AnalysisDeclContext &AC, ConsumedAnalyzer<br>
>> > &Analyzer,<br>
>> > - ConsumedStateMap *StateMap)<br>
>> > - : AC(AC), Analyzer(Analyzer), StateMap(StateMap) {}<br>
>> > -<br>
>> > - void reset() {<br>
>> > - PropagationMap.clear();<br>
>> > + ConsumedStmtVisitor(AnalysisDeclContext &AC, ConsumedAnalyzer<br>
>> > &Analyzer)<br>
>> > + : AC(AC), Analyzer(Analyzer), StateMap(NULL) {}<br>
>> > +<br>
>> > + PropagationInfo getInfo(const Stmt *StmtNode) const {<br>
>> > + ConstInfoEntry Entry = PropagationMap.find(StmtNode);<br>
>> > +<br>
>> > + if (Entry != PropagationMap.end())<br>
>> > + return Entry->second;<br>
>> > + else<br>
>> > + return PropagationInfo();<br>
>> > + }<br>
>> > +<br>
>> > + void reset(ConsumedStateMap *NewStateMap) {<br>
>> > + StateMap = NewStateMap;<br>
>> > }<br>
>> > };<br>
>> ><br>
>> > @@ -148,7 +288,7 @@<br>
>> ><br>
>> > if (!FunDecl->hasAttr<CallableWhenUnconsumedAttr>()) return;<br>
>> ><br>
>> > - if (PInfo.IsVar) {<br>
>> > + if (PInfo.isVar()) {<br>
>> > const VarDecl *Var = PInfo.getVar();<br>
>> ><br>
>> > switch (StateMap->getState(Var)) {<br>
>> > @@ -191,9 +331,24 @@<br>
>> > void ConsumedStmtVisitor::forwardInfo(const Stmt *From, const Stmt *To)<br>
>> > {<br>
>> > InfoEntry Entry = PropagationMap.find(From);<br>
>> ><br>
>> > - if (Entry != PropagationMap.end()) {<br>
>> > - PropagationMap.insert(PairType(To,<br>
>> > PropagationInfo(Entry->second)));<br>
>> > + if (Entry != PropagationMap.end())<br>
>> > + PropagationMap.insert(PairType(To, Entry->second));<br>
>> > +}<br>
>> > +<br>
>> > +void ConsumedStmtVisitor::handleTestingFunctionCall(const CallExpr<br>
>> > *Call,<br>
>> > + const VarDecl<br>
>> > *Var) {<br>
>> > +<br>
>> > + ConsumedState VarState = StateMap->getState(Var);<br>
>> > +<br>
>> > + if (VarState != CS_Unknown) {<br>
>> > + SourceLocation CallLoc = Call->getExprLoc();<br>
>> > +<br>
>> > + if (!CallLoc.isMacroID())<br>
>> > +<br>
>> > Analyzer.WarningsHandler.warnUnnecessaryTest(Var->getNameAsString(),<br>
>> > + stateToString(VarState), CallLoc);<br>
>> > }<br>
>> > +<br>
>> > + PropagationMap.insert(PairType(Call, PropagationInfo(Var,<br>
>> > CS_Unconsumed)));<br>
>> > }<br>
>> ><br>
>> > bool ConsumedStmtVisitor::isLikeMoveAssignment(<br>
>> > @@ -205,8 +360,43 @@<br>
>> ><br>
>> > MethodDecl->getParamDecl(0)->getType()->isRValueReferenceType());<br>
>> > }<br>
>> ><br>
>> > +void ConsumedStmtVisitor::Visit(const Stmt *StmtNode) {<br>
>> > +<br>
>> > + ConstStmtVisitor<ConsumedStmtVisitor>::Visit(StmtNode);<br>
>> > +<br>
>> > + for (Stmt::const_child_iterator CI = StmtNode->child_begin(),<br>
>> > + CE = StmtNode->child_end(); CI != CE; ++CI) {<br>
>> > +<br>
>> > + PropagationMap.erase(*CI);<br>
>> > + }<br>
>> > +}<br>
>> > +<br>
>> > void ConsumedStmtVisitor::VisitBinaryOperator(const BinaryOperator<br>
>> > *BinOp) {<br>
>> > switch (BinOp->getOpcode()) {<br>
>> > + case BO_LAnd:<br>
>> > + case BO_LOr : {<br>
>> > + InfoEntry LEntry = PropagationMap.find(BinOp->getLHS()),<br>
>> > + REntry = PropagationMap.find(BinOp->getRHS());<br>
>> > +<br>
>> > + VarTestResult LTest, RTest;<br>
>> > +<br>
>> > + if (LEntry != PropagationMap.end() && LEntry->second.isTest())<br>
>> > + LTest = LEntry->second.getTest();<br>
>> > + else<br>
>> > + LTest.Var = NULL;<br>
>> > +<br>
>> > + if (REntry != PropagationMap.end() && REntry->second.isTest())<br>
>> > + RTest = REntry->second.getTest();<br>
>> > + else<br>
>> > + RTest.Var = NULL;<br>
>> > +<br>
>> > + if (!(LTest.Var == NULL && RTest.Var == NULL))<br>
>> > + PropagationMap.insert(PairType(BinOp, PropagationInfo(BinOp,<br>
>> > + static_cast<EffectiveOp>(BinOp->getOpcode() == BO_LOr), LTest,<br>
>> > RTest)));<br>
>> > +<br>
>> > + break;<br>
>> > + }<br>
>> > +<br>
>> > case BO_PtrMemD:<br>
>> > case BO_PtrMemI:<br>
>> > forwardInfo(BinOp->getLHS(), BinOp);<br>
>> > @@ -217,16 +407,6 @@<br>
>> > }<br>
>> > }<br>
>> ><br>
>> > -void ConsumedStmtVisitor::Visit(const Stmt *StmtNode) {<br>
>> > - ConstStmtVisitor<ConsumedStmtVisitor>::Visit(StmtNode);<br>
>> > -<br>
>> > - for (Stmt::const_child_iterator CI = StmtNode->child_begin(),<br>
>> > - CE = StmtNode->child_end(); CI != CE; ++CI) {<br>
>> > -<br>
>> > - PropagationMap.erase(*CI);<br>
>> > - }<br>
>> > -}<br>
>> > -<br>
>> > void ConsumedStmtVisitor::VisitCallExpr(const CallExpr *Call) {<br>
>> > if (const FunctionDecl *FunDecl =<br>
>> > dyn_cast_or_null<FunctionDecl>(Call->getDirectCallee())) {<br>
>> > @@ -250,7 +430,7 @@<br>
>> ><br>
>> > InfoEntry Entry = PropagationMap.find(Call->getArg(Index));<br>
>> ><br>
>> > - if (Entry == PropagationMap.end() || !Entry->second.IsVar) {<br>
>> > + if (Entry == PropagationMap.end() || !Entry->second.isVar()) {<br>
>> > continue;<br>
>> > }<br>
>> ><br>
>> > @@ -274,10 +454,7 @@<br>
>> > }<br>
>> ><br>
>> > void ConsumedStmtVisitor::VisitCastExpr(const CastExpr *Cast) {<br>
>> > - InfoEntry Entry = PropagationMap.find(Cast->getSubExpr());<br>
>> > -<br>
>> > - if (Entry != PropagationMap.end())<br>
>> > - PropagationMap.insert(PairType(Cast, Entry->second));<br>
>> > + forwardInfo(Cast->getSubExpr(), Cast);<br>
>> > }<br>
>> ><br>
>> > void ConsumedStmtVisitor::VisitCXXConstructExpr(const CXXConstructExpr<br>
>> > *Call) {<br>
>> > @@ -298,7 +475,7 @@<br>
>> > PropagationInfo PInfo =<br>
>> > PropagationMap.find(Call->getArg(0))->second;<br>
>> ><br>
>> > - if (PInfo.IsVar) {<br>
>> > + if (PInfo.isVar()) {<br>
>> > const VarDecl* Var = PInfo.getVar();<br>
>> ><br>
>> > PropagationMap.insert(PairType(Call,<br>
>> > @@ -336,8 +513,10 @@<br>
>> ><br>
>> > checkCallability(PInfo, MethodDecl, Call);<br>
>> ><br>
>> > - if (PInfo.IsVar) {<br>
>> > - if (MethodDecl->hasAttr<ConsumesAttr>())<br>
>> > + if (PInfo.isVar()) {<br>
>> > + if (isTestingFunction(MethodDecl))<br>
>> > + handleTestingFunctionCall(Call, PInfo.getVar());<br>
>> > + else if (MethodDecl->hasAttr<ConsumesAttr>())<br>
>> > StateMap->setState(PInfo.getVar(), consumed::CS_Consumed);<br>
>> > else if (!MethodDecl->isConst())<br>
>> > StateMap->setState(PInfo.getVar(), consumed::CS_Unknown);<br>
>> > @@ -367,7 +546,7 @@<br>
>> > LPInfo = LEntry->second;<br>
>> > RPInfo = REntry->second;<br>
>> ><br>
>> > - if (LPInfo.IsVar && RPInfo.IsVar) {<br>
>> > + if (LPInfo.isVar() && RPInfo.isVar()) {<br>
>> > StateMap->setState(LPInfo.getVar(),<br>
>> > StateMap->getState(RPInfo.getVar()));<br>
>> ><br>
>> > @@ -375,12 +554,12 @@<br>
>> ><br>
>> > PropagationMap.insert(PairType(Call, LPInfo));<br>
>> ><br>
>> > - } else if (LPInfo.IsVar && !RPInfo.IsVar) {<br>
>> > + } else if (LPInfo.isVar() && !RPInfo.isVar()) {<br>
>> > StateMap->setState(LPInfo.getVar(), RPInfo.getState());<br>
>> ><br>
>> > PropagationMap.insert(PairType(Call, LPInfo));<br>
>> ><br>
>> > - } else if (!LPInfo.IsVar && RPInfo.IsVar) {<br>
>> > + } else if (!LPInfo.isVar() && RPInfo.isVar()) {<br>
>> > PropagationMap.insert(PairType(Call,<br>
>> > PropagationInfo(StateMap->getState(RPInfo.getVar()))));<br>
>> ><br>
>> > @@ -395,7 +574,7 @@<br>
>> ><br>
>> > LPInfo = LEntry->second;<br>
>> ><br>
>> > - if (LPInfo.IsVar) {<br>
>> > + if (LPInfo.isVar()) {<br>
>> > StateMap->setState(LPInfo.getVar(), consumed::CS_Unknown);<br>
>> ><br>
>> > PropagationMap.insert(PairType(Call, LPInfo));<br>
>> > @@ -410,7 +589,7 @@<br>
>> ><br>
>> > RPInfo = REntry->second;<br>
>> ><br>
>> > - if (RPInfo.IsVar) {<br>
>> > + if (RPInfo.isVar()) {<br>
>> > const VarDecl *Var = RPInfo.getVar();<br>
>> ><br>
>> > PropagationMap.insert(PairType(Call,<br>
>> > @@ -434,14 +613,16 @@<br>
>> ><br>
>> > checkCallability(PInfo, FunDecl, Call);<br>
>> ><br>
>> > - if (PInfo.IsVar) {<br>
>> > - if (FunDecl->hasAttr<ConsumesAttr>()) {<br>
>> > - // Handle consuming operators.<br>
>> > + if (PInfo.isVar()) {<br>
>> > + if (isTestingFunction(FunDecl)) {<br>
>> > + handleTestingFunctionCall(Call, PInfo.getVar());<br>
>> > +<br>
>> > + } else if (FunDecl->hasAttr<ConsumesAttr>()) {<br>
>> > StateMap->setState(PInfo.getVar(), consumed::CS_Consumed);<br>
>> > +<br>
>> > } else if (const CXXMethodDecl *MethodDecl =<br>
>> > - dyn_cast_or_null<CXXMethodDecl>(FunDecl)) {<br>
>> > + dyn_cast_or_null<CXXMethodDecl>(FunDecl)) {<br>
>> ><br>
>> > - // Handle non-constant member operators.<br>
>> > if (!MethodDecl->isConst())<br>
>> > StateMap->setState(PInfo.getVar(), consumed::CS_Unknown);<br>
>> > }<br>
>> > @@ -482,11 +663,21 @@<br>
>> > }<br>
>> ><br>
>> > void ConsumedStmtVisitor::VisitUnaryOperator(const UnaryOperator *UOp)<br>
>> > {<br>
>> > - if (UOp->getOpcode() == UO_AddrOf) {<br>
>> > - InfoEntry Entry = PropagationMap.find(UOp->getSubExpr());<br>
>> > -<br>
>> > - if (Entry != PropagationMap.end())<br>
>> > - PropagationMap.insert(PairType(UOp, Entry->second));<br>
>> > + InfoEntry Entry =<br>
>> > PropagationMap.find(UOp->getSubExpr()->IgnoreParens());<br>
>> > + if (Entry == PropagationMap.end()) return;<br>
>> > +<br>
>> > + switch (UOp->getOpcode()) {<br>
>> > + case UO_AddrOf:<br>
>> > + PropagationMap.insert(PairType(UOp, Entry->second));<br>
>> > + break;<br>
>> > +<br>
>> > + case UO_LNot:<br>
>> > + if (Entry->second.isTest() || Entry->second.isBinTest())<br>
>> > + PropagationMap.insert(PairType(UOp, Entry->second.invertTest()));<br>
>> > + break;<br>
>> > +<br>
>> > + default:<br>
>> > + break;<br>
>> > }<br>
>> > }<br>
>> ><br>
>> > @@ -495,74 +686,97 @@<br>
>> > PropagationInfo PInfo =<br>
>> > PropagationMap.find(Var->getInit())->second;<br>
>> ><br>
>> > - StateMap->setState(Var, PInfo.IsVar ?<br>
>> > + StateMap->setState(Var, PInfo.isVar() ?<br>
>> > StateMap->getState(PInfo.getVar()) : PInfo.getState());<br>
>> > }<br>
>> > }<br>
>> > -} // end anonymous::ConsumedStmtVisitor<br>
>> > +}} // end clang::consumed::ConsumedStmtVisitor<br>
>><br>
>> Two curly braces on the same line.<br>
><br>
><br>
> I've seen this happen in other places in the Clang codebase. It seems<br>
> better than wasting a newline to me.<br>
<br>
</div></div>Newlines are cheap, but it's not a huge issue.<br>
<div class=""><div class="h5"><br>
><br>
>><br>
>><br>
>> ><br>
>> > -namespace {<br>
>> > +namespace clang {<br>
>> > +namespace consumed {<br>
>> ><br>
>> > -// TODO: Handle variable definitions, e.g. bool valid = x.isValid();<br>
>> > -// if (valid) ...; (Deferred)<br>
>> > -class TestedVarsVisitor : public RecursiveASTVisitor<TestedVarsVisitor><br>
>> > {<br>
>> > -<br>
>> > - bool Invert;<br>
>> > - SourceLocation CurrTestLoc;<br>
>> > -<br>
>> > - ConsumedStateMap *StateMap;<br>
>> > -<br>
>> > -public:<br>
>> > - bool IsUsefulConditional;<br>
>> > - VarTestResult Test;<br>
>> > +void splitVarStateForIf(const IfStmt * IfNode, const VarTestResult<br>
>> > &Test,<br>
>> > + ConsumedStateMap *ThenStates,<br>
>> > + ConsumedStateMap *ElseStates) {<br>
>> > +<br>
>> > + ConsumedState VarState = ThenStates->getState(Test.Var);<br>
>> ><br>
>> > - TestedVarsVisitor(ConsumedStateMap *StateMap) : Invert(false),<br>
>> > - StateMap(StateMap), IsUsefulConditional(false) {}<br>
>> > + if (VarState == CS_Unknown) {<br>
>> > + ThenStates->setState(Test.Var, Test.TestsFor);<br>
>> > + if (ElseStates)<br>
>> > + ElseStates->setState(Test.Var,<br>
>> > invertConsumedUnconsumed(Test.TestsFor));<br>
>> ><br>
>> > - bool VisitCallExpr(CallExpr *Call);<br>
>> > - bool VisitDeclRefExpr(DeclRefExpr *DeclRef);<br>
>> > - bool VisitUnaryOperator(UnaryOperator *UnaryOp);<br>
>> > -};<br>
>> > -<br>
>> > -bool TestedVarsVisitor::VisitCallExpr(CallExpr *Call) {<br>
>> > - if (const FunctionDecl *FunDecl =<br>
>> > - dyn_cast_or_null<FunctionDecl>(Call->getDirectCallee())) {<br>
>> > -<br>
>> > - if (isTestingFunction(FunDecl)) {<br>
>> > - CurrTestLoc = Call->getExprLoc();<br>
>> > - IsUsefulConditional = true;<br>
>> > - return true;<br>
>> > - }<br>
>> > + } else if (VarState == invertConsumedUnconsumed(Test.TestsFor)) {<br>
>> > + ThenStates->markUnreachable();<br>
>> ><br>
>> > - IsUsefulConditional = false;<br>
>> > + } else if (VarState == Test.TestsFor && ElseStates) {<br>
>> > + ElseStates->markUnreachable();<br>
>> > }<br>
>> > -<br>
>> > - return false;<br>
>> > }<br>
>> ><br>
>> > -bool TestedVarsVisitor::VisitDeclRefExpr(DeclRefExpr *DeclRef) {<br>
>> > - if (const VarDecl *Var =<br>
>> > dyn_cast_or_null<VarDecl>(DeclRef->getDecl()))<br>
>> > - if (StateMap->getState(Var) != consumed::CS_None)<br>
>> > - Test = VarTestResult(Var, CurrTestLoc, !Invert);<br>
>> > +void splitVarStateForIfBinOp(const PropagationInfo &PInfo,<br>
>> > + ConsumedStateMap *ThenStates, ConsumedStateMap *ElseStates) {<br>
>> ><br>
>> > - return true;<br>
>> > -}<br>
>> > -<br>
>> > -bool TestedVarsVisitor::VisitUnaryOperator(UnaryOperator *UnaryOp) {<br>
>> > - if (UnaryOp->getOpcode() == UO_LNot) {<br>
>> > - Invert = true;<br>
>> > - TraverseStmt(UnaryOp->getSubExpr());<br>
>> > -<br>
>> > - } else {<br>
>> > - IsUsefulConditional = false;<br>
>> > + const VarTestResult <est = PInfo.getLTest(),<br>
>> > + &RTest = PInfo.getRTest();<br>
>> > +<br>
>> > + ConsumedState LState = LTest.Var ? ThenStates->getState(LTest.Var) :<br>
>> > CS_None,<br>
>> > + RState = RTest.Var ? ThenStates->getState(RTest.Var) :<br>
>> > CS_None;<br>
>> > +<br>
>> > + if (LTest.Var) {<br>
>> > + if (PInfo.testEffectiveOp() == EO_And) {<br>
>> > + if (LState == CS_Unknown) {<br>
>> > + ThenStates->setState(LTest.Var, LTest.TestsFor);<br>
>> > +<br>
>> > + } else if (LState == invertConsumedUnconsumed(LTest.TestsFor)) {<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>
>> > + ThenStates->markUnreachable();<br>
>> > + }<br>
>> > + }<br>
>> > +<br>
>> > + } else {<br>
>> > + if (LState == CS_Unknown && ElseStates) {<br>
>> > + ElseStates->setState(LTest.Var,<br>
>> > + invertConsumedUnconsumed(LTest.TestsFor));<br>
>> > +<br>
>> > + } else if (LState == LTest.TestsFor && ElseStates) {<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>
>> > + ThenStates->markUnreachable();<br>
>> > + }<br>
>> > + }<br>
>> > + }<br>
>> > }<br>
>> ><br>
>> > - return false;<br>
>> > + if (RTest.Var) {<br>
>> > + if (PInfo.testEffectiveOp() == EO_And) {<br>
>> > + if (RState == CS_Unknown)<br>
>> > + ThenStates->setState(RTest.Var, RTest.TestsFor);<br>
>> > + else if (RState == invertConsumedUnconsumed(RTest.TestsFor))<br>
>> > + ThenStates->markUnreachable();<br>
>> > +<br>
>> > + } else if (ElseStates) {<br>
>> > + if (RState == CS_Unknown)<br>
>> > + ElseStates->setState(RTest.Var,<br>
>> > + invertConsumedUnconsumed(RTest.TestsFor));<br>
>> > + else if (RState == RTest.TestsFor)<br>
>> > + ElseStates->markUnreachable();<br>
>> > + }<br>
>> > + }<br>
>> > }<br>
>> > -} // end anonymouse::TestedVarsVisitor<br>
>> > -<br>
>> > -namespace clang {<br>
>> > -namespace consumed {<br>
>> ><br>
>> > void ConsumedBlockInfo::addInfo(const CFGBlock *Block,<br>
>> > ConsumedStateMap *StateMap,<br>
>> > @@ -625,29 +839,44 @@<br>
>> > void ConsumedStateMap::intersect(const ConsumedStateMap *Other) {<br>
>> > ConsumedState LocalState;<br>
>> ><br>
>> > + if (this->From && this->From == Other->From && !Other->Reachable) {<br>
>> > + this->markUnreachable();<br>
>> > + return;<br>
>> > + }<br>
>> > +<br>
>> > for (MapType::const_iterator DMI = Other->Map.begin(),<br>
>> > DME = Other->Map.end(); DMI != DME; ++DMI) {<br>
>> ><br>
>> > LocalState = this->getState(DMI->first);<br>
>> ><br>
>> > - if (LocalState != CS_None && LocalState != DMI->second)<br>
>> > - setState(DMI->first, CS_Unknown);<br>
>> > + if (LocalState == CS_None) continue;<br>
>><br>
>> Body should be on a new line.<br>
>><br>
>> > +<br>
>> > + if (LocalState != DMI->second)<br>
>> > + Map[DMI->first] = CS_Unknown;<br>
>> > }<br>
>> > }<br>
>> ><br>
>> > +bool ConsumedStateMap::isReachable() {<br>
>> > + return Reachable;<br>
>> > +}<br>
>><br>
>> Function should be const and inline.<br>
>><br>
>> > +<br>
>> > +void ConsumedStateMap::markUnreachable() {<br>
>> > + this->Reachable = false;<br>
>> > + Map.clear();<br>
>> > +}<br>
>> > +<br>
>> > void ConsumedStateMap::makeUnknown() {<br>
>> > - PairType Pair;<br>
>> > -<br>
>> > for (MapType::const_iterator DMI = Map.begin(), DME = Map.end(); DMI<br>
>> > != DME;<br>
>> > ++DMI) {<br>
>> ><br>
>> > - Pair = *DMI;<br>
>> > -<br>
>> > - Map.erase(Pair.first);<br>
>> > - Map.insert(PairType(Pair.first, CS_Unknown));<br>
>> > + Map[DMI->first] = CS_Unknown;<br>
>> > }<br>
>> > }<br>
>> ><br>
>> > +void ConsumedStateMap::setSource(const Stmt *Source) {<br>
>> > + this->From = Source;<br>
>> > +}<br>
>> > +<br>
>> > void ConsumedStateMap::setState(const VarDecl *Var, ConsumedState<br>
>> > State) {<br>
>> > Map[Var] = State;<br>
>> > }<br>
>> > @@ -694,42 +923,84 @@<br>
>> > return false;<br>
>> > }<br>
>> ><br>
>> > -// TODO: Handle other forms of branching with precision, including<br>
>> > while- and<br>
>> > -// for-loops. (Deferred)<br>
>> > -void ConsumedAnalyzer::splitState(const CFGBlock *CurrBlock,<br>
>> > - const IfStmt *Terminator) {<br>
>> > -<br>
>> > - TestedVarsVisitor Visitor(CurrStates);<br>
>> > - Visitor.TraverseStmt(const_cast<Expr*>(Terminator->getCond()));<br>
>> > -<br>
>> > - bool HasElse = Terminator->getElse() != NULL;<br>
>> > +bool ConsumedAnalyzer::splitState(const CFGBlock *CurrBlock,<br>
>> > + const ConsumedStmtVisitor &Visitor) {<br>
>><br>
>> Formatting<br>
><br>
><br>
> Non-fixed-width font issue.<br>
><br>
>><br>
>><br>
>> ><br>
>> > - ConsumedStateMap *ElseOrMergeStates = new<br>
>> > ConsumedStateMap(*CurrStates);<br>
>> > + ConsumedStateMap *FalseStates = new ConsumedStateMap(*CurrStates);<br>
>><br>
>> I am still concerted about the leaks with naked pointers.<br>
>><br>
>> > + PropagationInfo PInfo;<br>
>> ><br>
>> > - if (Visitor.IsUsefulConditional) {<br>
>> > - ConsumedState VarState = CurrStates->getState(Visitor.Test.Var);<br>
>> > + if (const IfStmt *IfNode =<br>
>> > + dyn_cast_or_null<IfStmt>(CurrBlock->getTerminator().getStmt())) {<br>
>> ><br>
>> > - if (VarState != CS_Unknown) {<br>
>> > - // FIXME: Make this not warn if the test is from a macro<br>
>> > expansion.<br>
>> > - // (Deferred)<br>
>> > -<br>
>> > WarningsHandler.warnUnnecessaryTest(Visitor.Test.Var->getNameAsString(),<br>
>> > - stateToString(VarState), Visitor.Test.Loc);<br>
>> > - }<br>
>> > + bool HasElse = IfNode->getElse() != NULL;<br>
>> > + const Stmt *Cond = IfNode->getCond();<br>
>> > +<br>
>> > + PInfo = Visitor.getInfo(Cond);<br>
>> > + if (!PInfo && isa<BinaryOperator>(Cond))<br>
>> > + PInfo = Visitor.getInfo(cast<BinaryOperator>(Cond)->getRHS());<br>
>> ><br>
>> > - if (Visitor.Test.UnconsumedInTrueBranch) {<br>
>> > - CurrStates->setState(Visitor.Test.Var, CS_Unconsumed);<br>
>> > - if (HasElse) ElseOrMergeStates->setState(Visitor.Test.Var,<br>
>> > CS_Consumed);<br>
>> > + if (PInfo.isTest()) {<br>
>> > + CurrStates->setSource(Cond);<br>
>> > + FalseStates->setSource(Cond);<br>
>> > + splitVarStateForIf(IfNode, PInfo.getTest(), CurrStates,<br>
>> > + HasElse ? FalseStates : NULL);<br>
>> > +<br>
>> > + } else if (PInfo.isBinTest()) {<br>
>> > + CurrStates->setSource(PInfo.testSourceNode());<br>
>> > + FalseStates->setSource(PInfo.testSourceNode());<br>
>> > + splitVarStateForIfBinOp(PInfo, CurrStates, HasElse ? FalseStates<br>
>> > :NULL);<br>
>> ><br>
>> > } else {<br>
>> > - CurrStates->setState(Visitor.Test.Var, CS_Consumed);<br>
>> > - if (HasElse) ElseOrMergeStates->setState(Visitor.Test.Var,<br>
>> > CS_Unconsumed);<br>
>> > + return false;<br>
>> > + }<br>
>> > +<br>
>> > + } else if (const BinaryOperator *BinOp =<br>
>> > +<br>
>> > dyn_cast_or_null<BinaryOperator>(CurrBlock->getTerminator().getStmt())) {<br>
>> > +<br>
>> > + PInfo = Visitor.getInfo(BinOp->getLHS());<br>
>> > + if (!PInfo.isTest()) {<br>
>> > + if ((BinOp = dyn_cast_or_null<BinaryOperator>(BinOp->getLHS())))<br>
>> > {<br>
>> > + PInfo = Visitor.getInfo(BinOp->getRHS());<br>
>> > +<br>
>> > + if (!PInfo.isTest())<br>
>> > + return false;<br>
>> > +<br>
>> > + } else {<br>
>> > + return false;<br>
>> > + }<br>
>> > + }<br>
>> > +<br>
>> > + CurrStates->setSource(BinOp);<br>
>> > + FalseStates->setSource(BinOp);<br>
>> > +<br>
>> > + const VarTestResult &Test = PInfo.getTest();<br>
>> > + ConsumedState VarState = CurrStates->getState(Test.Var);<br>
>> > +<br>
>> > + if (BinOp->getOpcode() == BO_LAnd) {<br>
>> > + if (VarState == CS_Unknown)<br>
>> > + CurrStates->setState(Test.Var, Test.TestsFor);<br>
>> > + else if (VarState == invertConsumedUnconsumed(Test.TestsFor))<br>
>> > + CurrStates->markUnreachable();<br>
>> > +<br>
>> > + } else if (BinOp->getOpcode() == BO_LOr) {<br>
>> > + if (VarState == CS_Unknown)<br>
>> > + FalseStates->setState(Test.Var,<br>
>> > + invertConsumedUnconsumed(Test.TestsFor));<br>
>> > + else if (VarState == Test.TestsFor)<br>
>> > + FalseStates->markUnreachable();<br>
>> > }<br>
>> > - }<br>
>> ><br>
>> > + } else {<br>
>> > + return false;<br>
>> > + }<br>
>> > +<br>
>> > CFGBlock::const_succ_iterator SI = CurrBlock->succ_begin();<br>
>> ><br>
>> > - if (*SI) BlockInfo.addInfo(*SI, CurrStates);<br>
>> > - if (*++SI) BlockInfo.addInfo(*SI, ElseOrMergeStates);<br>
>> > + if (*SI) BlockInfo.addInfo(*SI, CurrStates);<br>
>> > + if (*++SI) BlockInfo.addInfo(*SI, FalseStates);<br>
>> > +<br>
>> > + CurrStates = NULL;<br>
>> > + return true;<br>
>> > }<br>
>> ><br>
>> > void ConsumedAnalyzer::run(AnalysisDeclContext &AC) {<br>
>> > @@ -742,6 +1013,7 @@<br>
>> > PostOrderCFGView *SortedGraph = AC.getAnalysis<PostOrderCFGView>();<br>
>> ><br>
>> > CurrStates = new ConsumedStateMap();<br>
>> > + ConsumedStmtVisitor Visitor(AC, *this);<br>
>> ><br>
>> > // Visit all of the function's basic blocks.<br>
>> > for (PostOrderCFGView::iterator I = SortedGraph->begin(),<br>
>> > @@ -752,9 +1024,19 @@<br>
>> ><br>
>> > if (CurrStates == NULL)<br>
>> > CurrStates = BlockInfo.getInfo(CurrBlock);<br>
>> > -<br>
>> > - ConsumedStmtVisitor Visitor(AC, *this, CurrStates);<br>
>> > -<br>
>> > +<br>
>> > + if (!CurrStates) {<br>
>> > + CurrStates = NULL;<br>
>> > + continue;<br>
>> > +<br>
>> > + } else if (!CurrStates->isReachable()) {<br>
>> > + delete CurrStates;<br>
>> > + CurrStates = NULL;<br>
>> > + continue;<br>
>> > + }<br>
>> > +<br>
>> > + Visitor.reset(CurrStates);<br>
>> > +<br>
>> > // Visit all of the basic block's statements.<br>
>> > for (CFGBlock::const_iterator BI = CurrBlock->begin(),<br>
>> > BE = CurrBlock->end(); BI != BE; ++BI) {<br>
>> > @@ -770,36 +1052,34 @@<br>
>> > }<br>
>> > }<br>
>> ><br>
>> > - if (const IfStmt *Terminator =<br>
>> > - dyn_cast_or_null<IfStmt>(CurrBlock->getTerminator().getStmt())) {<br>
>> > + // TODO: Handle other forms of branching with precision, including<br>
>> > while-<br>
>> > + // and for-loops. (Deferred)<br>
>> > + if (!splitState(CurrBlock, Visitor)) {<br>
>> > + CurrStates->setSource(NULL);<br>
>> ><br>
>> > - splitState(CurrBlock, Terminator);<br>
>> > - CurrStates = NULL;<br>
>> > -<br>
>> > - } else if (CurrBlock->succ_size() > 1) {<br>
>> > - CurrStates->makeUnknown();<br>
>> > -<br>
>> > - bool OwnershipTaken = false;<br>
>> > -<br>
>> > - for (CFGBlock::const_succ_iterator SI = CurrBlock->succ_begin(),<br>
>> > - SE = CurrBlock->succ_end(); SI != SE; ++SI) {<br>
>> > + if (CurrBlock->succ_size() > 1) {<br>
>> > + CurrStates->makeUnknown();<br>
>> > +<br>
>> > + bool OwnershipTaken = false;<br>
>> ><br>
>> > - if (*SI) BlockInfo.addInfo(*SI, CurrStates, OwnershipTaken);<br>
>> > + for (CFGBlock::const_succ_iterator SI =<br>
>> > CurrBlock->succ_begin(),<br>
>> > + SE = CurrBlock->succ_end(); SI != SE; ++SI) {<br>
>> > +<br>
>> > + if (*SI) BlockInfo.addInfo(*SI, CurrStates, OwnershipTaken);<br>
>> > + }<br>
>> > +<br>
>> > + if (!OwnershipTaken)<br>
>> > + delete CurrStates;<br>
>> > +<br>
>> > + CurrStates = NULL;<br>
>> > +<br>
>> > + } else if (CurrBlock->succ_size() == 1 &&<br>
>> > + (*CurrBlock->succ_begin())->pred_size() > 1) {<br>
>> > +<br>
>> > + BlockInfo.addInfo(*CurrBlock->succ_begin(), CurrStates);<br>
>> > + CurrStates = NULL;<br>
>> > }<br>
>> > -<br>
>> > - if (!OwnershipTaken)<br>
>> > - delete CurrStates;<br>
>> > -<br>
>> > - CurrStates = NULL;<br>
>> > -<br>
>> > - } else if (CurrBlock->succ_size() == 1 &&<br>
>> > - (*CurrBlock->succ_begin())->pred_size() > 1) {<br>
>> > -<br>
>> > - BlockInfo.addInfo(*CurrBlock->succ_begin(), CurrStates);<br>
>> > - CurrStates = NULL;<br>
>> > }<br>
>> > -<br>
>> > - Visitor.reset();<br>
>> > } // End of block iterator.<br>
>> ><br>
>> > // Delete the last existing state map.<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>
>> > @@ -4,6 +4,8 @@<br>
>> > #define CONSUMES __attribute__ ((consumes))<br>
>> > #define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed))<br>
>> ><br>
>> > +#define TEST_VAR(Var) Var.isValid()<br>
>> > +<br>
>> > typedef decltype(nullptr) nullptr_t;<br>
>> ><br>
>> > template <typename T><br>
>> > @@ -11,7 +13,7 @@<br>
>> > T var;<br>
>> ><br>
>> > public:<br>
>> > - ConsumableClass(void);<br>
>> > + ConsumableClass();<br>
>> > ConsumableClass(T val);<br>
>> > ConsumableClass(ConsumableClass<T> &other);<br>
>> > ConsumableClass(ConsumableClass<T> &&other);<br>
>> > @@ -26,20 +28,24 @@<br>
>> > template <typename U><br>
>> > ConsumableClass<T>& operator=(ConsumableClass<U> &&other);<br>
>> ><br>
>> > - void operator*(void) const CALLABLE_WHEN_UNCONSUMED;<br>
>> > + void operator()(int a) CONSUMES;<br>
>> > + void operator*() const CALLABLE_WHEN_UNCONSUMED;<br>
>> > + void unconsumedCall() const CALLABLE_WHEN_UNCONSUMED;<br>
>> ><br>
>> > - bool isValid(void) const TESTS_UNCONSUMED;<br>
>> > + bool isValid() const TESTS_UNCONSUMED;<br>
>> > + operator bool() const TESTS_UNCONSUMED;<br>
>> > + bool operator!=(nullptr_t) const TESTS_UNCONSUMED;<br>
>> ><br>
>> > - void constCall(void) const;<br>
>> > - void nonconstCall(void);<br>
>> > + void constCall() const;<br>
>> > + void nonconstCall();<br>
>> ><br>
>> > - void consume(void) CONSUMES;<br>
>> > + void consume() CONSUMES;<br>
>> > };<br>
>> ><br>
>> > void baf0(ConsumableClass<int> &var);<br>
>> > void baf1(ConsumableClass<int> *var);<br>
>> ><br>
>> > -void testIfStmt(void) {<br>
>> > +void testIfStmt() {<br>
>> > ConsumableClass<int> var;<br>
>> ><br>
>> > if (var.isValid()) { // expected-warning {{unnecessary test. Variable<br>
>> > 'var' is known to be in the 'consumed' state}}<br>
>> > @@ -51,27 +57,113 @@<br>
>> > }<br>
>> > }<br>
>> ><br>
>> > -void testConditionalMerge(void) {<br>
>> > - ConsumableClass<int> var;<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 (var.isValid()) {// expected-warning {{unnecessary test. Variable<br>
>> > 'var' is known to be in the 'consumed' state}}<br>
>> > + if (var0 && var1) {<br>
>> > + *var0;<br>
>> > + *var1;<br>
>> ><br>
>> > - // Empty<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in an unknown state}}<br>
>> > }<br>
>> ><br>
>> > - *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in an unknown state}}<br>
>> > + if (var0 || var1) {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in an unknown state}}<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > + }<br>
>> ><br>
>> > - if (var.isValid()) {<br>
>> > - // Empty<br>
>> > + if (var0 && !var1) {<br>
>> > + *var0;<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> ><br>
>> > } else {<br>
>> > - // Empty<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in an unknown state}}<br>
>> > }<br>
>> ><br>
>> > - *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in an unknown state}}<br>
>> > + if (var0 || !var1) {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in an unknown state}}<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > 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<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in an unknown state}}<br>
>> > + }<br>
>> > +<br>
>> > + if (!(var0 || var1)) {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in an unknown state}}<br>
>> > + }<br>
>> > +<br>
>> > + if (!var0 || !var1) {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > 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<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > 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<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in an unknown state}}<br>
>> > + *var2; // expected-warning {{invocation of method 'operator*' on<br>
>> > 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<br>
>> > object 'var0' while it is in an unknown state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in an unknown state}}<br>
>> > + *var2; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var2' while it is in an unknown state}}<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > + *var2; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var2' while it is in the 'consumed' state}}<br>
>> > + }<br>
>> > +#endif<br>
>> > }<br>
>> ><br>
>> > -void testCallingConventions(void) {<br>
>> > +void testCallingConventions() {<br>
>> > ConsumableClass<int> var(42);<br>
>> ><br>
>> > baf0(var);<br>
>> > @@ -82,14 +174,14 @@<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in an unknown state}}<br>
>> > }<br>
>> ><br>
>> > -void testMoveAsignmentish(void) {<br>
>> > +void testMoveAsignmentish() {<br>
>> > ConsumableClass<int> var;<br>
>> ><br>
>> > var = nullptr;<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in an unknown state}}<br>
>> > }<br>
>> ><br>
>> > -void testConstAndNonConstMemberFunctions(void) {<br>
>> > +void testConstAndNonConstMemberFunctions() {<br>
>> > ConsumableClass<int> var(42);<br>
>> ><br>
>> > var.constCall();<br>
>> > @@ -99,7 +191,15 @@<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in an unknown state}}<br>
>> > }<br>
>> ><br>
>> > -void testSimpleForLoop(void) {<br>
>> > +void testNoWarnTestFromMacroExpansion() {<br>
>> > + ConsumableClass<int> var(42);<br>
>> > +<br>
>> > + if (TEST_VAR(var)) {<br>
>> > + *var;<br>
>> > + }<br>
>> > +}<br>
>> > +<br>
>> > +void testSimpleForLoop() {<br>
>> > ConsumableClass<int> var;<br>
>> ><br>
>> > for (int i = 0; i < 10; ++i) {<br>
>> > @@ -109,7 +209,7 @@<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in an unknown state}}<br>
>> > }<br>
>> ><br>
>> > -void testSimpleWhileLoop(void) {<br>
>> > +void testSimpleWhileLoop() {<br>
>> > int i = 0;<br>
>> ><br>
>> > ConsumableClass<int> var;<br>
>> > Index: test/SemaCXX/warn-consumed-analysis.cpp<br>
>> > ===================================================================<br>
>> > --- test/SemaCXX/warn-consumed-analysis.cpp<br>
>> > +++ test/SemaCXX/warn-consumed-analysis.cpp<br>
>> > @@ -11,7 +11,7 @@<br>
>> > T var;<br>
>> ><br>
>> > public:<br>
>> > - ConsumableClass(void);<br>
>> > + ConsumableClass();<br>
>> > ConsumableClass(nullptr_t p) CONSUMES;<br>
>> > ConsumableClass(T val);<br>
>> > ConsumableClass(ConsumableClass<T> &other);<br>
>> > @@ -28,17 +28,17 @@<br>
>> > ConsumableClass<T>& operator=(ConsumableClass<U> &&other);<br>
>> ><br>
>> > void operator()(int a) CONSUMES;<br>
>> > - void operator*(void) const CALLABLE_WHEN_UNCONSUMED;<br>
>> > - void unconsumedCall(void) const CALLABLE_WHEN_UNCONSUMED;<br>
>> > + void operator*() const CALLABLE_WHEN_UNCONSUMED;<br>
>> > + void unconsumedCall() const CALLABLE_WHEN_UNCONSUMED;<br>
>> ><br>
>> > - bool isValid(void) const TESTS_UNCONSUMED;<br>
>> > + bool isValid() const TESTS_UNCONSUMED;<br>
>> > operator bool() const TESTS_UNCONSUMED;<br>
>> > bool operator!=(nullptr_t) const TESTS_UNCONSUMED;<br>
>> ><br>
>> > - void constCall(void) const;<br>
>> > - void nonconstCall(void);<br>
>> > + void constCall() const;<br>
>> > + void nonconstCall();<br>
>> ><br>
>> > - void consume(void) CONSUMES;<br>
>> > + void consume() CONSUMES;<br>
>> > };<br>
>> ><br>
>> > void baf0(const ConsumableClass<int> var);<br>
>> > @@ -47,7 +47,7 @@<br>
>> ><br>
>> > void baf3(ConsumableClass<int> &&var);<br>
>> ><br>
>> > -void testInitialization(void) {<br>
>> > +void testInitialization() {<br>
>> > ConsumableClass<int> var0;<br>
>> > ConsumableClass<int> var1 = ConsumableClass<int>();<br>
>> ><br>
>> > @@ -58,10 +58,10 @@<br>
>> ><br>
>> > if (var0.isValid()) {<br>
>> > *var0;<br>
>> > - *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > + *var1;<br>
>> ><br>
>> > } else {<br>
>> > - *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > }<br>
>> > }<br>
>> ><br>
>> > @@ -69,7 +69,7 @@<br>
>> > *ConsumableClass<int>(); // expected-warning {{invocation of method<br>
>> > 'operator*' on a temporary object while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > -void testSimpleRValueRefs(void) {<br>
>> > +void testSimpleRValueRefs() {<br>
>> > ConsumableClass<int> var0;<br>
>> > ConsumableClass<int> var1(42);<br>
>> ><br>
>> > @@ -82,39 +82,149 @@<br>
>> > *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > -void testIfStmt(void) {<br>
>> > +void testIfStmt() {<br>
>> > ConsumableClass<int> var;<br>
>> ><br>
>> > if (var.isValid()) {<br>
>> > - // Empty<br>
>> > -<br>
>> > + *var;<br>
>> > } else {<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > if (!var.isValid()) {<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in the 'consumed' state}}<br>
>> > -<br>
>> > } else {<br>
>> > *var;<br>
>> > }<br>
>> ><br>
>> > if (var) {<br>
>> > // Empty<br>
>> > -<br>
>> > } else {<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > if (var != nullptr) {<br>
>> > // Empty<br>
>> > -<br>
>> > } else {<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in the 'consumed' state}}<br>
>> > }<br>
>> > }<br>
>> ><br>
>> > -void testCallingConventions(void) {<br>
>> > +void testComplexConditionals() {<br>
>> > + ConsumableClass<int> var0, var1, var2;<br>
>> > +<br>
>> > + if (var0 && var1) {<br>
>> > + *var0;<br>
>> > + *var1;<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > + }<br>
>> > +<br>
>> > + if (var0 || var1) {<br>
>> > + *var0;<br>
>> > + *var1;<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > + }<br>
>> > +<br>
>> > + if (var0 && !var1) {<br>
>> > + *var0;<br>
>> > + *var1;<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > + }<br>
>> > +<br>
>> > + if (var0 || !var1) {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0;<br>
>> > + *var1;<br>
>> > + }<br>
>> > +<br>
>> > + if (!var0 && !var1) {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0;<br>
>> > + *var1;<br>
>> > + }<br>
>> > +<br>
>> > + if (!var0 || !var1) {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0;<br>
>> > + *var1;<br>
>> > + }<br>
>> > +<br>
>> > + if (!(var0 && var1)) {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0;<br>
>> > + *var1;<br>
>> > + }<br>
>> > +<br>
>> > + if (!(var0 || var1)) {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' 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<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > + *var2; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var2' while it is in the 'consumed' state}}<br>
>> > + }<br>
>> > +<br>
>> > +#if 0<br>
>> > + // FIXME: Get this test to pass.<br>
>> > + if (var0 || var1 || var2) {<br>
>> > + *var0;<br>
>> > + *var1;<br>
>> > + *var2;<br>
>> > +<br>
>> > + } else {<br>
>> > + *var0; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var0' while it is in the 'consumed' state}}<br>
>> > + *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > + *var2; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var2' while it is in the 'consumed' state}}<br>
>> > + }<br>
>> > +#endif<br>
>> > +}<br>
>> > +<br>
>> > +void testStateChangeInBranch() {<br>
>> > + ConsumableClass<int> var;<br>
>> > +<br>
>> > + // Make var enter the 'unknown' state.<br>
>> > + baf1(var);<br>
>> > +<br>
>> > + if (!var) {<br>
>> > + var = ConsumableClass<int>(42);<br>
>> > + }<br>
>> > +<br>
>> > + *var;<br>
>> > +}<br>
>> > +<br>
>> > +void testCallingConventions() {<br>
>> > ConsumableClass<int> var(42);<br>
>> ><br>
>> > baf0(var);<br>
>> > @@ -130,7 +240,7 @@<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > -void testMoveAsignmentish(void) {<br>
>> > +void testMoveAsignmentish() {<br>
>> > ConsumableClass<int> var0;<br>
>> > ConsumableClass<long> var1(42);<br>
>> ><br>
>> > @@ -143,26 +253,25 @@<br>
>> > *var1; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var1' while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > -void testConditionalMerge(void) {<br>
>> > +void testConditionalMerge() {<br>
>> > ConsumableClass<int> var;<br>
>> ><br>
>> > if (var.isValid()) {<br>
>> > // Empty<br>
>> > }<br>
>> ><br>
>> > - *var;<br>
>> > + *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in the 'consumed' state}}<br>
>> ><br>
>> > if (var.isValid()) {<br>
>> > // Empty<br>
>> > -<br>
>> > } else {<br>
>> > // Empty<br>
>> > }<br>
>> ><br>
>> > - *var;<br>
>> > + *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > -void testConsumes0(void) {<br>
>> > +void testConsumes0() {<br>
>> > ConsumableClass<int> var(42);<br>
>> ><br>
>> > *var;<br>
>> > @@ -172,13 +281,13 @@<br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > -void testConsumes1(void) {<br>
>> > +void testConsumes1() {<br>
>> > ConsumableClass<int> var(nullptr);<br>
>> ><br>
>> > *var; // expected-warning {{invocation of method 'operator*' on<br>
>> > object 'var' while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > -void testConsumes2(void) {<br>
>> > +void testConsumes2() {<br>
>> > ConsumableClass<int> var(42);<br>
>> ><br>
>> > var.unconsumedCall();<br>
>> > @@ -187,7 +296,19 @@<br>
>> > var.unconsumedCall(); // expected-warning {{invocation of method<br>
>> > 'unconsumedCall' on object 'var' while it is in the 'consumed' state}}<br>
>> > }<br>
>> ><br>
>> > -void testSimpleForLoop(void) {<br>
>> > +void testNonsenseState() {<br>
>> > + ConsumableClass<int> var(42);<br>
>> > +<br>
>> > + if (var) {<br>
>> > + *var;<br>
>> > + } else {<br>
>> > + *var;<br>
>> > + }<br>
>> > +<br>
>> > + *var;<br>
>> > +}<br>
>> > +<br>
>> > +void testSimpleForLoop() {<br>
>> > ConsumableClass<int> var;<br>
>> ><br>
>> > for (int i = 0; i < 10; ++i) {<br>
>> > @@ -197,7 +318,7 @@<br>
>> > *var;<br>
>> > }<br>
>> ><br>
>> > -void testSimpleWhileLoop(void) {<br>
>> > +void testSimpleWhileLoop() {<br>
>> > int i = 0;<br>
>> ><br>
>> > ConsumableClass<int> var;<br>
>> ><br>
>><br>
>> On Fri, Aug 23, 2013 at 8:58 PM, Christian Wailes<br>
>> <<a href="mailto:chriswailes@google.com">chriswailes@google.com</a>> wrote:<br>
>> ><br>
>> > This depends on patch <a href="http://llvm-reviews.chandlerc.com/D1500" target="_blank">http://llvm-reviews.chandlerc.com/D1500</a><br>
>> ><br>
>> > <a href="http://llvm-reviews.chandlerc.com/D1501" target="_blank">http://llvm-reviews.chandlerc.com/D1501</a><br>
><br>
><br>
<br>
</div></div><span class=""><font color="#888888">~Aaron<br>
</font></span></blockquote></div><br></div></div></div></div></div>