[cfe-dev] [ClangToolsExtra] PreprocessorTrackerImpl's use of llvm::StringPool

[Reid Kleckner via cfe-dev]

I also noticed StringPool last month and had more or less the same thought:
this looks dead, can we remove it?

+1 for replacing this usage with StringSet and removing StringPool.

On Mon, Apr 13, 2020 at 11:03 AM Chris Lattner via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> Cc John Thompson, but I don’t know his email address.
>
> I recently ran across llvm::StringPool in llvm/lib/Support.  It is an old
> class which isn’t ideal in several ways, and I would like to remove it in
> favor of direct uses of llvm::StringSet, which is nearly the same thing and
> a lot more efficient.  The one difference is that in this one, each entry
> is individually reference counted, so they get deallocated as soon as the
> last reference is dropped.
>
> The only use of this class is in PreprocessorTrackerImpl in
> clang-tools-extra/modularize/PreprocessorTracker.cpp.  Does anyone object
> to changing this to being a simple llvm::StringSet?  Such a change would
> make this more efficient (and allow deleting a misleading class out of
> llvm/lib/Support), allow simplifying away the "StringHandle” logic (in
> favor of using StringRef directly) and the only cost that I can see is that
> the strings won’t get deallocated as eagerly.
>
> An alternative solution here is to move StringPool into clang-tools-extra
> as a private implementation detail.  I’d personally rather remove it
> outright than preserve it.
>
> Any thoughts?
>
> -Chris
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200414/7406cd83/attachment.html>