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

[Jessica Paquette via Phabricator via llvm-commits]

paquette abandoned this revision.
paquette added a comment.

Please see updated revision at: https://reviews.llvm.org/D26872



================
Comment at: include/llvm/ADT/TerminatedString.h:38
+
+template <typename C> struct Character {
+  bool IsTerminator; /// True if the current character is a terminator.
----------------
MatzeB wrote:
> Do you expect this to be instantiated with various types? Would avoiding the template and hardcoding something like uint32_t make sense? (I didn't look at the using code yet).
The idea was to eventually be able to perform outlining on higher level structures. It made sense to have a general character class which would facilitate general strings.

Since it hasn't been used yet, it might be better to just specialize the entire thing for unsigned integers and bring back templatization if it proves to be necessary. In that case, it would be possible to just specialize the suffix tree for, say, vectors or arrays of unsigned ints and bypass the terminated string entirely.


================
Comment at: include/llvm/ADT/TerminatedString.h:179
+  size_t length() const { return StrContainer.size() - 1; }
+  CharLike getTerminator() const { return StrContainer.back(); }
+
----------------
MatzeB wrote:
> why not call it back() to keep in line with STL containers?
I feel like back() would imply that the terminator is a proper part of the string. I'd expect back() to return the last character in the string I inserted, not the terminator. (However, a back() function would make sense to have)


Repository:
  rL LLVM

https://reviews.llvm.org/D26870