[PATCH] Branch handling cleanup in Consumed analysis

[David Blaikie]

================
Comment at: include/clang/Analysis/Analyses/Consumed.h:95
@@ -92,3 +94,3 @@
     // No state information for the given variable.
-    CS_None,
+    CS_None = 0,
     
----------------
any particular reason to number these?

================
Comment at: lib/Analysis/Consumed.cpp:46
@@ -43,1 +45,3 @@
 
+#define INVERT_CONSUMED_UNCONSUMED(State) static_cast<ConsumedState>(State ^ 1)
+
----------------
I guess it was to allow you to write this?

This should just be a function (we don't much like macros) & I don't think it benefits all that much from the ability to bit twiddle it into inversion - I'd just write it out in a (file-local static) free function with conditional statements or conditional operators.

================
Comment at: lib/Analysis/Consumed.cpp:93
@@ -73,4 +92,3 @@
   
-  union PropagationUnion {
-    ConsumedState State;
-    const VarDecl *Var;
+  enum ConstantInfo {
+    CV_None,
----------------
unclear what this type represents - maybe renaming the values and/or adding comments would help

================
Comment at: lib/Analysis/Consumed.cpp:117
@@ +116,3 @@
+      IT_Var
+    } InfoType;
+    
----------------
should this really be an unencapsulated, modifiable public member?

================
Comment at: lib/Analysis/Consumed.cpp:135
@@ -96,2 +134,3 @@
     
-    ConsumedState getState() const { return StateOrVar.State; }
+    const ConsumedState & getState() const { return InfoUnion.State; }
+    const RangeInfo     & getRange() const { return InfoUnion.Range; }
----------------
Are these intended to change the discriminated union state to the chosen one, or is there meant to be an invariant that you only call getState when isState, getRange when isRange, etc?

================
Comment at: lib/Analysis/Consumed.cpp:189
@@ +188,3 @@
+                            Entry->second.getRange().End);
+    } else {
+      return std::make_pair(0, 0);
----------------
"else after return" (drop the else, just have the second return be unconditional)
and braces on single-statement conditionals - you can drop the braces (this isn't hard-and-fast in this case, some people allow/prefer/accept braces on multiline, single-statement blocks in LLVM)


http://llvm-reviews.chandlerc.com/D1480