[PATCH] D40170: [llvm-tblgen] - Stop using std:string in RecordKeeper.

[Zachary Turner via llvm-commits]

Ok, just a suggestion. If it’s being iterated over it may not be safe to
change. Lgtm otherwise
On Mon, Nov 20, 2017 at 1:15 AM George Rimar via Phabricator <
reviews at reviews.llvm.org> wrote:

> grimar added a comment.
>
> I just realized that I do not know if ordering is important or not. I know
> almost nothing about TableGen.
> Some code iterates over std::map returned, but it does not automatially
> means ordering is really important.
>
> And some code just applies sorting by itself (
> https://github.com/llvm-mirror/llvm/blob/master/utils/TableGen/CTagsEmitter.cpp#L65
> ):
>
>   for (const auto &C : Classes)
>     Tags.push_back(Tag(C->getName(), locate(C)));
>   for (const auto &D : Defs)
>     Tags.push_back(Tag(D->getName(), locate(D)));
>   std::sort(Tags.begin(), Tags.end());
>
> My earlier suggested approach (https://reviews.llvm.org/D40239) also
> changes ordering to insertion order, what may be excessive (if we do not
> need ordering)
> or incorrect (if we need sorted order and not just any fixed order), so I
> now don't think it was good idea to suggest.
>
>
> https://reviews.llvm.org/D40170
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171120/84d115bb/attachment.html>