================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
