[PATCH] D26871: Outliner: Add a suffix tree type for the outliner

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 19 07:43:33 PST 2016


fhahn added a comment.

Added a few minor stylistic comments.



================
Comment at: include/llvm/ADT/SuffixTree.h:93
+  void deleteNode(Node *N) {
+    if (!N)
+      return;
----------------
I'd try to keep the nullptr checks consistent. Here you're using !N, whereas further at other places in this file you use `ChildPair.second != nullptr` instead of just `ChildPair.second`.


================
Comment at: unittests/ADT/SuffixTreeTest.cpp:21
+protected:
+  STree ST;
+
----------------
Does this have to be a member variable of SuffixTreeTest? As far as I can see it's only used in EmptyTreeTest and it looks like it could just be a local variable there.


================
Comment at: unittests/ADT/SuffixTreeTest.cpp:58
+  unsigned StrSize;
+  STree testTree = initTree(StrSize);
+
----------------
You use `testTree` as variable name here, but in FindOccurrencesTest you use `TestTree`.


Repository:
  rL LLVM

https://reviews.llvm.org/D26871





More information about the llvm-commits mailing list