[PATCH] D26870: Outliner: Add uniquely terminated strings for a suffix tree

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 18 14:28:39 PST 2016


davide added inline comments.


================
Comment at: include/llvm/ADT/TerminatedString.h:48-56
+    bool IsEqual = false;
+
+    if (IsTerminator && Rhs.IsTerminator)
+      IsEqual = (Id == Rhs.Id);
+
+    else if (!IsTerminator && !Rhs.IsTerminator)
+      IsEqual = (Symbol == Rhs.Symbol);
----------------
```
     if (IsTerminator && Rhs.IsTerminator)
      return Id == Rhs.Id; 
     if () 
       [...]
     return false
``` 
maybe shorter?


================
Comment at: include/llvm/ADT/TerminatedString.h:93-96
+    if (IsTerminator)
+      return true;
+
+    return (Symbol < Rhs);
----------------
I'd use 
```return IsTerminator || (Symbol < Rhs);```
here and in cases where you have a similar pattern.


================
Comment at: include/llvm/ADT/TerminatedString.h:125-128
+    if (IsTerminator)
+      return false;
+
+    return (Symbol > Rhs);
----------------
like here.


================
Comment at: include/llvm/ADT/TerminatedString.h:175-178
+  size_t size() const { return StrContainer.size(); }
+  size_t size() { return StrContainer.size(); }
+  size_t length() { return StrContainer.size() - 1; }
+  size_t length() const { return StrContainer.size() - 1; }
----------------
do you need both `const` and `non-const` variant of `size()` and `length()`?


================
Comment at: include/llvm/ADT/TerminatedString.h:327-328
+  TerminatedString(const InputContainer &C) {
+    for (auto it = C.begin(); it != C.end(); it++) {
+      CharLike Ch;
+      Ch.Symbol = *it;
----------------
same here, can you use a range-loop?


Repository:
  rL LLVM

https://reviews.llvm.org/D26870





More information about the llvm-commits mailing list