[llvm-commits] [llvm] r44851 - /llvm/trunk/include/llvm/ADT/Trie.h

[Anton Korobeynikov]

Hello, Wojciech.

> The Parent field seems to be unused internally by the Trie
> implementation. Assuming the Edge interface is invisible for Trie
> clients, may this field be removed?
The whole Edge is not needed actually. See a list of FIXMEs in the
beginning :)

> I'm not familiar what Trie structure will be used for, but shouldn't
> this method be private? Currently, it exposes a way to call most of the
> Node's (and indirectly Edge's) methods, which I suppose, should be
> inaccessible to clients.
This will be needed for GraphTraits stuff. In the future I will want to
iterate over nodes.

> Do we really want to hit the assert here, and not only return false?
Right. It's here just for debugging purposes. I'll clean most of asserts.

> > +        case Edge::StringIsPrefix:
> 
> Shouldn't the edge be splitted in this case and the payload be attached
> to the new node?
Current implementation doesn't allow to have two strings in the trie,
one being a prefix of another. I will remove this requirement soon. This
will beed to introduce splitting here and some more bookkeeping in
another place.

-- 
With best regards, Anton Korobeynikov.

Faculty of Mathematics & Mechanics, Saint Petersburg State University.