[PATCH] D37591: [LVI] Move LVILatticeVal class to separate header file (NFC).

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 25 05:04:55 PDT 2017


sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

LGTM with nits



================
Comment at: include/llvm/Analysis/ValueLattice.h:27
+namespace llvm {
+class ValueLatticeVal {
+  enum ValueLatticeValueTy {
----------------
Let's call this `ValueLatticeElement`.  It is a bit verbose, but I think it is to the point, and with `auto` the more verbose type name probably won't bite us as much.


================
Comment at: include/llvm/Analysis/ValueLattice.h:176
+public:
+  /// Merge the specified lattice value into this one, updating this
+  /// one and returning true if anything changed.
----------------
Please elaborate on what "Merge" means in this context.


================
Comment at: include/llvm/Analysis/ValueLattice.h:244
+
+raw_ostream &operator<<(raw_ostream &OS,
+                        const ValueLatticeVal &Val) LLVM_ATTRIBUTE_USED;
----------------
Is the `LLVM_ATTRIBUTE_USED` still needed?


https://reviews.llvm.org/D37591





More information about the llvm-commits mailing list