[cfe-dev] [ClangToolsExtra] PreprocessorTrackerImpl's use of llvm::StringPool
Chris Lattner via cfe-dev
cfe-dev at lists.llvm.org
Wed Apr 15 22:29:53 PDT 2020
Cool, thanks! I put up this patch to replace this usage in favor of StringSet - once this goes in, I’ll remove StringPool entirely. I’d appreciate an LGTM, thanks!
> On Apr 15, 2020, at 11:17 AM, Fangrui Song <maskray at google.com> wrote:
> On 2020-04-14, Reid Kleckner via cfe-dev wrote:
>> 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.
> +1. llvm/Support/StringSaver.h:UniqueStringSaver does more or less the
> same thing.
>> 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?
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-dev