[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