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

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 23 00:54:08 PST 2016


silvas added a comment.

High level concern: if this is just going to be used by one pass currently (with no anticipated other users), it might be best to just keep it as a local utility (similar to e.g. lib/Transforms/Instrumentation/MaximumSpanningTree.h).

Putting something in ADT generally has a higher bar than putting things as a local helper, since the expectation is that the code is generic-ish, which can require a lot of work and make the code much more complicated (e.g. all the iterator boilerplate and the boilerplate to forwarding a bunch of methods to an underlying std::vector's methods). Putting in something generic-ish that doesn't have multiple users out of the gate can also result in a lot of dead code. Adding in just what you need for the outliner as a local utility and then promoting/genericizing to ADT later if necessary avoids this.

Also, by keeping it as a local utility you can avoid all the API design architecture astronauting that will inevitably ensue when trying to come up with a generic utility.


Repository:
  rL LLVM

https://reviews.llvm.org/D26870





More information about the llvm-commits mailing list