This is an archive of the discontinued LLVM Phabricator instance.

Move SuffixTree type to a common location
Needs RevisionPublic

Authored by trixirt on Aug 23 2018, 8:37 PM.

Details

Summary

Following up on earlier IR Outliner discussion
http://lists.llvm.org/pipermail/llvm-dev/2017-September/117814.html

The general approach is to reuse the MachineOutliner.
So this change addresses

  1. SuffixTree or SuffixArray.

Picking SuffixTree.
Using SuffixTree generally, requires it move to a common location

Diff Detail

Repository
rL LLVM

Event Timeline

trixirt created this revision.Aug 23 2018, 8:37 PM
Bigcheese requested changes to this revision.Sep 7 2018, 2:16 PM
Bigcheese added a subscriber: Bigcheese.
Bigcheese added inline comments.
include/llvm/ADT/SuffixTree.h
20

You shouldn't use anonymous namespaces in headers as it means each TU that includes it gets a different copy of the code. This should be namespace llvm.

23

This was fine in the cpp file, but shouldn't be added to the global or llvm namespace in a header. It should probably be moved into SuffixTree or SuffixTreeNode.

294

This function body should be moved out to its own .cpp file. The general rule is any definition of a non-template function of more than a few statements should be in a .cpp file instead of the header.

This revision now requires changes to proceed.Sep 7 2018, 2:16 PM