[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